HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation #18035

rakina · 2019-07-24T13:24:04Z

WPT for whatwg/html#4796.
As noted there, the behavior in Chrome for focus() when the flattened tabindex-ordered focus navigation scope is empty (there's nothing to focus on) so it fails in some of the tests.

shadow-dom/focus/focus-method-delegatesFocus.html

Will ad the delegatesFocus test in the delegatesFocus PR instead (web-platform-tests#18035)

…#17523) * HTML: Sequential focus navigation with shadow dom The delegatesFocus test will be in the delegatesFocus PR (web-platform-tests#18035)

…hadow dom, a=testonly Automatic update from web-platform-tests HTML: Sequential focus navigation with shadow dom (#17523) * HTML: Sequential focus navigation with shadow dom The delegatesFocus test will be in the delegatesFocus PR (web-platform-tests/wpt#18035) -- wpt-commits: 888da5c479354a840d253c4acbceb18999bb9bf7 wpt-pr: 17523

…#17523) * HTML: Sequential focus navigation with shadow dom The delegatesFocus test will be in the delegatesFocus PR (web-platform-tests#18035)

…focusing steps

rakina · 2019-09-25T08:21:14Z

Updated the tests after the change in whatwg/html#4796. We have tests for focus() and sequential focus navigation here, since click focus delegation can't really be tested? (there is no difference between focusable and click focusable in Blink)

domenic

Very nice exhaustive tests :D.

We should probably also add tests for .click(), I think... in particular checking that it doesn't use sequential focus navigation order, in the way Chrome does currently.

shadow-dom/focus/focus-tabindex-order-shadow-varying-delegatesFocus.html

domenic · 2019-09-26T08:29:44Z

shadow-dom/focus/focus-tabindex-order-shadow-zero-delegatesFocus.html

+  resetFocus();
+  // Focus should move in flat tree order since every one of them has tabindex ==0,
+  // but doesn't include slots and #host.
+  return assertFocusOrder(elementsInFlatTreeOrder.filter(el => (el !== slotAbove && el !== slotBelow && el !== host)));


I think listing them explicitly would make the test easier to read...

Yeah that makes sense. Changed!

domenic

LGTM with nit about explicitly listing them

rakina · 2019-09-26T09:37:58Z

We should probably also add tests for .click(), I think... in particular checking that it doesn't use sequential focus navigation order, in the way Chrome does currently.

click() fires a synthetic click event, which triggers activation behavior but not focus - discovered this with @tkent-google earlier today, gotta update that part of the spec D: - so we can't test click focus with that. I'm not sure if there's another way to trigger clicks in WPTs...

tkent-google · 2019-09-26T10:06:10Z

I'm not sure if there's another way to trigger clicks in WPTs...

You can do it by test_driver.click(element).

rakina · 2019-09-27T02:14:48Z

I'm not sure if there's another way to trigger clicks in WPTs...

You can do it by test_driver.click(element).

Tried this just now, looks like this also doesn't trigger the focus on any element I tried it on :(

domenic · 2019-09-27T02:21:58Z

Let's at least add a basic test or two that .click() does not focus things, since we also need to update that in the PR.

rakina · 2019-09-27T04:09:15Z

Let's at least add a basic test or two that .click() does not focus things, since we also need to update that in the PR.

Makes sense, added. Thanks!

domenic · 2019-09-27T04:12:19Z

shadow-dom/focus/click-focus-delegatesFocus.html

+test(() => {
+  resetTabIndexAndFocus();
+  setAllTabIndex(0);
+  test_driver.click(host);


I don't think we should have these test_driver tests. It feels like a bug in test_driver that it doesn't properly emulate real clicks. So, this test is really a test of test_driver, not a test of the spec, and I think it's testing that test_driver has a bug.

Just found out that test_driver.click actually works (click() still won't focus though), so I separated the test driver tests now. PTAL!

tkent-google · 2019-09-27T04:17:07Z

shadow-dom/focus/click-focus-delegatesFocus.html

+test(() => {
+  resetTabIndexAndFocus();
+  setAllTabIndex(0);
+  test_driver.click(host);


test_driver.click() is asynchronous. We need to wait until the retuned promise is resolved.

promise_test(async () => { reset... setAll... await test_driver.click(host); assert_equals(... assert_equals(... }, ...

OMG, you're right. It actually focuses now!! Thank you :D

shadow-dom/focus/click-focus-delegatesFocus.html

shadow-dom/focus/click-focus-delegatesFocus-tabindex-varies.html

…hadow dom, a=testonly Automatic update from web-platform-tests HTML: Sequential focus navigation with shadow dom (#17523) * HTML: Sequential focus navigation with shadow dom The delegatesFocus test will be in the delegatesFocus PR (web-platform-tests/wpt#18035) -- wpt-commits: 888da5c479354a840d253c4acbceb18999bb9bf7 wpt-pr: 17523 UltraBlame original commit: 514a4abb6174e424e238d5e3ee31819777408e49

We should use flat-tree order instead of tabindex order, add test to ensure that.

rniwa · 2019-10-11T05:57:26Z

shadow-dom/focus/focus-method-delegatesFocus.html

+  // No focusable element under #host in the flat tree.
+  host.focus();
+  assert_equals(shadowRoot.activeElement, null);
+  assert_equals(document.activeElement, null);


This should be document.body instead.

rniwa · 2019-10-11T05:57:36Z

shadow-dom/focus/focus-method-delegatesFocus.html

+  // No focusable element under #host in the flat tree.
+  host.focus();
+  assert_equals(shadowRoot.activeElement, null);
+  assert_equals(document.activeElement, null);


rniwa · 2019-10-11T07:05:49Z

shadow-dom/focus/click-focus-delegatesFocus-tabindex-varies.html

+  setTabIndex([aboveSlot], 2);
+  setTabIndex([slot, slotted], 1);
+  await test_driver.click(host);
+  assert_equals(shadowRoot.activeElement, null);


Clearly you meant aboveSlot here.

Oops, thanks!

rniwa · 2019-10-11T07:05:56Z

shadow-dom/focus/click-focus-delegatesFocus-tabindex-varies.html

+  setTabIndex([slot, slotted], 1);
+  await test_driver.click(host);
+  assert_equals(shadowRoot.activeElement, null);
+  assert_equals(document.activeElement, aboveSlot);


And host here.

rakina

Thanks for the review @rniwa!

rakina · 2019-10-11T11:46:46Z

shadow-dom/focus/click-focus-delegatesFocus-tabindex-varies.html

+  setTabIndex([slot, slotted], 1);
+  await test_driver.click(host);
+  assert_equals(shadowRoot.activeElement, null);
+  assert_equals(document.activeElement, aboveSlot);


rakina · 2019-10-11T11:46:52Z

shadow-dom/focus/click-focus-delegatesFocus-tabindex-varies.html

+  setTabIndex([aboveSlot], 2);
+  setTabIndex([slot, slotted], 1);
+  await test_driver.click(host);
+  assert_equals(shadowRoot.activeElement, null);


Oops, thanks!

rakina · 2019-10-11T11:46:59Z

shadow-dom/focus/focus-method-delegatesFocus.html

+  // No focusable element under #host in the flat tree.
+  host.focus();
+  assert_equals(shadowRoot.activeElement, null);
+  assert_equals(document.activeElement, null);


rakina · 2019-10-11T11:47:03Z

shadow-dom/focus/focus-method-delegatesFocus.html

+  // No focusable element under #host in the flat tree.
+  host.focus();
+  assert_equals(shadowRoot.activeElement, null);
+  assert_equals(document.activeElement, null);


rniwa

r=me.

This is the last piece of #2013. Incoporating support for delegates-focus shadow roots into the focusing steps impact the behavior of programmatic and click focus. (Sequential focus navigation was handled in d19d963.) Tests: web-platform-tests/wpt#18035

wpt-pr-bot added shadow-dom wg-webplatform labels Jul 24, 2019

wpt-pr-bot assigned TakayoshiKochi Jul 24, 2019

wpt-pr-bot requested review from hayatoito, kojiishi, rniwa and TakayoshiKochi July 24, 2019 13:24

rakina mentioned this pull request Jul 24, 2019

Handle shadow host with delegatesFocus=true in focusing steps (click focus + focus()) whatwg/html#4796

Merged

3 tasks

rniwa reviewed Aug 14, 2019

View reviewed changes

shadow-dom/focus/focus-method-delegatesFocus.html Outdated Show resolved Hide resolved

rniwa mentioned this pull request Aug 14, 2019

Add delegatesFocus init to attachShadow and flag to ShadowRoot whatwg/dom#768

Merged

rakina added a commit to rakina/wpt that referenced this pull request Aug 15, 2019

Remove delegatesFocus test

24aa432

Will ad the delegatesFocus test in the delegatesFocus PR instead (web-platform-tests#18035)

This was referenced Aug 15, 2019

HTML: Sequential focus navigation with shadow dom #17523

Merged

HTML: Add WPT for DocumentOrShadowRoot.activeElement #18447

Merged

rakina added 2 commits September 25, 2019 11:00

HTML: Handle shadow host with delegatesFocus=true in Element.focus()/…

808281b

…focusing steps

Update focus() test, add sequential focus navigation tests

0116422

rakina force-pushed the focusmethod branch from 391db46 to 0116422 Compare September 25, 2019 08:18

rakina changed the title ~~HTML: Handle shadow host with delegatesFocus=true in Element.focus()~~ HTML: Handle shadow host with delegatesFocus=true in Element.focus() and sequential focus navigation Sep 25, 2019

domenic reviewed Sep 26, 2019

View reviewed changes

domenic approved these changes Sep 26, 2019

View reviewed changes

Explicitly list order

8edc86f

Add test for click focus

95b0f05

rakina changed the title ~~HTML: Handle shadow host with delegatesFocus=true in Element.focus() and sequential focus navigation~~ HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation Sep 27, 2019

domenic reviewed Sep 27, 2019

View reviewed changes

tkent-google reviewed Sep 27, 2019

View reviewed changes

Add actual testdriver tests

e53c8e0

domenic reviewed Sep 27, 2019

View reviewed changes

shadow-dom/focus/click-focus-delegatesFocus-tabindex-varies.html Outdated Show resolved Hide resolved

use promise_test

7004d81

domenic approved these changes Sep 27, 2019

View reviewed changes

rniwa mentioned this pull request Oct 8, 2019

Focus delegation from shadow host should not behave like sequential focus navigation WICG/webcomponents#830

Closed

Add tabindex=1 test to focus()

d50f1a3

We should use flat-tree order instead of tabindex order, add test to ensure that.

rniwa reviewed Oct 11, 2019

View reviewed changes

rniwa requested changes Oct 11, 2019

View reviewed changes

rakina added 2 commits October 11, 2019 20:45

Update focus-method-delegatesFocus.html

a1a7eeb

Update click-focus-delegatesFocus-tabindex-varies.html

a8a89f2

rakina commented Oct 11, 2019

View reviewed changes

rniwa approved these changes Oct 12, 2019

View reviewed changes

domenic merged commit e2ee0f7 into web-platform-tests:master Oct 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation #18035

HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation #18035

rakina commented Jul 24, 2019

rakina commented Sep 25, 2019

domenic left a comment

domenic Sep 26, 2019

rakina Sep 26, 2019

domenic left a comment

rakina commented Sep 26, 2019

tkent-google commented Sep 26, 2019

rakina commented Sep 27, 2019

domenic commented Sep 27, 2019

rakina commented Sep 27, 2019

domenic Sep 27, 2019

rakina Sep 27, 2019

tkent-google Sep 27, 2019

rakina Sep 27, 2019

rniwa Oct 11, 2019

rakina Oct 11, 2019

rniwa Oct 11, 2019

rakina Oct 11, 2019

rniwa Oct 11, 2019

rakina Oct 11, 2019

rniwa Oct 11, 2019

rakina Oct 11, 2019

rakina left a comment

rakina Oct 11, 2019

rakina Oct 11, 2019

rakina Oct 11, 2019

rakina Oct 11, 2019

rniwa left a comment

HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation #18035

HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation #18035

Conversation

rakina commented Jul 24, 2019

rakina commented Sep 25, 2019

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

rakina commented Sep 26, 2019

tkent-google commented Sep 26, 2019

rakina commented Sep 27, 2019

domenic commented Sep 27, 2019

rakina commented Sep 27, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakina left a 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

rniwa left a comment

Choose a reason for hiding this comment