When an iframe loads new content after it already has an AccessibilityObject, the child of the iframe will still point to the old scroll area's AccessibilityObject, which will be invalid, rather than the new scroll area's AccessibilityObject. This is a regression caused by https://bugs.webkit.org/show_bug.cgi?id=68636. Prior to 68636, the whole ax tree was cleared when any iframe loaded new content. After 68636, the tree isn't cleared when only an iframe as opposed to the top document reloads. However, if the user has already explored the iframe and then it loads new content, there's now an invalid tree. I'll send a patch that correctly updates an iframe's AccessibilityObject when new content is loaded.
Created attachment 114645 [details] Patch
Comment on attachment 114645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114645&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:441 > + i'm not sure this will be true on webkit1. on wk1 the web area's parent should be an attachment > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3422 > I think we could remove this isAccessibilityRenderObject restriction and postNotification can take an axObject instead. that will reduce the code in this method and you won't need to add your comment > Source/WebCore/accessibility/AccessibilityRenderObject.h:207 > since this method is in AXObject.h now as public, it does not need to be public in AXRenderObject > Source/WebCore/accessibility/AccessibilityScrollView.cpp:183 > + what's up with this change? > Source/WebCore/dom/Document.cpp:2232 > } this change is not reflected in the layout test. it should be in a different bug
Comment on attachment 114645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114645&action=review >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:441 >> + > > i'm not sure this will be true on webkit1. on wk1 the web area's parent should be an attachment This is copied directly from parentObject except with get() instead of getOrCreate(), so I'd be really surprised if it breaks anything. >> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:3422 >> > > I think we could remove this isAccessibilityRenderObject restriction and postNotification can take an axObject instead. that will reduce the code in this method and you won't need to add your comment Good idea! That does simplify the code a lot, I even pulled the postNotification out of the loop. >> Source/WebCore/accessibility/AccessibilityRenderObject.h:207 >> > > since this method is in AXObject.h now as public, it does not need to be public in AXRenderObject Done. >> Source/WebCore/accessibility/AccessibilityScrollView.cpp:183 >> + > > what's up with this change? It was returning the grandparent, not the parent. An iframe in a document might look something like this: GroupRole id=10 UnknownRole id=11 <-- iframe tag ScrollAreaRole id=12 WebAreaRole id=13 Calling parent() on the AccessibilityScrollView (id=12) was returning the parent of the iframe, in this case id=10. My change modifies it to return the renderer of the owner element itself (id=11 in this example). This is necessary for this patch because otherwise setNeedsToUpdateChildren won't get called on the iframe when childrenChanged is walking up the parent chain from the web area. >> Source/WebCore/dom/Document.cpp:2232 >> } > > this change is not reflected in the layout test. it should be in a different bug You're right, I'll pull it into a separate patch.
Created attachment 114726 [details] Patch
Comment on attachment 114726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114726&action=review > Source/WebCore/accessibility/AccessibilityScrollView.h:73 > + mutable bool m_childrenDirty; this shouldn't have to be mutable anymore i think because setNeedsToUpdateChildren() is not const. same probably goes for the ivar in the AX render object as well > Source/WebCore/accessibility/AccessibilityScrollView.h:-85 > - is this removing the newline at the end of the file?
Comment on attachment 114726 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=114726&action=review >> Source/WebCore/accessibility/AccessibilityScrollView.h:73 >> + mutable bool m_childrenDirty; > > this shouldn't have to be mutable anymore i think because setNeedsToUpdateChildren() is not const. same probably goes for the ivar in the AX render object as well Done. >> Source/WebCore/accessibility/AccessibilityScrollView.h:-85 >> - > > is this removing the newline at the end of the file? Sorry - my editor did that automatically. I fixed it manually, will try to avoid that.
Created attachment 114739 [details] Patch
Created attachment 114747 [details] Patch for landing
Comment on attachment 114747 [details] Patch for landing Clearing flags on attachment: 114747 Committed r100057: <http://trac.webkit.org/changeset/100057>
All reviewed patches have been landed. Closing bug.