| Summary: | AX: AccessibilityObject::children() returns invalid results sometimes | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | James Craig <jcraig> | ||||||||||
| Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | commit-queue, jcraig, joepeck, mario, timothy, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Bug Depends on: | 130721 | ||||||||||||
| Bug Blocks: | |||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 227699 [details]
patch
Attachment 227699 [details] did not pass style-queue:
ERROR: Source/WebCore/inspector/InspectorDOMAgent.h:237: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5]
ERROR: Source/WebCore/inspector/InspectorDOMAgent.cpp:1418: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5]
Total errors found: 2 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 227701 [details]
patch
Comment on attachment 227701 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=227701&action=review > Source/WebCore/inspector/InspectorDOMAgent.cpp:1418 > +void InspectorDOMAgent::processAccessibilityChildren(PassRefPtr<AccessibilityObject> axObject, RefPtr<Inspector::TypeBuilder::Array<int>>& childNodeIds) Wouldn't this be better as AccessibilityObject::childrenPromoted() similar to AccessibilityObject::parentObjectUnignored()? This is is the approach I was planning to take if you determined AccessibilityObject::children() behaved correctly. > LayoutTests/inspector-protocol/dom/getAccessibilityPropertiesForNode-expected.txt:397 > - childNodeIds.length: 1 > + childNodeIds.length: 3 There's a FIXME comment in the layout test that should be removed, too. Comment on attachment 227701 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=227701&action=review >> Source/WebCore/inspector/InspectorDOMAgent.cpp:1418 >> +void InspectorDOMAgent::processAccessibilityChildren(PassRefPtr<AccessibilityObject> axObject, RefPtr<Inspector::TypeBuilder::Array<int>>& childNodeIds) > > Wouldn't this be better as AccessibilityObject::childrenPromoted() similar to AccessibilityObject::parentObjectUnignored()? This is is the approach I was planning to take if you determined AccessibilityObject::children() behaved correctly. No. The DOM inspector only cares about Nodes, while the AXTree also cares about render objects. Created attachment 227703 [details]
patch
Comment on attachment 227703 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=227703&action=review > LayoutTests/inspector-protocol/dom/getAccessibilityPropertiesForNode-expected.txt:397 > - childNodeIds.length: 1 > + childNodeIds.length: 3 You need to regenerate the layout test result again now that you've removed the comment. The outerHTML (including FIXME comment) gets output. Created attachment 227704 [details]
patch
Comment on attachment 227704 [details]
patch
r+
Is this likely to produce very large lists of children if you select something like <body>?
It doesn't return all the leaf nodes of the entire tree if that's what you mean. It stops at accessible group boundaries (e.g. a list, but not all it's listitem children). This will return long lists in places, but the UI tweak for that should be covered in bug 130911. |
At least one of the test elements in the inspector-protocol tests gets an invalid result for children(). It should either be including the <span> as an child, or promoting the <span>'s descendant nodes (because the <span> is ignored), but WebCore is doing neither of these things. Somehow it's coming out right in the AX API (the ignored <span> is being exposed as an AXGroup) but that output need to be reconciled with this output. Test case is in LayoutTests/inspector-protocol/dom/getAccessibilityPropertiesForNode.html <div role="group" class="ex"> <!-- FIXME: accessibilityChildNodeIds.length should be 3 (foo, bar, baz). WebCore only reports 1 (foo). --> <div>foo</div> <span> bar <span role="button">baz</span> </span> <div></div> <div></div> </div> Source/WebCore/accessibility/AccessibilityObject.cpp:1389 Blocked by http://webkit.org/b/130264 Probably related to http://webkit.org/b/130181 If there is a reason it's done this way, kick this back to me in the Web Inspector component. It's possible I'll need to add a new method for AccessibilityObject::unignoredChildren()