HTML: Sequential focus navigation with shadow dom #17523
Conversation
wpt-pr-bot
requested review from
annevk,
domenic,
foolip,
jdm,
jgraham,
zcorpan and
zqzhang
There was a problem hiding this comment.
Some style nits, but this overall looks good. We may also want to move into shadow-dom/
It would be good to get a second implementers' eye on the setup + expected orders (which are the most important parts of the tests). @rniwa maybe :)
| // <div #belowSlot tabindex=-1> | ||
| // <div #belowHost tabindex=0> | ||
|
|
||
| async_test(async (t) => { |
There was a problem hiding this comment.
This could be promise_test( and then you wouldn't need t.step or t.done()
| setTabIndex([aboveHost, host, belowHost], 0); | ||
| resetFocus(); | ||
| const expectedOrder = [aboveHost, host, belowHost]; | ||
| for (let el of expectedOrder) { |
There was a problem hiding this comment.
Can this be a utility function too? It seems like the loop is the same in all tests.
await assertFocusOrder([aboveHost, host, belowHost]);| } | ||
| } | ||
|
|
||
| function navigateFocusForward() { |
There was a problem hiding this comment.
Can you just use the shared one and then say const focused = shadowRoot.activeElement || document.activeElement afterward?
|
cc @annevk and/or @smaug---- for Firefox? |
| @@ -69,3 +70,14 @@ function navigateFocusForward() { | |||
| return test_driver.send_keys(document.body, "\ue004"); | |||
| } | |||
|
|
|||
| function assertFocusOrder(expectedOrder) { | |||
There was a problem hiding this comment.
The return new Promise isn't needed here. Just make the whole function async, and then put the loop body inside the function:
async function assertFocusOrder(expectedOrder) {
let shadowRoot = document.getElementById("host").shadowRoot;
for (let el of expectedOrder) {
await navigateFocusForward();
let focused = shadowRoot.activeElement ? shadowRoot.activeElement : document.activeElement;
assert_equals(focused, el);
}
}There was a problem hiding this comment.
Oh awesome, thanks :D
|
All but one test (focus-tabindex-order-shadow-zero-delegatesFocus.html) with |
Will ad the delegatesFocus test in the delegatesFocus PR instead (web-platform-tests#18035)
Yay, thanks for trying it out. I've removed the delegatesFocus test, will move that to #18035. |
…#17523) * HTML: Sequential focus navigation with shadow dom The delegatesFocus test will be in the delegatesFocus PR (web-platform-tests#18035)
This defines an explicit list for the document's "sequential focus navigation order", whose contents are defined to include elements in shadow trees. Previously the contents of the sequential focus navigation order were defined mostly implicitly, in the tabindex section. This also expands the ordering requirements for sequential focus navigation order to account for shadow trees and slotted elements. Finally, this has a minor connection to delegatesFocus, in that it excludes elements that are shadow hosts with delegatesFocus = true from being focusable areas. Part of #2013. Tests: web-platform-tests/wpt#17523
This defines an explicit list for the document's "sequential focus navigation order", whose contents are defined to include elements in shadow trees. Previously the contents of the sequential focus navigation order were defined mostly implicitly, in the tabindex section. This also expands the ordering requirements for sequential focus navigation order to account for shadow trees and slotted elements. Finally, this has a minor connection to delegatesFocus, in that it excludes elements that are shadow hosts with delegatesFocus = true from being focusable areas. Part of #2013. Tests: web-platform-tests/wpt#17523
See whatwg/html#4735 for the spec change.
I used the term "flat tree order" in some variable names and comments, but it isn't defined in the spec so it might be confusing. However I'm not sure what else I can use since "composed tree" is also not in the spec, and "shadow-including tree order" is a little bit different because of slots.