Bug 174965 – AX: findMatchingObjects doesn't work when the startObject is ignored

• Component: Accessibility
• Status: RESOLVED FIXED

[Nan Wang]

When we pass an ignored object as the start object to findMatchingObjects, we are getting either the first element or last element of the container.

<rdar://problem/31823926>

[Nan Wang]

Created attachment 316694 [details]
patch

[chris fleizach]

Comment on attachment 316694 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316694&action=review

> Source/WebCore/accessibility/AccessibilityObject.cpp:615
> +    if (startObject && searchPosition == WTF::notFound && childrenSize > 0) {

what does childrenSize == 0 mean in this context? it seems like we would still want to go up to find a valid start element

> Source/WebCore/accessibility/AccessibilityObject.cpp:617
> +        if (startObject->isDescendantOfObject(object) && startObject->accessibilityIsIgnored()) {

does it matter that it's ignored? It's already not found in the children list right?

> Source/WebCore/accessibility/AccessibilityObject.cpp:618
> +            AccessibilityObject* searchObject = startObject;

if you made searchObject = startObject->parent(), then your next while loop just looks like

while (searchObject && ...) which is a bit clearer

> Source/WebCore/accessibility/AccessibilityObject.cpp:620
> +            while (searchObject->parentObject() && searchObject->parentObject()->accessibilityIsIgnored())

do you want to stop if searchObject == object? if you go higher than that then searchChildren wouldn't be correct

> Source/WebCore/accessibility/AccessibilityObject.cpp:625
> +            searchPosition = searchObject ? searchChildren.find(searchObject) : WTF::notFound;

this is the same line as 613

I wonder if you could delete this line, move line 613 here and then your first if check is just
if (startObject && startObject->isIgnored()), because we know if it's ignore, searchPos == WTF::NotFound

[Nan Wang]

Comment on attachment 316694 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316694&action=review

>> Source/WebCore/accessibility/AccessibilityObject.cpp:615
>> +    if (startObject && searchPosition == WTF::notFound && childrenSize > 0) {
> 
> what does childrenSize == 0 mean in this context? it seems like we would still want to go up to find a valid start element

The function is appendChildrenToArray, I think if childrenSize == 0 means we don't want to append anything into this array.

>> Source/WebCore/accessibility/AccessibilityObject.cpp:617
>> +        if (startObject->isDescendantOfObject(object) && startObject->accessibilityIsIgnored()) {
> 
> does it matter that it's ignored? It's already not found in the children list right?

Probably right.

>> Source/WebCore/accessibility/AccessibilityObject.cpp:620
>> +            while (searchObject->parentObject() && searchObject->parentObject()->accessibilityIsIgnored())
> 
> do you want to stop if searchObject == object? if you go higher than that then searchChildren wouldn't be correct

Ok I'll add that

>> Source/WebCore/accessibility/AccessibilityObject.cpp:625
>> +            searchPosition = searchObject ? searchChildren.find(searchObject) : WTF::notFound;
> 
> this is the same line as 613
> 
> I wonder if you could delete this line, move line 613 here and then your first if check is just
> if (startObject && startObject->isIgnored()), because we know if it's ignore, searchPos == WTF::NotFound

Ok

[Nan Wang]

Created attachment 316708 [details]
patch

update from review

[WebKit Commit Bot]

Comment on attachment 316708 [details]
patch

Clearing flags on attachment: 316708

Committed r220042: <http://trac.webkit.org/changeset/220042>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.