Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What happens if the delegated element is also a shadow host with delegatesFocus set #5052

Closed
rniwa opened this issue Oct 28, 2019 · 7 comments · Fixed by web-platform-tests/wpt#20079

Comments

@rniwa
Copy link
Collaborator

rniwa commented Oct 28, 2019

It seems that the current logic in the focusing step doesn't take into the account the possibility that the first element in the flat tree UA picks might also be a shadow host with delegatesFocus set.

In such cases, don't we need to recursively resolve the delegated element? Neither the spec nor WebKit's implementation has this logic.

The following test case shows that Chrome's current implementation seems to delegate the focus recursively:

const outerHost = document.createElement('div');
const outerShadow = outerHost.attachShadow({mode: 'closed', delegatesFocus: true});

const innerHost = document.createElement('div');
const innerShadow = innerHost.attachShadow({mode: 'closed', delegatesFocus: true});
outerHost.tabIndex = 0;
innerHost.tabIndex = 0;
innerShadow.innerHTML = 'a <span tabindex=0>b</span> c';

document.body.appendChild(outerHost);
outerShadow.append('0 ', innerHost, ' 1');

outerHost.focus();
@rniwa
Copy link
Collaborator Author

rniwa commented Oct 28, 2019

@rakina
Copy link
Member

rakina commented Oct 29, 2019

Oh yeah you're right, nice catch. The chrome implementation is already recursive, I'll update the spec tonight.

@rakina
Copy link
Member

rakina commented Oct 29, 2019

Oh wait nope I think we're already handling it. Shadow hosts with delegatesFocus=true are explicitly can never be a focusable area (see table under https://html.spec.whatwg.org/multipage/interaction.html#focusable-area).

First in flat tree order will be the same, whether we delegate recursively or not, since flat tree order means we're already gathering everything recursively.

@rniwa
Copy link
Collaborator Author

rniwa commented Oct 30, 2019

Hm... that's a good point. I guess we can just add a WPT test for this.

@rakina
Copy link
Member

rakina commented Nov 5, 2019

Good idea. @tkent-google can you add a WPT either in Github or Gerrit as part of your focus impl work? (I'm out this week)

@tkent-google
Copy link
Collaborator

Ok, here it is: web-platform-tests/wpt#20079

@rniwa
Copy link
Collaborator Author

rniwa commented Nov 6, 2019

tkent-google added a commit to web-platform-tests/wpt that referenced this issue Nov 19, 2019
tkent-google added a commit to web-platform-tests/wpt that referenced this issue Nov 19, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 29, 2019
…nd nested shadow hosts., a=testonly

Automatic update from web-platform-tests
shadow-dom: Add test cases for focus() and nested shadow hosts. (#20079)

Closes whatwg/html#5052
--

wpt-commits: c1e7abe048f9baee56488c23971ee4af08336886
wpt-pr: 20079
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Nov 29, 2019
…nd nested shadow hosts., a=testonly

Automatic update from web-platform-tests
shadow-dom: Add test cases for focus() and nested shadow hosts. (#20079)

Closes whatwg/html#5052
--

wpt-commits: c1e7abe048f9baee56488c23971ee4af08336886
wpt-pr: 20079
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Nov 30, 2019
…nd nested shadow hosts., a=testonly

Automatic update from web-platform-tests
shadow-dom: Add test cases for focus() and nested shadow hosts. (#20079)

Closes whatwg/html#5052
--

wpt-commits: c1e7abe048f9baee56488c23971ee4af08336886
wpt-pr: 20079

UltraBlame original commit: 45b376ea5dbb855f9e7689f8066ce0c3382862fc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Nov 30, 2019
…nd nested shadow hosts., a=testonly

Automatic update from web-platform-tests
shadow-dom: Add test cases for focus() and nested shadow hosts. (#20079)

Closes whatwg/html#5052
--

wpt-commits: c1e7abe048f9baee56488c23971ee4af08336886
wpt-pr: 20079

UltraBlame original commit: 45b376ea5dbb855f9e7689f8066ce0c3382862fc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Nov 30, 2019
…nd nested shadow hosts., a=testonly

Automatic update from web-platform-tests
shadow-dom: Add test cases for focus() and nested shadow hosts. (#20079)

Closes whatwg/html#5052
--

wpt-commits: c1e7abe048f9baee56488c23971ee4af08336886
wpt-pr: 20079

UltraBlame original commit: 45b376ea5dbb855f9e7689f8066ce0c3382862fc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants