Summary: | AX: AccessibilityNodeObject::textForLabelElement() doesn't follow AccName calculation rules | ||
---|---|---|---|
Product: | WebKit | Reporter: | Joanmarie Diggs <jdiggs> |
Component: | Accessibility | Assignee: | Joanmarie Diggs <jdiggs> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, ews-watchlist, jcraig, samuel_white, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
Description
Joanmarie Diggs
2018-03-15 08:51:26 PDT
Created attachment 335851 [details]
Patch
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? 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. 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
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 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
(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! Created attachment 335935 [details]
Patch
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. 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 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
Created attachment 335943 [details]
patch including results for 'win-future'
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 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
Created attachment 335957 [details]
patch with results for 'win' rather than 'win-future'
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> All reviewed patches have been landed. Closing bug. |