Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Starred by 16 users
Status: Fixed
Owner:
Closed: Apr 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature
Team-Accessibility

Blocked on:
issue 704902

Blocking:
issue 747818



Sign in to add a comment
Implement :focus-within pseudo-class from Selectors Level 4
Reported by cvreb...@gmail.com, Jun 4 2016 Back to list
UserAgent: 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
 
focus-within-test.html
455 bytes View Download
Comment 1 by tkent@chromium.org, Jun 4 2016
Components: Blink>Focus Blink>CSS
Labels: -Pri-2 -OS-Mac OS-All Pri-3
Status: Untriaged
Status: Available
The version of Selectors Level 4 that focus-within appears in is still an editor's draft.
Comment 3 by kochi@chromium.org, Jun 30 2016
Components: UI>Accessibility
FYI WebKit already have this https://bugs.webkit.org/show_bug.cgi?id=140144
Comment 4 by cvrebert@google.com, Sep 26 2016
Firefox 52 has implemented this: https://bugzilla.mozilla.org/show_bug.cgi?id=1176997
Comment 5 by tkent@chromium.org, Jan 25 2017
Labels: Needs-BlinkIntent
Per http://caniuse.com/#feat=css-focus-within this will ship in Firefox 52 and Safari 10.1.
Labels: Update-Quarterly
Owner: r...@igalia.com
Status: Assigned
I'll give it a try.

Intent thread at:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg
Blockedon: 704902
Labels: Objective
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.
Is the spec include anything aboout Shadow DOM?

How Safari 10.1 treats shadow tree?
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

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 :)
focuswithin_withoutslot.png
23.5 KB View Download
focuswithin_withslot.png
25.0 KB View Download
@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. :-)
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.
Labels: NewComponent-Accessibility-Blink NewComponent-Accessibility
Status: Started
The intent thread still lacks 1 LGTM, but an initial patch is ready at:
https://codereview.chromium.org/2795143004/
Project Member Comment 18 by bugdroid1@chromium.org, Apr 12
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

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).

Project Member Comment 20 by bugdroid1@chromium.org, Apr 17
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

Components: Blink>Accessibility
Components: -UI>Accessibility
Labels: -newcomponent-accessibility-blink -newcomponent-accessibility
Project Member Comment 23 by bugdroid1@chromium.org, Apr 25
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

Cc: r...@opera.com
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.
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?

> 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.
Nope, I think you can ship.
Status: Fixed
Project Member Comment 30 by bugdroid1@chromium.org, May 2
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

Project Member Comment 31 by bugdroid1@chromium.org, May 5
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

Blocking: 747818
Sign in to add a comment