Issue metadata
Sign in to add a comment
|
Implement :focus-within pseudo-class from Selectors Level 4
Reported by
cvreb...@gmail.com,
Jun 4 2016
|
||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.79 Safari/537.36 Steps to reproduce the problem: 1. Open the attached testcase in Chrome. 2. Click/tap into the text <input> so as to focus it. What is the expected behavior? The <div> should turn green since the <input>, which is a descendant of the <div>, has become focused, thus causing the :focus pseudo-class to match the <input>, which should cause the <div> to match :focus-within, because it now has a descendant which matches :focus. What went wrong? The <div> remained white, indicating that the :focus-within pseudo-class in the webpage's CSS didn't start matching the <div>. Did this work before? No Chrome version: 51.0.2704.79 Channel: stable OS Version: OS X 10.11.5 Flash Version: Shockwave Flash 21.0 r0 Specification: https://drafts.csswg.org/selectors-4/#the-focus-within-pseudo
,
Jun 6 2016
The version of Selectors Level 4 that focus-within appears in is still an editor's draft.
,
Jun 30 2016
FYI WebKit already have this https://bugs.webkit.org/show_bug.cgi?id=140144
,
Sep 26 2016
Firefox 52 has implemented this: https://bugzilla.mozilla.org/show_bug.cgi?id=1176997
,
Jan 25 2017
,
Feb 10 2017
Per http://caniuse.com/#feat=css-focus-within this will ship in Firefox 52 and Safari 10.1.
,
Feb 12 2017
,
Mar 24 2017
I'll give it a try. Intent thread at: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg
,
Mar 24 2017
,
Mar 27 2017
Since this is a high-level tracking bug, adding the label Objective. Per Blink>CSS bug protocols, for clear responsibility, any concrete work on this issue should take place on a dependent bug that is filed under one component only.
,
Mar 27 2017
Is the spec include anything aboout Shadow DOM? How Safari 10.1 treats shadow tree?
,
Mar 27 2017
The spec explicitly says (https://drafts.csswg.org/selectors-4/#the-focus-within-pseudo): "An element also matches :focus-within if one of its shadow-including descendants matches :focus." This is pretty similar to the text for for :active or :hover pseudo-classes. And about "shadow-including descendants" the spec says (https://dom.spec.whatwg.org/#concept-shadow-including-descendant): "An object A is a shadow-including descendant of an object B, if A is a descendant of B, or A’s root is a shadow root and A’s root’s host is a shadow-including inclusive descendant of B." In Safari they have a test like this: https://trac.webkit.org/browser/webkit/trunk/LayoutTests/fast/css/pseudo-focus-within-inside-shadow-dom.html Which adds a green border to all the elements on the page including <body> and <div id="root">. Also the W3C suite has 5 tests testing Shadow DOM too: https://github.com/w3c/csswg-test/tree/master/selectors-4
,
Mar 27 2017
rego@ thanks for the checking! I've tried Safari TP25 myself, and it passed my casual tests of using :focus-within. The behavior about Shadow DOM looks reasonable and the specs sound Shadow DOM-ready. BTW the csswg tests use Shadow V0 API - I'll find time to convert them to V1 APIs :)
,
Mar 27 2017
@kochi, yeah I realized about that. We should change it to V1, I can do it too. But note that the csswg-test is merging into wpt tomorrow, so it's probably better to wait a few days. :-)
,
Mar 27 2017
rego@ it's good to know, and feel free to take it :) BTW, the screenshot in comment#13 was for what I was playing with on Safari TP25. http://jsbin.com/cokigavero/edit?html,css,output (without <slot>) http://jsbin.com/tasoxomivu/edit?html,css,output (with <slot>) Notice the difference that the latter <input> gets green border, as it matches the :focus-within in the document stylesheet. In the former <input> didn't get green border, even though it is focused, because it is inside a shadow tree and the style rule in the document tree didn't reach there.
,
Mar 27 2017
,
Apr 4 2017
The intent thread still lacks 1 LGTM, but an initial patch is ready at: https://codereview.chromium.org/2795143004/
,
Apr 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ffc655c99fef91f49ad7906344d51678c3853b74 commit ffc655c99fef91f49ad7906344d51678c3853b74 Author: rego <rego@igalia.com> Date: Wed Apr 12 11:31:29 2017 [selectors4] Implement :focus-within pseudo-class This patch adds support for the new ":focus-within" pseudo-class. Most of the patch is basically the regular boilerplate code to add a new selector. Then the interesting changes happen on ContainerNode. The patch is covered by a bunch of tests imported from the W3C test suite (which includes generic tests in addition to the tests from Firefox and WebKit implementations too). Apart from some minor changes on current tests to add the new selector. Intent-to-ship thread is available at: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg BUG= 617371 Review-Url: https://codereview.chromium.org/2795143004 Cr-Commit-Position: refs/heads/master@{#463992} [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/fast/css/css-selector-text-expected.txt [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/fast/css/css-selector-text.html [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/fast/css/css-set-selector-text-expected.txt [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/fast/css/css-set-selector-text.html [add] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html [add] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/LayoutTests/fast/selectors/focus-within-window-inactive.html [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/build/scripts/make_computed_style_base.py [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/css/CSSSelector.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/css/CSSSelector.h [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/css/RuleFeature.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/css/SelectorChecker.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/css/resolver/SharedStyleFinderTest.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/ContainerNode.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/ContainerNode.h [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/Node.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/Node.h [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/StyleChangeReason.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/StyleChangeReason.h [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/UserActionElementSet.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/dom/UserActionElementSet.h [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.cpp [modify] https://crrev.com/ffc655c99fef91f49ad7906344d51678c3853b74/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5
,
Apr 12 2017
Initial support for :focus-within has just landed behind a runtime flag. The next steps would be (as described in the CL): * Fix the test on WPT: https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-within-009.html * Investigate the common ancestor approach deeply (the same we currently use for :hover).
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a51a79c74201875de7299e5daf61e0c297d64647 commit a51a79c74201875de7299e5daf61e0c297d64647 Author: rego <rego@igalia.com> Date: Mon Apr 17 17:34:37 2017 [selectors4] Remove :focus-within test from TestExpectations The test is passing since r463992 but was marked as Failure in TestExpectations file. BUG= 617371 Review-Url: https://codereview.chromium.org/2820063002 Cr-Commit-Position: refs/heads/master@{#464944} [modify] https://crrev.com/a51a79c74201875de7299e5daf61e0c297d64647/third_party/WebKit/LayoutTests/TestExpectations
,
Apr 21 2017
,
Apr 21 2017
,
Apr 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2 commit 8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2 Author: rego <rego@igalia.com> Date: Tue Apr 25 12:06:44 2017 [selectors4] Use common ancestor strategy for :focus-within Following the same approach than what we do for :active and :hover pseudo-classes, it seems a nice optimization to use the common ancestor strategy for :focus-within too. The use case would be for example that you've a from with several inputs in a deep part of the DOM, and in the body element you've :focus-within to show some warning while editing the form. When you switch focus from one input to the next one, we don't need to invalidate the whole chain up to the body, but it'd be enough to just do it up to the common ancestor of the old and new focused elements (in this example the form). To achieve that Document::SetFocusedElement() has been modified to look for the common ancestor and then pass it to ContainerNode::SetHasFocusWithinUpToAncestor(). This new helper method is also used from FocusController to set :focus-within flag when the window loses/gains focus. A new unit test has been added to AffectedByFocusTest.cpp verifying that we only recalculate styles up to the common ancestor. BUG= 617371 TEST=fast/selectors/focus-within-window-inactive.html Review-Url: https://codereview.chromium.org/2821303005 Cr-Commit-Position: refs/heads/master@{#466949} [modify] https://crrev.com/8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2/third_party/WebKit/Source/core/css/AffectedByFocusTest.cpp [modify] https://crrev.com/8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2/third_party/WebKit/Source/core/dom/ContainerNode.cpp [modify] https://crrev.com/8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2/third_party/WebKit/Source/core/dom/ContainerNode.h [modify] https://crrev.com/8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2/third_party/WebKit/Source/core/dom/Document.cpp [modify] https://crrev.com/8789e1ce7cc6848e0cb4ba4bb546828df4fe7ca2/third_party/WebKit/Source/core/page/FocusController.cpp
,
Apr 25 2017
Common ancestor patch has landed, and I've added a new WPT test in: https://github.com/w3c/web-platform-tests/pull/5685 (still pending to review). @rune do you think we're still missing something else? Or could we mark the Runtime Flag as stable for shipping this in M60? Thanks for your feedback.
,
Apr 25 2017
IIUC, the test is about clearing :focus on elements becoming display:none, so it's not really directly a question about :focus-within as :focus-within is just kept in sync with :focus, right?
,
Apr 26 2017
> IIUC, the test is about clearing :focus on elements becoming display:none, so it's not really directly a question about :focus-within as :focus-within is just kept in sync with :focus, right? Yeah that's true, I've splitted it into 2 different tests. Anyway that should be reviewed and merged first on WPT and then they'll be imported into Blink. @rune my question about enabling the runtime flag still stands. Are you missing anything else regarding :focus-within before we ship the feature? Thanks.
,
Apr 26 2017
Nope, I think you can ship.
,
Apr 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7638b8e164ca3abae4522f98a82bdb1c44b4b4cc commit 7638b8e164ca3abae4522f98a82bdb1c44b4b4cc Author: rego <rego@igalia.com> Date: Thu Apr 27 12:21:35 2017 [selectors4] Ship ":focus-within" pseudo-class Intent-to-ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg BUG= 617371 Review-Url: https://codereview.chromium.org/2840183002 Cr-Commit-Position: refs/heads/master@{#467646} [modify] https://crrev.com/7638b8e164ca3abae4522f98a82bdb1c44b4b4cc/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5
,
Apr 27 2017
,
May 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/004ddc7b1a8b2c907f31179020ed17186b6639eb commit 004ddc7b1a8b2c907f31179020ed17186b6639eb Author: rego <rego@igalia.com> Date: Tue May 02 11:40:55 2017 [selectors4] :focus-within test when iframe loses focus This test should be part of https://codereview.chromium.org/2821303005 but at that time I didn't find the way to focus/unfocus an iframe. The solution was to use internals.setFocused() for that purpose. This patch adds a new test pretty similar to the one for windows (fast/selectors/focus-within-window-inactive.html), but checking what happens when an iframe loses and regains focus. BUG= 617371 TEST=fast/selectors/focus-within-iframe.html Review-Url: https://codereview.chromium.org/2847803007 Cr-Commit-Position: refs/heads/master@{#468601} [add] https://crrev.com/004ddc7b1a8b2c907f31179020ed17186b6639eb/third_party/WebKit/LayoutTests/fast/selectors/focus-within-iframe.html
,
May 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/29a9ed148cf67932faa0868735ab9a405ad81c97 commit 29a9ed148cf67932faa0868735ab9a405ad81c97 Author: rego <rego@igalia.com> Date: Fri May 05 07:12:13 2017 [selectors4] Remove :focus-within test that is now duplicated This patch removes the following test: fast/selectors/focus-within-display-none.html The test was upstreamed into the WPT repository and it's imported as: external/wpt/css/selectors4/focus-within-display-none-001.html The new test includes the old checks and some more, so we don't need our own test anymore. BUG= 617371 Review-Url: https://codereview.chromium.org/2862013002 Cr-Commit-Position: refs/heads/master@{#469610} [modify] https://crrev.com/29a9ed148cf67932faa0868735ab9a405ad81c97/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [delete] https://crrev.com/be16e0bd842e5f7c1d62211f95e04ba41d2c3e11/third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html
,
Jul 24 2017
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f2c4d52d5ac4acec983b370ef4258b9fb57b915e commit f2c4d52d5ac4acec983b370ef4258b9fb57b915e Author: ericwilligers <ericwilligers@chromium.org> Date: Wed Sep 13 12:25:28 2017 CSS: Retire flag CSSSelectorsFocusWithin The :focus-within pseudo-class shipped in Chrome 60. The runtime flag is no longer needed. BUG= 617371 Change-Id: I12db298d40df458c24987e7928d85aea179e6046 Reviewed-on: https://chromium-review.googlesource.com/662984 Reviewed-by: Manuel Rego Casasnovas <rego@igalia.com> Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/master@{#501599} [modify] https://crrev.com/f2c4d52d5ac4acec983b370ef4258b9fb57b915e/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp [modify] https://crrev.com/f2c4d52d5ac4acec983b370ef4258b9fb57b915e/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5
,
Sep 29 2017
,
Sep 29 2017
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by tkent@chromium.org
, Jun 4 2016Labels: -Pri-2 -OS-Mac OS-All Pri-3
Status: Untriaged (was: Unconfirmed)