display contents accessibility tests #43740

rahimabdi · 2023-12-20T02:22:20Z

Closes web-platform-tests/interop-accessibility#60

WPT investigation as part of:

Follow on work in web-platform-tests/interop-accessibility#97

cookiecrook · 2024-01-24T21:59:37Z

Adding @annevk and @dbaron re: the focus tests and

Allow elements with display:contents to be focusable. whatwg/html#9425

annevk

Very nice to see so much additional test coverage. Just a couple tiny nits from my side.

annevk · 2024-01-25T10:43:00Z

css/css-display/focus/display-contents-focus.html

+      promise_test(async t => {
+        el.focus();
+
+        assert_equals(document.activeElement, el, "Element is keyboard focusable");
+        }, `${testName}`);


The indentation here doesn't quite line up, but also, is there a reason this is a promise test? It seems this can just be a test.

Thank you @annevk, I've now wrapped the focusability tests in an anonymous, synchronous test() function which seems to be the approach of other WPT tests.

annevk · 2024-01-25T10:44:24Z

css/css-display/focus/display-contents-focus.html

+    if (!els.length) {
+      throw `Selector passed in verifyElementsAreKeyboardFocusable should match at least one element.`;
+    }


I would leave this out. It should never be hit and makes it take a bit longer to read what is going on.

I'd prefer he keep it. The problem we've seen (on multiple occasions) is that PR authors have misspelled one of the selectors and not noticed a the silent failure of a missing test result in a larger list of subtests... It's sometimes difficult for reviewers to spot minor selector errors, too.

Maybe use assert_unreached(msg) instead of throw

This is outside a test so asserts wouldn't work.

@cookiecrook this querySelector() is the static part of this test, no? Seems like it would not be changed anymore. It seems more likely someone would typo "ex-focusable" when adding a new element above, which we can't really guard against. Or maybe we could, but we aren't now.

I may be misunderstanding your point, but I'm coming around to leaving it out... This is only a help for authors while writing the test, and a classname typo would still result in a silent failure unless all of them had the same typo, so I can see your point about limited utility.

Refactor the throws in aria-utils interop-accessibility#102

@annevk Also linking this PR as a potential avenue for improved error catching: #42769.

As there are other explorations, I'll leave the display: contents test as is but thanks for raising this issue!

As long as that happens I guess that's okay.

…abelsBySelector

annevk · 2024-01-30T08:56:12Z

css/css-display/focus/display-contents-focus.html

+    if (!els.length) {
+      throw `Selector passed in verifyElementsAreKeyboardFocusable should match at least one element.`;
+    }


As long as that happens I guess that's okay.

dbaron · 2024-01-30T14:22:43Z

I'm concerned that one of the tests shows "Harness status: error" across 3 browser engines.

dbaron · 2024-01-30T14:24:47Z

css/css-display/accessibility/display-contents-role-and-label.html

+
+<script>
+  AriaUtils.verifyRolesBySelector(".ex-role");
+  AriaUtils.verifyRoleAndLabelBySelector(".ex-role-and-label");


Ah, the reason for the error is that the function name here is verifyRoleAndLabelBySelector but the function name that you've added in AriaUtils is verifyRolesAndLabelsBySelector.

Thanks @dbaron, updated the <script> block with correct (and new) function name.

dbaron

The focus test looks good to me, though see one comment below and one earlier comment.

dbaron · 2024-01-30T14:30:11Z

css/css-display/focus/display-contents-focus.html

+      let testName = el.getAttribute("data-testname");
+      test(() => {
+        el.focus();
+        assert_equals(document.activeElement, el, "Element is keyboard focusable");


I think this test seems reasonable, but I'd probably describe the test assertion as Element is focusable with Element.focus() since this isn't specifically testing keyboard focus.

If you wanted to also test something like tabbing you could use the approach that I used in the slot-element-tabbable.tentative.html test in #39247 , though it's probably not necessary.

Agreed, I've updated the test name to be more specific. I've also modified the local focusability function name:

OLD: verifyElementsAreKeyboardFocusable() -> NEW: verifyElementsAreFocusable()

…ole-and-label.html

cookiecrook

Approving with David's requested change in https://github.com/web-platform-tests/wpt/pull/43740/files#r1471328585

…/pull/43740

* New tests for display : contents accessibility, verifying role and label expectations Authored-by: Rahim Abdi <rahim_abdi@apple.com>

Rahim Abdi added 15 commits December 18, 2023 19:59

Create display-contents-accessibility.html

d991102

Add tests for buttons, links, generics, headings

7e99ddc

Add more tests

992e5c8

Change file content structure

4f0cca3

Add grid test

69107f7

Add display grid, flex tests

d8c6ec9

Add more tests

7f03352

Add more list tests

0ca9f24

Add ARIA table tests

fc63d28

Add grid and treegrid tests

ce5868e

Fix markup error with table test

11eeecb

Add aria-label to form test to ensure it's provided accname

374c9db

Add accname section and initial tests

aa7688b

Add more accname tests

d9de7a6

Fix typos

dcfd400

wpt-pr-bot added the css-display label Dec 20, 2023

wpt-pr-bot assigned fantasai Dec 20, 2023

wpt-pr-bot requested review from fantasai, kojiishi and tabatkins December 20, 2023 02:22

rahimabdi requested review from cookiecrook, jensimmons, spectranaut, jnurthen and MelSumner and removed request for tabatkins, fantasai and kojiishi December 20, 2023 02:23

rahimabdi assigned rahimabdi and unassigned fantasai Dec 20, 2023

cookiecrook requested a review from dbaron January 24, 2024 21:59

cookiecrook mentioned this pull request Jan 24, 2024

display: contents accessibility web-platform-tests/interop#568

Closed

3 tasks

rahimabdi mentioned this pull request Jan 24, 2024

Investigate keyboard operability testing web-platform-tests/interop-accessibility#101

Open

annevk reviewed Jan 25, 2024

View reviewed changes

This was referenced Jan 26, 2024

Refactor the throws in aria-utils web-platform-tests/interop-accessibility#102

Open

[accname] Require specification of subtest count #42769

Closed

Rahim Abdi added 4 commits January 29, 2024 17:54

Update function name: verifyRoleAndLabelBySelector -> verifyRolesAndL…

b98d727

…abelsBySelector

Flesh out role=grid, tabindex=0 test

eb6f5e6

Refactor focus tests to use generic test() rather than promise

e112517

Remove extra line breaks in function verifyRolesAndLabels..()

2e5489f

rahimabdi requested a review from cookiecrook January 30, 2024 03:04

Update documentation for verifyRolesandLabels..()

610f48d

annevk approved these changes Jan 30, 2024

View reviewed changes

dbaron reviewed Jan 30, 2024

View reviewed changes

dbaron approved these changes Jan 30, 2024

View reviewed changes

Rahim Abdi added 2 commits January 30, 2024 11:47

Fix verifyRolesAndLabels..() in <script> block for display-contents-r…

6d3e64f

…ole-and-label.html

Update test description for such tests in display-contents-focus.html

49707e0

cookiecrook approved these changes Jan 30, 2024

View reviewed changes

Rahim Abdi added 2 commits January 30, 2024 11:52

Update verifyElementsAreFocusable() function name

fadd813

Minor documentation fix for description list

829393f

cookiecrook changed the title ~~Create css/css-display/display-contents-accessibility.html~~ display contents accessibility tests Jan 31, 2024

cookiecrook merged commit 2883689 into master Jan 31, 2024
20 checks passed

cookiecrook deleted the rahim/display-contents-accessibility-test branch January 31, 2024 17:16

github-actions bot pushed a commit to web-platform-tests/wpt-metadata that referenced this pull request Jan 31, 2024

updating to match new file path direction from web-platform-tests/wpt…

65ec319

…/pull/43740

github-actions bot pushed a commit to web-platform-tests/wpt-metadata that referenced this pull request Jan 31, 2024

updating to match new file path direction from web-platform-tests/wpt…

22db139

…/pull/43740

mbrodesser-Igalia mentioned this pull request Feb 21, 2024

Change namespace for Trusted-Types tests for setAttributeNS to null mbrodesser-Igalia/wpt#2

Closed

marcoscaceres pushed a commit that referenced this pull request Feb 23, 2024

display contents accessibility tests (#43740)

d1a1492

* New tests for display : contents accessibility, verifying role and label expectations Authored-by: Rahim Abdi <rahim_abdi@apple.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

display contents accessibility tests #43740

display contents accessibility tests #43740

rahimabdi commented Dec 20, 2023 •

edited by cookiecrook

Loading

cookiecrook commented Jan 24, 2024 •

edited

Loading

annevk left a comment

annevk Jan 25, 2024

rahimabdi Jan 30, 2024 •

edited

Loading

annevk Jan 25, 2024

cookiecrook Jan 25, 2024

zcorpan Jan 25, 2024

annevk Jan 26, 2024

cookiecrook Jan 26, 2024

cookiecrook Jan 26, 2024

rahimabdi Jan 30, 2024

annevk Jan 30, 2024

annevk Jan 30, 2024

dbaron commented Jan 30, 2024

dbaron Jan 30, 2024

rahimabdi Jan 30, 2024

dbaron left a comment

dbaron Jan 30, 2024

rahimabdi Jan 30, 2024 •

edited

Loading

cookiecrook left a comment

display contents accessibility tests #43740

display contents accessibility tests #43740

Conversation

rahimabdi commented Dec 20, 2023 • edited by cookiecrook Loading

cookiecrook commented Jan 24, 2024 • edited Loading

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimabdi Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbaron commented Jan 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbaron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahimabdi Jan 30, 2024 • edited Loading

Choose a reason for hiding this comment

cookiecrook left a comment

Choose a reason for hiding this comment

rahimabdi commented Dec 20, 2023 •

edited by cookiecrook

Loading

cookiecrook commented Jan 24, 2024 •

edited

Loading

rahimabdi Jan 30, 2024 •

edited

Loading

rahimabdi Jan 30, 2024 •

edited

Loading