Fixes to delegatesFocus #7079

domenic · 2021-09-16T20:54:44Z

Make it use the shadow tree instead of the flat tree. Closes Should delegatesFocus use the flat tree? #7207.
Make it work with the unfocusing steps symmetrically to the focusing steps. Closes Unfocusing steps should account for delegatesFocus somehow #7070.

At least two implementers are interested (and none opposed):
- Gecko raised this issue and has implemented these changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1737020
- Chrome is willing to align
Tests are written and can be reviewed and commented upon at:
- [Gecko Bug 1730566] Make blur() works on shadow host with delegatesFocus web-platform-tests/wpt#30875
- [Gecko Bug 1737020] Update delegatesFocus to use its shadow tree rather than the flat tree. web-platform-tests/wpt#31349
Implementation bugs are filed:
- Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1265439
- Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1730566
- Safari: https://bugs.webkit.org/show_bug.cgi?id=232564

(See WHATWG Working Mode: Changes for more details.)

emilio

Other than the bit below, it looks good, thank you!

I think this should probably be before the inert check. It seems more consistent with https://html.spec.whatwg.org/#get-the-focusable-area (since an inert shadow host still delegates focus to a potentially non-inert child, afaict).

I also think we might want to move the <area> special-case before the inert case, for consistency as well, though that's another normative change (as unlikely to matter as it is). It'd be nice to put this step after the area special-case, for symmetry with "get the focusable area" (though I don't think it matters because neither <area> not <a> can be shadow hosts).

source

sefeng211 · 2021-09-17T15:14:21Z

(Chrome passes the test written)

source

* Make it use the shadow tree instead of the flat tree. Closes #7207. * Make it work with the unfocusing steps symmetrically to the focusing steps. Closes #7070.

domenic · 2021-10-28T19:38:59Z

@emilio, can I get a re-review? I update this PR to also include the flat tree changes.

emilio

Looks good to me, thanks!

domenic · 2021-10-28T21:31:51Z

OK, now I need a green checkmark, so @annevk? :)

annevk · 2021-11-01T11:44:21Z

source

+     <li><p>Return the first <span>focusable area</span> in <span>shadow-including tree
+     order</span> of who their <span data-x="DOM anchor">DOM anchors</span> are in <var>possible
+     focus delegates</var>, or null if <var>possible focus delegates</var> is empty.</p></li>


Nit: should this be "... tree order whose DOM anchor is in ..."?

Well tree order needs to apply to DOM anchor... but yeah the current phrasing is weird. Will fix.

I see the intent of the current phrasing now. It's "Return the first focusable area, in shadow-including tree order of who their DOM anchors are, in possible focus delegates."

This is just too confusing though. I will add an explicit sorting step so as to avoid the very long sentence with too many clauses that step on each other in confusing ways.

https://bugs.webkit.org/show_bug.cgi?id=232564 Reviewed by Darin Adler. Implemented the new behavior of focusDelegates and added the support for autofocus focus delegation. Also fixed a bug that an element with autofocus content attribute inside a shadow tree gets focus. See whatwg/html#7079 * LayoutTests/fast/shadow-dom/autofocus-in-shadow-tree-autofocus-host-expected.txt: Added. * LayoutTests/fast/shadow-dom/autofocus-in-shadow-tree-autofocus-host.html: Added. * LayoutTests/fast/shadow-dom/autofocus-in-shadow-tree-expected.txt: Added. * LayoutTests/fast/shadow-dom/autofocus-in-shadow-tree.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/selectors/focus-visible-020.html: Merge the test fix posted in web-platform-tests/wpt#35257. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/focus/focus-autofocus-expected.txt: Rebaselined. * LayoutTests/imported/w3c/web-platform-tests/shadow-dom/focus/focus-method-delegatesFocus-expected.txt: Ditto. * Source/WebCore/dom/Element.cpp: (WebCore::shouldAutofocus): Fixed the bug that this wasn't ignoring elements inside shadow trees. Autofocus attribute yields autofocus candidates if the relevant element is **inserted** into a document, not connected to it. See https://html.spec.whatwg.org/multipage/interaction.html#the-autofocus-attribute (WebCore::autoFocusDelegate): Added. This implements autofocus delegation mechanism for shadow root/host. (WebCore::focusDelegateFromShadowHost): Renamed from findFirstProgramaticallyFocusableElementInComposedTree and made it recursively traverse shadow trees instead of the composed/flat tree. (WebCore::Element::focus): Canonical link: https://commits.webkit.org/252901@main

This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Bug: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d

Fixes #8156. Fixes #8157. See also previous work in #7361, #7284, and #7079.

This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Bug: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d

This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Fixed: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d

This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 The gist of the changes are: 1. Use the DOM tree instead of the flat tree to search for an element to give initial focus. 2. Don't traverse into shadow roots when looking for elements to give initial focus unless the shadow root has delegatesFocus. This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Fixed: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d

This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 The gist of the changes are: 1. Use the DOM tree instead of the flat tree to search for an element to give initial focus. 2. Don't traverse into shadow roots when looking for elements to give initial focus unless the shadow root has delegatesFocus. This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Fixed: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067796}

This reverts commit afeccd1f5b2dd72595abe55bc9ef093f11b495f1. Reason for revert: dialog-focus-shadow.html is failing on a couple of mac builders: https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.13%20Tests/57992/overview https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.14%20Tests/28597/overview Original change's description: > Implement new dialog shadow focus behavior > > This implements the new dialog initial focus behavior specified in these > changes: > whatwg/html#7079 > whatwg/html#7284 > whatwg/html#7361 > whatwg/html#8174 > > The gist of the changes are: > 1. Use the DOM tree instead of the flat tree to search for an element to > give initial focus. > 2. Don't traverse into shadow roots when looking for elements to give > initial focus unless the shadow root has delegatesFocus. > > This will stay experimental until I have also made the other changes for > dialog initial focus: whatwg/html#8199 > After those changes have been made as well, I will carefully enable the > flag by default. > > Fixed: 383230, 670130, 1292852 > Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Joey Arhar <jarhar@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1067796} Change-Id: I010be9a77fd8c289edb8c028355182d0fe7dec39

This reverts commit afeccd1. Reason for revert: dialog-focus-shadow.html is failing on a couple of mac builders: https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.13%20Tests/57992/overview https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.14%20Tests/28597/overview Original change's description: > Implement new dialog shadow focus behavior > > This implements the new dialog initial focus behavior specified in these > changes: > whatwg/html#7079 > whatwg/html#7284 > whatwg/html#7361 > whatwg/html#8174 > > The gist of the changes are: > 1. Use the DOM tree instead of the flat tree to search for an element to > give initial focus. > 2. Don't traverse into shadow roots when looking for elements to give > initial focus unless the shadow root has delegatesFocus. > > This will stay experimental until I have also made the other changes for > dialog initial focus: whatwg/html#8199 > After those changes have been made as well, I will carefully enable the > flag by default. > > Fixed: 383230, 670130, 1292852 > Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Joey Arhar <jarhar@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1067796} Change-Id: I010be9a77fd8c289edb8c028355182d0fe7dec39 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4005547 Commit-Queue: Tim Sergeant <tsergeant@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Tim Sergeant <tsergeant@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Tim Sergeant <tsergeant@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067950}

This reverts commit afeccd1f5b2dd72595abe55bc9ef093f11b495f1. Reason for revert: dialog-focus-shadow.html is failing on a couple of mac builders: https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.13%20Tests/57992/overview https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.14%20Tests/28597/overview Original change's description: > Implement new dialog shadow focus behavior > > This implements the new dialog initial focus behavior specified in these > changes: > whatwg/html#7079 > whatwg/html#7284 > whatwg/html#7361 > whatwg/html#8174 > > The gist of the changes are: > 1. Use the DOM tree instead of the flat tree to search for an element to > give initial focus. > 2. Don't traverse into shadow roots when looking for elements to give > initial focus unless the shadow root has delegatesFocus. > > This will stay experimental until I have also made the other changes for > dialog initial focus: whatwg/html#8199 > After those changes have been made as well, I will carefully enable the > flag by default. > > Fixed: 383230, 670130, 1292852 > Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Joey Arhar <jarhar@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1067796} Change-Id: I010be9a77fd8c289edb8c028355182d0fe7dec39 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4005547 Commit-Queue: Tim Sergeant <tsergeant@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Tim Sergeant <tsergeant@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Tim Sergeant <tsergeant@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067950}

This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 The gist of the changes are: 1. Use the DOM tree instead of the flat tree to search for an element to give initial focus. 2. Don't traverse into shadow roots when looking for elements to give initial focus unless the shadow root has delegatesFocus. This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Fixed: 383230, 670130, 1292852 Change-Id: If024ab0f22716c55b439e44b2298e39897ee844e

…or, a=testonly Automatic update from web-platform-tests Implement new dialog shadow focus behavior This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 The gist of the changes are: 1. Use the DOM tree instead of the flat tree to search for an element to give initial focus. 2. Don't traverse into shadow roots when looking for elements to give initial focus unless the shadow root has delegatesFocus. This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Fixed: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067796} -- wpt-commits: e1d2b674fbcc1ca01eccb624fdd758c38f3407a7 wpt-pr: 36498

…s behavior", a=testonly Automatic update from web-platform-tests Revert "Implement new dialog shadow focus behavior" This reverts commit afeccd1f5b2dd72595abe55bc9ef093f11b495f1. Reason for revert: dialog-focus-shadow.html is failing on a couple of mac builders: https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.13%20Tests/57992/overview https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.14%20Tests/28597/overview Original change's description: > Implement new dialog shadow focus behavior > > This implements the new dialog initial focus behavior specified in these > changes: > whatwg/html#7079 > whatwg/html#7284 > whatwg/html#7361 > whatwg/html#8174 > > The gist of the changes are: > 1. Use the DOM tree instead of the flat tree to search for an element to > give initial focus. > 2. Don't traverse into shadow roots when looking for elements to give > initial focus unless the shadow root has delegatesFocus. > > This will stay experimental until I have also made the other changes for > dialog initial focus: whatwg/html#8199 > After those changes have been made as well, I will carefully enable the > flag by default. > > Fixed: 383230, 670130, 1292852 > Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Joey Arhar <jarhar@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1067796} Change-Id: I010be9a77fd8c289edb8c028355182d0fe7dec39 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4005547 Commit-Queue: Tim Sergeant <tsergeant@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Tim Sergeant <tsergeant@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Tim Sergeant <tsergeant@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067950} -- wpt-commits: 011077556f5634fe685c613564c3325e77387462 wpt-pr: 36838

…or, a=testonly Automatic update from web-platform-tests Implement new dialog shadow focus behavior This implements the new dialog initial focus behavior specified in these changes: whatwg/html#7079 whatwg/html#7284 whatwg/html#7361 whatwg/html#8174 The gist of the changes are: 1. Use the DOM tree instead of the flat tree to search for an element to give initial focus. 2. Don't traverse into shadow roots when looking for elements to give initial focus unless the shadow root has delegatesFocus. This will stay experimental until I have also made the other changes for dialog initial focus: whatwg/html#8199 After those changes have been made as well, I will carefully enable the flag by default. Fixed: 383230, 670130, 1292852 Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067796} -- wpt-commits: e1d2b674fbcc1ca01eccb624fdd758c38f3407a7 wpt-pr: 36498

…s behavior", a=testonly Automatic update from web-platform-tests Revert "Implement new dialog shadow focus behavior" This reverts commit afeccd1f5b2dd72595abe55bc9ef093f11b495f1. Reason for revert: dialog-focus-shadow.html is failing on a couple of mac builders: https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.13%20Tests/57992/overview https://ci.chromium.org/ui/p/chromium/builders/ci/Mac10.14%20Tests/28597/overview Original change's description: > Implement new dialog shadow focus behavior > > This implements the new dialog initial focus behavior specified in these > changes: > whatwg/html#7079 > whatwg/html#7284 > whatwg/html#7361 > whatwg/html#8174 > > The gist of the changes are: > 1. Use the DOM tree instead of the flat tree to search for an element to > give initial focus. > 2. Don't traverse into shadow roots when looking for elements to give > initial focus unless the shadow root has delegatesFocus. > > This will stay experimental until I have also made the other changes for > dialog initial focus: whatwg/html#8199 > After those changes have been made as well, I will carefully enable the > flag by default. > > Fixed: 383230, 670130, 1292852 > Change-Id: I13995197f1942aa356cab0f3b41a0e226d1d170d > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3961528 > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Joey Arhar <jarhar@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1067796} Change-Id: I010be9a77fd8c289edb8c028355182d0fe7dec39 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4005547 Commit-Queue: Tim Sergeant <tsergeant@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Tim Sergeant <tsergeant@chromium.org> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Tim Sergeant <tsergeant@chromium.org> Cr-Commit-Position: refs/heads/main@{#1067950} -- wpt-commits: 011077556f5634fe685c613564c3325e77387462 wpt-pr: 36838

domenic added topic: shadow Relates to shadow trees (as defined in DOM) topic: focus labels Sep 16, 2021

domenic requested a review from emilio September 16, 2021 20:54

emilio suggested changes Sep 16, 2021

View reviewed changes

source Outdated Show resolved Hide resolved

emilio approved these changes Sep 18, 2021

View reviewed changes

annevk reviewed Sep 21, 2021

View reviewed changes

source Outdated Show resolved Hide resolved

domenic mentioned this pull request Oct 12, 2021

Should delegatesFocus use the flat tree? #7207

Closed

966563058015 mentioned this pull request Oct 13, 2021

See discussion in https://github.com/whatwg/html/pull/7079. Including the effects of that PR, focusing and unfocusing (e.g. via focus() and blur()) a delegatesFocus shadow host uses the flat tree: given github/site-policy#474

Closed

Fixes to delegatesFocus

8615199

* Make it use the shadow tree instead of the flat tree. Closes #7207. * Make it work with the unfocusing steps symmetrically to the focusing steps. Closes #7070.

domenic force-pushed the delegatesfocus-blur branch from 9c5c4af to 8615199 Compare October 28, 2021 19:35

domenic changed the title ~~Make unfocusing steps work well with delegatesFocus~~ Fixes to delegatesFocus Oct 28, 2021

emilio approved these changes Oct 28, 2021

View reviewed changes

annevk approved these changes Nov 1, 2021

View reviewed changes

Explicit sorting step

f2f171d

annevk approved these changes Nov 1, 2021

View reviewed changes

domenic merged commit 9ba7124 into main Nov 1, 2021

domenic deleted the delegatesfocus-blur branch November 1, 2021 15:51

domenic mentioned this pull request Nov 1, 2021

Also fix delegatesFocus and autofocus="" #7284

Merged

3 tasks

domenic mentioned this pull request Nov 23, 2021

Further fixes to focus delegation #7361

Merged

3 tasks

rniwa mentioned this pull request Jul 27, 2022

Update delegatesFocus to match recent spec changes WebKit/WebKit#2777

Merged

domenic mentioned this pull request Aug 9, 2022

Yet more fixes to focus delegation #8174

Merged

3 tasks

chromium-wpt-export-bot mentioned this pull request Oct 17, 2022

Implement new dialog shadow focus behavior web-platform-tests/wpt#36498

Merged

domenic added a commit that referenced this pull request Oct 22, 2022

Yet more fixes to focus delegation

4d6609f

Fixes #8156. Fixes #8157. See also previous work in #7361, #7284, and #7079.

domenic mentioned this pull request Nov 1, 2022

Don't change focus when re-focusing delegatesFocus shadowhost #8453

Merged

4 tasks

chromium-wpt-export-bot mentioned this pull request Nov 7, 2022

Revert "Implement new dialog shadow focus behavior" web-platform-tests/wpt#36838

Merged

chromium-wpt-export-bot mentioned this pull request Nov 7, 2022

Reland "Implement new dialog shadow focus behavior" web-platform-tests/wpt#36851

Merged

josepharhar mentioned this pull request Aug 26, 2023

Dialog initial focus when dialog contains a slot element #9640

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes to delegatesFocus #7079

Fixes to delegatesFocus #7079

domenic commented Sep 16, 2021 •

edited by pr-preview bot

Loading

emilio left a comment

sefeng211 commented Sep 17, 2021

domenic commented Oct 28, 2021

emilio left a comment

domenic commented Oct 28, 2021

annevk Nov 1, 2021

domenic Nov 1, 2021 •

edited

Loading

domenic Nov 1, 2021

Fixes to delegatesFocus #7079

Fixes to delegatesFocus #7079

Conversation

domenic commented Sep 16, 2021 • edited by pr-preview bot Loading

emilio left a comment

Choose a reason for hiding this comment

sefeng211 commented Sep 17, 2021

domenic commented Oct 28, 2021

emilio left a comment

Choose a reason for hiding this comment

domenic commented Oct 28, 2021

annevk Nov 1, 2021

Choose a reason for hiding this comment

domenic Nov 1, 2021 • edited Loading

Choose a reason for hiding this comment

domenic Nov 1, 2021

Choose a reason for hiding this comment

domenic commented Sep 16, 2021 •

edited by pr-preview bot

Loading

domenic Nov 1, 2021 •

edited

Loading