Summary: | AX: [ATK] We need to be smarter about flattening and the accessible text implementation | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joanmarie Diggs <jdiggs> | ||||||
Component: | Accessibility | Assignee: | Joanmarie Diggs <jdiggs> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, gyuyoung.kim, jcraig, mario, samuel_white, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Attachments: |
|
Description
Joanmarie Diggs
2015-05-05 14:04:56 PDT
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. |