Bug 183661 – AX: AccessibilityNodeObject::textForLabelElement() doesn't follow AccName calculation rules

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

[Joanmarie Diggs]

AccessibilityNodeObject::textForLabelElement() doesn't include pseudo-element text. For platforms where accessible objects get their name from the associated label element, not including pseudo-element text means part of the name is missing.

[Radar WebKit Bug Importer]

<rdar://problem/38502201>

[Joanmarie Diggs]

Created attachment 335851 [details]
Patch

[chris fleizach]

Comment on attachment 335851 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1288
> +    if (result.isEmpty() && labelObject && (label->beforePseudoElement() || label->afterPseudoElement()))

do we need to check the pseudo element status? would we not just want to default to textUnderElement() anyway?

is there any possibility that a self referential label would lead to recursion here?

[Joanmarie Diggs]

Chris: Any suggestions for how to test this on your platform? I was thinking the label text would show up as one of the text alternatives (e.g. AXTitle or AXDescription). But as you'll see in the results I generated in macOS, that's not the case.

[chris fleizach]

Comment on attachment 335851 [details]
Patch

it's possible Mac is not using textForLabelElement since we return the list of elements in titleUIElements. possible there is no Mac test here beyond what you generated

[EWS Watchlist]

Comment on attachment 335851 [details]
Patch

Attachment 335851 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6966047

New failing tests:
accessibility/label-with-pseudo-elements.html

[EWS Watchlist]

Created attachment 335860 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Joanmarie Diggs]

(In reply to chris fleizach from comment #3)
> Comment on attachment 335851 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335851&action=review
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1288
> > +    if (result.isEmpty() && labelObject && (label->beforePseudoElement() || label->afterPseudoElement()))
> 
> do we need to check the pseudo element status? would we not just want to
> default to textUnderElement() anyway?

I took a look at some other AccName failures. The answer is "no" to both of your questions it turns out. For instance, if the label has children then the AccName algorithm descends the children and builds the name from them. So the problem is bigger than pseudo elements and cannot be solved by textUnderElement(). I'll do a new version in a little bit.

> is there any possibility that a self referential label would lead to
> recursion here?

I don't see how. Did you have a specific case in mind? Regardless I can add a self-referential label to the layout test.

Thanks for the review!

[Joanmarie Diggs]

Created attachment 335935 [details]
Patch

[Joanmarie Diggs]

Thanks for the review Chris!

I had to play "guess the Windows results" and I apparently guessed wrong. I'll do a revised patch with whatever the bot claims are the right results for Windows once it finishes running.

[EWS Watchlist]

Comment on attachment 335935 [details]
Patch

Attachment 335935 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6982133

New failing tests:
accessibility/label-with-pseudo-elements.html

[EWS Watchlist]

Created attachment 335942 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Joanmarie Diggs]

Created attachment 335943 [details]
patch including results for 'win-future'

[EWS Watchlist]

Comment on attachment 335943 [details]
patch including results for 'win-future'

Attachment 335943 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6983725

New failing tests:
accessibility/label-with-pseudo-elements.html

[EWS Watchlist]

Created attachment 335952 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Joanmarie Diggs]

Created attachment 335957 [details]
patch with results for 'win' rather than 'win-future'

[WebKit Commit Bot]

Comment on attachment 335957 [details]
patch with results for 'win' rather than 'win-future'

Clearing flags on attachment: 335957

Committed r229682: <https://trac.webkit.org/changeset/229682>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.