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

[selectors-4] About shadow-including descendants #1135

Closed
mrego opened this Issue Mar 28, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@mrego
Member

mrego commented Mar 28, 2017

For three pseudo-classes (:hover, :active and :focus-within) the spec has a similar text:

An element also matches :hover if one of its shadow-including descendants matches :hover.

The reference to shadow-including descendants was introduced back in April 2016 by @tabatkins.

However it seems it doesn't match the behavior in either Chrome and Safari.
Using an example from @rune-opera:

  <div id="host">
    <span>Hover me</span>
  </div>
  <script>
    host.attachShadow({mode:"open"}).innerHTML = "<style>#shadowDiv:hover { border: 5px solid green }</style><div id='shadowDiv'><slot></slot></div>";
  </script>

In both Blink and WebKit a green border appears on the shadowDiv when you hover the <span>. This seems the right behavior from the user point of view.
However, the <span> is not a shadow-including descendant of the shadowDiv. Shouldn't the spec mention the flat tree instead?

This discussion started on the blink-dev thread about :focus-within.

@dbaron dbaron added the selectors-4 label Mar 28, 2017

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 28, 2017

Note that the Blink implementation walks the layout tree for :hover/:active, not the flat tree, so there can be subtleties.

I've reported https://crbug.com/705984 for display:contents. Firefox also has an invalidation problem with :hover and display:contents https://bugzilla.mozilla.org/show_bug.cgi?id=1351339

ghost commented Mar 28, 2017

Note that the Blink implementation walks the layout tree for :hover/:active, not the flat tree, so there can be subtleties.

I've reported https://crbug.com/705984 for display:contents. Firefox also has an invalidation problem with :hover and display:contents https://bugzilla.mozilla.org/show_bug.cgi?id=1351339

mrego added a commit to mrego/wpt that referenced this issue Mar 30, 2017

[selectors4] Add one more test for :focus-within and Shadow DOM
This test is similar to the example in w3c/csswg-drafts#1135.
It checks that when the slotted element is focused,
":focus-within" pseudo-class affects to the ancentors in the flat tree.

mrego added a commit to mrego/wpt that referenced this issue Mar 30, 2017

[selectors4] Add one more test for :focus-within and Shadow DOM
This test is similar to the example in w3c/csswg-drafts#1135.
It checks that when the slotted element is focused,
":focus-within" pseudo-class affects to the ancestors in the flat tree.
@ghost

This comment has been minimized.

Show comment
Hide comment

ghost commented Mar 31, 2017

cc editors @tabatkins @fantasai

@tabatkins

This comment has been minimized.

Show comment
Hide comment
@tabatkins

tabatkins Mar 31, 2017

Member

Hm, I didn't realize that "shadow-including descendants" doesn't include that case. That's annoying. The intention is definitely that they run on the flat tree.

Member

tabatkins commented Mar 31, 2017

Hm, I didn't realize that "shadow-including descendants" doesn't include that case. That's annoying. The intention is definitely that they run on the flat tree.

@tabatkins

This comment has been minimized.

Show comment
Hide comment
@tabatkins

tabatkins Mar 31, 2017

Member

Suggested edit (for all of the relevant classes, and fixing #1141 too):

An element also matches '':hover''
if one of its descendants in the flat tree
(including non-element nodes, such as text nodes)
matches the above conditions.

Sound good?

Member

tabatkins commented Mar 31, 2017

Suggested edit (for all of the relevant classes, and fixing #1141 too):

An element also matches '':hover''
if one of its descendants in the flat tree
(including non-element nodes, such as text nodes)
matches the above conditions.

Sound good?

@mrego

This comment has been minimized.

Show comment
Hide comment
@mrego

mrego Apr 3, 2017

Member

Yeah, it sounds good. Thanks @tabatkins!

Member

mrego commented Apr 3, 2017

Yeah, it sounds good. Thanks @tabatkins!

frivoal added a commit to frivoal/csswg-drafts that referenced this issue Apr 6, 2017

[selectors-4] Make :active :hover and :focus-within work on the flat …
…tree

They were incorrectly defined as applying to shadow-including descendants,
while the intent was for them to work on flat tree descendants

Closes #1135

mrego added a commit to mrego/wpt that referenced this issue Apr 7, 2017

[selectors4] Add one more test for :focus-within and Shadow DOM
This test is similar to the example in w3c/csswg-drafts#1135.
It checks that when the slotted element is focused,
":focus-within" pseudo-class affects to the ancestors in the flat tree.
@mrego

This comment has been minimized.

Show comment
Hide comment
@mrego

mrego Apr 10, 2017

Member

So this has just been fixed by @tabatkins at 9d0024d

Thanks!

Member

mrego commented Apr 10, 2017

So this has just been fixed by @tabatkins at 9d0024d

Thanks!

@mrego mrego closed this Apr 10, 2017

hubot pushed a commit to WebKit/webkit that referenced this issue Apr 25, 2017

rego@igalia.com
[selectors4] :focus-within should use the flat tree
https://bugs.webkit.org/show_bug.cgi?id=170899

Reviewed by Antti Koivisto.

Source/WebCore:

This has been discussed in the following CSS WG issue:
w3c/csswg-drafts#1135

And the spec has been updated (https://drafts.csswg.org/selectors-4/#the-focus-within-pseudo):
"An element also matches :focus-within if one of its descendants in the flat tree
 (including non-element nodes, such as text nodes) matches the conditions for matching :focus."

Test: imported/w3c/web-platform-tests/css/selectors4/focus-within-shadow-006.html

* dom/Element.cpp:
(WebCore::Element::setFocus): Use "flat tree" ("composed tree" in WebKit)
to set focus-within flag.

LayoutTests:

* TestExpectations: Remove test that is passing now.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@215719 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment