GNOME Bugzilla – Bug 469718
Gecko.inDocumentContent() needs to account for 'embedded component'
Last modified: 2007-11-27 19:48:06 UTC
Gecko.inDocumentContent() is used to test if an object is within the 'document frame', which is the root object for a webpage. For XUL applications, the document frame object is an 'embedded component'. These obj.role names must also be tested for.
Created attachment 94219 [details] [review] first version of Gecko.inDocumentContent() needs to account for 'embedded component' Simple but effective patch for http://www.brailcom.org/~pdm/test.xul We may need to do a more thorough test because multiple document frames and embedded components have been seen within a single page. We could test the given obj against getDocumentFrame() to make sure it is the root object for that page. getDocumentFrame() returns the relation placed on the frame pointing to the current active document. This test would be a performance hit but probably a better test.
Created attachment 94255 [details] [review] alternative version for Gecko.inDocumentContent() needs to account for 'embedded component' This version uses the frame relation that points to the current active doc frame/embedded component and compares to the current parent chain object to see if it is within the doc frame.
(In reply to comment #2) > Created an attachment (id=94255) [edit] > alternative version for Gecko.inDocumentContent() needs to account for > 'embedded component' > > This version uses the frame relation that points to the current active doc > frame/embedded component and compares to the current parent chain object to see > if it is within the doc frame. On the surface, this seems like a decent patch (THANKS!), but I think it might need a fair amount of testing to make sure it didn't disrupt some inadvertent logic. I'm also not sure the following line is necessary since ROLE_EMBEDDED in rolenames.py already maps to "embedded component": +ROLE_EMBEDDED_COMPONENT = "embedded component" But, the name "ROLE_EMBEDDED" is indeed inconsistent with the naming pattern and we probably should consider renaming it to ROLE_EMBEDDED_COMPONENT. This would touch default.py, flat_review.py, and rolenames.py.
> I'm also not sure the following line is necessary since ROLE_EMBEDDED in > rolenames.py already maps to "embedded component": > > +ROLE_EMBEDDED_COMPONENT = "embedded component" > > But, the name "ROLE_EMBEDDED" is indeed inconsistent with the naming pattern > and we probably should consider renaming it to ROLE_EMBEDDED_COMPONENT. This > would touch default.py, flat_review.py, and rolenames.py. > Doh! I completely missed ROLE_EMBEDDED. If we go the patch 1 route, I agree, we should change the name. However, I believe patch 2 provides a much better test.
User feedback indicates that patch 2 does have a noticeable impact on performance. Something along the lines of patch 1 is probably our best solution.
Created attachment 99042 [details] [review] post pyatspi version Best solution post-pyatspi migration patch. Hopefully there will never be an 'embedded component' in the chrome.
I think this looks fine. Have you tested it to see if any regressions are introduced? It doesn't seem like it should, but just checking...
I have performed only simple testing, no formal regression tests. I think this fix is simple enough and should not pose any problems.
Excellent. Please commit and close. Thanks!
Committed to trunk and marked as fixed.