Given: <div> <span>Span!</span> <h1>Foo</h1> <h2>Bar</h2> <p>Hello world</p> </div> There is an object of ATK_ROLE_SECTION which implements the accessible text interface and contains the text of all of its children: 'Span!\nFoo\n\nBar\n\nHello world' Getting rid of the span results in the ATK_ROLE_SECTION going away. While the above test case is not a big deal, given a page with a ton of content in a div with a single span, we're doing a lot of work getting all the text. That's bad for performance. Furthermore, for a variety of reasons, Orca is going to have to implement its own caret navigation for WebKitGtk. Having duplicated text (the headings and paragraphs are still there in the tree, correctly implementing AtkText) is really screwing things up. So we need to be smarter about our flattening. I have some ideas which I'll pursue.
<rdar://problem/20824771>
The idea, which I'm pursuing, will be to defer to WebCore Accessibility regarding inclusion of anonymous blocks. What we're doing in ATK at the moment is a heuristic hack. But before we can defer to WebCore, we need to sort out things like bug 144653. (And there will likely be many more such issues.)
Created attachment 277439 [details] Patch
Comment on attachment 277439 [details] Patch Chris: When you have a spare moment, please review. Thanks!
Comment on attachment 277439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277439&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:-2651 > - if (node && (node->hasTagName(tdTag) || node->hasTagName(thTag))) might be worth putting this in a helper method > Source/WebCore/accessibility/atk/AccessibilityObjectAtk.cpp:91 > + return renderObject->firstChildSlow() ? IncludeObject : DefaultBehavior; will this handle inlines? would doing children().size() ? be a better check
Created attachment 277554 [details] Patch
Comment on attachment 277439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277439&action=review > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2652 > + || (m_renderer->isAnonymous() && m_renderer->isTablePart())) The helper method makes sense, but after making some additional test cases, I discovered that we don't want the anonymous check (fully-formed CSS tables which include "display:table-cell" have a non-anonymous renderer). "is<RenderTableCell>(m_renderer)" seems to be what we need. So I wound up not making the helper method in the new patch. >> Source/WebCore/accessibility/atk/AccessibilityObjectAtk.cpp:91 >> + return renderObject->firstChildSlow() ? IncludeObject : DefaultBehavior; > > will this handle inlines? would doing children().size() ? be a better check Since we're looking at the RenderObject children, it should. And if you mean calling AccessibilityObject::children().size(), is it safe to get the children of the AccessibilityObject in the method being used to determine if that AccessibilityObject should be included or ignored? Also, I don't think we want to base our decision on what AccessibilityObject children are present. (Though perhaps I am missing something.) All of that said, your question led me to make additional test cases in order to see where WebCore Accessibility failed to include a paragraph element I'd expect included. And the only place I've found a difference is when that paragraph element's renderer has a RenderBlock child. So the new check looks explicitly for that condition and leaves the rest to WebCore Accessibility. And the new patch has more paragraph tests.
Comment on attachment 277554 [details] Patch Clearing flags on attachment: 277554 Committed r200188: <http://trac.webkit.org/changeset/200188>
All reviewed patches have been landed. Closing bug.