[Interop] Orphaned listitems should be exposed as generic role [chromium/src : main]
Aaron Leventhal (Gerrit)
Aaron Leventhal added 1 comment![]( "Open in Gerrit")
IIRC we tried to change this before and it broke something in Slack where there was an intervening element between the list ancestor and the list item. I haven't looked yet, but does this CL address that case?
Related details
- Jocelyn Tran
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal added 3 comments![]( "Open in Gerrit")
++++++++genericContainer htmlTag='div'This test was added here: https://issues.chromium.org/issues/40815275 via this CL: https://chromium-review.googlesource.com/c/chromium/src/+/3398126 in order to address a regression.
Specifically, the CL says, 'the children of the "directory" role are also expected to be listitems.
Investigate a bit more and let me know what you think.
<li>My concern is that the former is the way many authors have written lists over the years, even though it's wrong.
I remember fixing an issue with this back in the Firefox days where we needed to make things less restrictive and accept this case.
Is there a way we can pass the test and still allow the old structure? We should be testing both ways.
AXObject* ancestor = CachedParentObject();Let's switch this to using ParentObjectUnignored(). That way, if there is an intervening object with role=none (which is a legal way to remove something from the a11y tree), it will not break the content's expectation that there is a list with listitems.
Related details
- Jocelyn Tran
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal added 2 comments![]( "Open in Gerrit")
This CL also fixes aria-level.html, list-item-level.html, andI would rather have us test both the incorrect and correct authoring approach, since I've definitely seen content do nested lists the incorrect way.
AXObject* ancestor = CachedParentObject();Let's switch this to using ParentObjectUnignored(). That way, if there is an intervening object with role=none (which is a legal way to remove something from the a11y tree), it will not break the content's expectation that there is a list with listitems.
Actually (if we need to continue pursuing this change), please add a new method ParentObjectNonGeneric() that works like ParentObjectUnignored() but also skips kGenericContainer. See Scott's comment here which shows an intervening div: https://github.com/web-platform-tests/wpt/issues/45715#issuecomment-2057463585
Jocelyn Tran (Gerrit)
Jocelyn Tran voted and added 4 comments![]( "Open in Gerrit")
Votes added by Jocelyn Tran
| Commit-Queue | +1 |
4 comments
This CL also fixes aria-level.html, list-item-level.html, andI would rather have us test both the incorrect and correct authoring approach, since I've definitely seen content do nested lists the incorrect way.
Done
This test was added here: https://issues.chromium.org/issues/40815275 via this CL: https://chromium-review.googlesource.com/c/chromium/src/+/3398126 in order to address a regression.
Specifically, the CL says, 'the children of the "directory" role are also expected to be listitems.
Investigate a bit more and let me know what you think.
Marking as resolved as this file is no longer in this CL.
My concern is that the former is the way many authors have written lists over the years, even though it's wrong.
I remember fixing an issue with this back in the Firefox days where we needed to make things less restrictive and accept this case.
Is there a way we can pass the test and still allow the old structure? We should be testing both ways.
Done
Aaron LeventhalLet's switch this to using ParentObjectUnignored(). That way, if there is an intervening object with role=none (which is a legal way to remove something from the a11y tree), it will not break the content's expectation that there is a list with listitems.
Actually (if we need to continue pursuing this change), please add a new method ParentObjectNonGeneric() that works like ParentObjectUnignored() but also skips kGenericContainer. See Scott's comment here which shows an intervening div: https://github.com/web-platform-tests/wpt/issues/45715#issuecomment-2057463585
- Aaron Leventhal
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal added 4 comments![]( "Open in Gerrit")
Looks good except for tests that are no longer testing what they meant to test.
++++++genericContainerProbably should put the listitem inside of a list for this test, because it's supposed to test whether expanded is supported on listitem, but it's now only testing whether expanded is supported on generic.
++[section]We should put these list items in a list so that we test the same thing as before.
++++++genericContainerFor this test, I think we should have one display:list-item inside of a list, and one outside, so we test both situations.
Related details
- Jocelyn Tran
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jocelyn Tran (Gerrit)
Jocelyn Tran added 3 comments![]( "Open in Gerrit")
Probably should put the listitem inside of a list for this test, because it's supposed to test whether expanded is supported on listitem, but it's now only testing whether expanded is supported on generic.
Done
++[section]We should put these list items in a list so that we test the same thing as before.
We already have listitems in a list right below this. Should we keep these listitems outside of a list so that we can test that they are correctly exposed?
For this test, I think we should have one display:list-item inside of a list, and one outside, so we test both situations.
- Aaron Leventhal
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Aaron Leventhal (Gerrit)
Aaron Leventhal voted and added 2 comments![]( "Open in Gerrit")
Votes added by Aaron Leventhal
| Code-Review | +1 |
2 comments
++[section]Jocelyn TranWe should put these list items in a list so that we test the same thing as before.
We already have listitems in a list right below this. Should we keep these listitems outside of a list so that we can test that they are correctly exposed?
- Jocelyn Tran
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jocelyn Tran (Gerrit)
Jocelyn Tran added 1 comment![]( "Open in Gerrit")
Hi Aki, could you please take a look at this CL? I'm looking for owners approval for `chrome/browser/resources/chromeos/accessibility/switch_access/ switch_access_predicate_test.js`.
For context, I made the listitem in the test the child of a list so that it would be treated as a listitem. Otherwise, the listitem would not be treated as a listitem (and it would not pass the clickable listitem test on line 276).
Related details
- Akihiro Ota
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Akihiro Ota (Gerrit)
Akihiro Ota voted and added 1 comment![]( "Open in Gerrit")
Votes added by Akihiro Ota
| Code-Review | +1 |
1 comment
Thanks Jocelyn, `switch_access_predicate_test.js` LGTM - didn't look too closely at the other files 😊
Related details
- Jocelyn Tran
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jocelyn Tran (Gerrit)
Jocelyn Tran voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
22 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
[Interop] Orphaned listitems should be exposed as generic role
According to [1], if a listitem is not a child of ol, menu, or ul, or if
the containing list element is no longer exposed with a list role, then
the listitem element should be exposed with a generic role.
This CL adds ParentObjectUnignoredNonGeneric() which skips intervening
divs when looking for the parent of a listitem. See [2] for context.
These changes make the following web platform subtests pass:
el-li-orphaned in html-aam/roles-generic.html
orphan div with listitem role in wai-aria/role/list-roles.html
[1]https://w3c.github.io/html-aam/#el-li
[2]https://github.com/web-platform-tests/wpt/issues/45715#issuecomment-2057463585
- M chrome/browser/resources/chromeos/accessibility/switch_access/switch_access_predicate_test.js
- M content/test/data/accessibility/aria/aria-expanded-roles-supported-expected-blink.txt
- M content/test/data/accessibility/aria/aria-expanded-roles-supported.html
- M content/test/data/accessibility/aria/aria-level-expected-auralinux.txt
- M content/test/data/accessibility/aria/aria-level-expected-blink.txt
- M content/test/data/accessibility/aria/aria-level-expected-uia-win.txt
- M content/test/data/accessibility/aria/aria-level-expected-win.txt
- M content/test/data/accessibility/aria/aria-level.html
- M content/test/data/accessibility/aria/aria-owns-expected-android-assist-data.txt
- M content/test/data/accessibility/aria/aria-owns-expected-android-external.txt
- M content/test/data/accessibility/aria/aria-owns-expected-auralinux.txt
- M content/test/data/accessibility/aria/aria-owns-expected-blink.txt
- M content/test/data/accessibility/css/display-list-item-expected-blink.txt
- M content/test/data/accessibility/css/display-list-item.html
- M content/test/data/accessibility/html/list-item-level-expected-blink.txt
- M content/test/data/accessibility/html/list-item-level.html
- M content/test/data/accessibility/html/reparent-crash-expected-blink.txt
- M third_party/blink/renderer/modules/accessibility/ax_object.cc
- M third_party/blink/renderer/modules/accessibility/ax_object.h
- M third_party/blink/web_tests/platform/linux-chrome/external/wpt/html-aam/roles-generic-expected.txt
- M third_party/blink/web_tests/platform/linux-chrome/external/wpt/wai-aria/role/list-roles-expected.txt
Code-Review: +1 by Akihiro Ota, +1 by Aaron Leventhal
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |