Bug 130563 – AX: AccessibilityObject::children() returns invalid results sometimes

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[James Craig]

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()

[Radar WebKit Bug Importer]

<rdar://problem/16388161>

[chris fleizach]

Created attachment 227699 [details]
patch

[WebKit Commit Bot]

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.

[chris fleizach]

Created attachment 227701 [details]
patch

[James Craig]

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.

[chris fleizach]

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.

[chris fleizach]

Created attachment 227703 [details]
patch

[James Craig]

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.

[chris fleizach]

Created attachment 227704 [details]
patch

[Joseph Pecoraro]

Comment on attachment 227704 [details]
patch

r+

Is this likely to produce very large lists of children if you select something like <body>?

[James Craig]

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.

[chris fleizach]

http://trac.webkit.org/changeset/166703