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

[selectors] :focus-visible and Shadow DOM #5893

Open
mrego opened this issue Jan 25, 2021 · 19 comments
Open

[selectors] :focus-visible and Shadow DOM #5893

mrego opened this issue Jan 25, 2021 · 19 comments
Labels
selectors-4 Current Work

Comments

@mrego
Copy link
Member

mrego commented Jan 25, 2021

The spec doesn't mention anything of what happen on a ShadowRoot is the root of an element that is focused. The ShadowRoot matches :focus (see HTML spec) but I believe it shouldn't match :focus-visible or you'll get 2 outlines.

WebKit has implemented a :-webkit-direct-focus internal pseudo-class to avoid this issue (see bug #202432)

Example:

<div id="host" style="width: 200px; height: 100px; padding: 1em;"></div>
<script>
  const shadowRoot = host.attachShadow({mode: 'open', delegatesFocus: true});
  shadowRoot.innerHTML = '<input autofocus value="Focus me">';
</script>

Here if you focus the <input> you'll get 2 outlines right now in Chromium (one in the host and another in the <input>, both match :focus which is right, and also :focus-visible). That won't happen in WebKit thanks to that special UA stylesheet rule. In Firefox it looks like there are other problems because the host doesn't match :focus.

So the question is, should the :focus-visible spec include some text about this and describe WebKit's behavior?

CC @alice @emilio

@alice
Copy link

alice commented Feb 1, 2021

Can you expand on why the host element is matching :focus-visible?

@astearns astearns added this to the VF2F-2021-02-09 EUR milestone Feb 2, 2021
@emilio
Copy link
Collaborator

emilio commented Feb 2, 2021

So this behavior was part of all the delegatesFocus stuff (which I haven't followed that closely). Firefox doesn't implement it yet.

The general idea behind this behavior, IIRC, is that it's a bit weird that .activeElement does retarget but you can still observe the shadow state by checking :focus.

But if you use an internal pseudo-class and change the computed style as a result, doesn't it defeat the purpose?

I might be misremembering some other more compelling reason for this behavior?

cc @annevk @rniwa

@alice
Copy link

alice commented Feb 2, 2021

That seems like an explanation for why it matches :focus, but not why it would match :focus-visible.

@mrego
Copy link
Member Author

mrego commented Feb 2, 2021

I don't know why it matches :focus-visible, but it's matching it in Chromium implementation.

Checking :focus-visible spec there's not any mention to this specifically. The ShadowRoot matches :focus so it looks like Chromium determines that it also matches :focus-visible. From :focus-visible spec the only part related to that could be:

The :focus-visible pseudo-class applies while an element matches the :focus pseudo-class and the user agent determines via heuristics that the focus should be made evident on the element.

I was wondering if the list of suggestions we could include something specifically about this, mentioning that if ShadowRoot matches :focus because some of their descendants match it, it shouldn't match :focus-visible. But maybe you believe this is not needed and every engine can decide what to do here.

@annevk
Copy link
Member

annevk commented Feb 2, 2021

Related HTML issues:

@rniwa next time you add new UA style sheet rules please file an issue against whatwg/html 😊

@alice
Copy link

alice commented Feb 2, 2021

Is there any text anywhere about the intent of delegatesFocus?

@mrego
Copy link
Member Author

mrego commented Feb 2, 2021

I don't know a lot about this topic, but I have found this Chromium bug (https://crbug.com/1014091) which mentions

Recently the spec for :focus has been updated, now if an element in a shadow tree is focused, the :focus selector should match on the host too, regardless of delegatesFocus. Currently in Blink it only matches when delegatesFocus is true, I think.

And it links to whatwg/html#4731.

delegatesFocus seems to be in the DOM spec, but I'm not really sure if I get the definition there: https://dom.spec.whatwg.org/#shadowroot-delegates-focus

@alice
Copy link

alice commented Feb 2, 2021

Thank you for digging! I had found the same lack of definition in the DOM spec, but the whatwg bug might provide more clues.

@mrego
Copy link
Member Author

mrego commented Feb 3, 2021

Also found a mention to delegatesFocus in the HTML spec: https://html.spec.whatwg.org/#focus-processing-model

@annevk
Copy link
Member

annevk commented Feb 3, 2021

Yeah, DOM just defines the member, HTML defines behavior.

@rniwa
Copy link

rniwa commented Feb 4, 2021

FWIW, it would be problematic for :focus-visible to become a way of detecting whether there is a shadow root or not.

@mrego
Copy link
Member Author

mrego commented Feb 5, 2021

FWIW, it would be problematic for :focus-visible to become a way of detecting whether there is a shadow root or not.

Do you mean that you could know or not if an element is a shadow root by focusing it via script (host.focus()) and then checking it matches :focus but not :focus-visible?

I believe in WebKit you can do something similar already, as elements with :focus have an outline, you could do host.focus() and then check that host actually matches :focus but the outline-style is none. It would be a way to detect the same thing form a script somehow (e.g. https://cdpn.io/mrego/debug/WNorXNv/bZAQWKNVvEyM).

@alice
Copy link

alice commented Feb 7, 2021

In any case, by design :focus-visible is not guaranteed to match when an element is focused, so at best it would be a way to heuristically detect that something maybe-probably has a shadow root.

Compared with the user harm from authors disabling focus styling, which is the problem :focus-visible is intended to solve, detecting that something might have a shadow root seems like it's at the opposite extreme of the priority of constituencies.

@mrego
Copy link
Member Author

mrego commented Feb 17, 2021

BTW, one of the HTML issues linked before (whatwg/html#3748), ends up in a CSSWG issue #3248.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [selectors] :focus-visible and Shadow DOM.

The full IRC log of that discussion <dael> Topic: [selectors] :focus-visible and Shadow DOM
<dael> github: https://github.com//issues/5893
<dael> rego: Both are about focus-visiable. This is about what happens when you have shadow dom
<dael> rego: focus pseudo element matches when you have focus in a shadow tree so matches in shadow-host. Wondering if focus-visible should match.
<dael> rego: Special thing is WK has -wk-direct-focus pseudo call
<dael> rego: People use it in UA stylesheet to avoid outline when you focus in the shadow tree.
<Rossen_> q?
<dael> rego: In chromium in this case if you focus the input in the shadow tree you get the focus indicator. I believe focus-visual is about avoiding people removing the outline with focus outline:none
<dael> rego: Tricky part is in theory you shouldn't be able to know if an element is shadow host. Thanks to that you could check if it is a shadow host. There are other cases wher eyou can do that. shadow host only matches in some cases. Need to think about it
<dael> rego: with WK solution you can inspect if it's in shadow host because you can focus it and check if there's an outline shown.
<dael> rego: It's a trick.
<dael> florian: QUestion on indirect level. Asking if we should show focus-visible in this case, but focus-visible isn't defined when it shows. Ther'es heuristics, but it says it should match when focus indicator is shown. Are you trying to spec more detail on when browser shows?
<dael> rego: It would be adding one more item on the heuristic list to allow this case. One could be browsers don't show focus indicator in shadow host when there's an element in shadow tree with focus. It would be add a heuristic since current impl follow the heuristic. It's non-normative, but adding one more heurisitc
<Rossen_> q?
<dael> Rossen_: Back to shadow root topic. I was re-reading thread in issue. Appears topic is discussed but not solved.
<dael> Rossen_: If I hear you, you're saying there are other precedents which would allow shadow root detection. If this is a fact we shouldn't necessarily add things to make it easier and more obvious to detect
<dael> Rossen_: Where is the current thinking on that?
<dael> rego: When you focus an element you're not sure if it will match focus-visible. When you focus inside shadow tree if shadow host doesn't match maybe you have it for a reason because user did a different way to focus.
<emilio> q+
<dael> rego: There's another open issue on how that makes shadow host observable
<dael> Rossen_: Right. I think Alex's last comment summarizes it well. [reads]
<rego> s/Alex/Alice/
<Rossen_> q?
<dael> Rossen_: This doesn't seem to have the answer. I see your comemnt pointed to another issue. Seems up in the air
<dael> rego: I agree with Alice. I think Alice would be fine not matching focus-visible in shadow root. emilio might have something to say
<dael> emilio: I agree with Alice but for a different reason. Don't agree we're only probablistically exposing. Given spec heuristics you can force focus-visible to show. If we want focus-visible to be used it needs to not match shadow host or not match from UA rules. Don't know if we have precidence for UA rules
<rego> Chromium also implements the :focus propagation on delegateFocus:true, that's why you get 2 outlines in Chromium
<fantasai> https://drafts.csswg.org/selectors/#the-focus-visible-pseudo needs fixing to use RFC2119 terminology correctly :/
<Rossen_> ack emilio
<dael> emilio: WK is only one that impl and they have special pseudo class for UA stylesheet. I think either column-focus or focus-visible should do the right thing or they shouldn't match. Right is if you use from UA stylesheet it shouldn't show on shadow host ancestors
<dael> Rossen_: Catching up on IRC chat.
<dael> Rossen_: emilio, the overall take away is you agree with problem of exposing awareness of shadow host, though for a slightly dirrent reason?
<dael> emilio: Yeah. think it's solvable if we have special case to not prop. I don't know if we want to go that root
<dael> s/root/route
<dael> emilio: And we would still expose via gCS so I don't feel too strong. Either do what rego and Alice propose or making focus-visible do right thing from UA stylesheet are workable
<dael> Rossen_: What is the ask here, rego?
<dael> Rossen_: Asking for resolution with the proposed behavior you summerized in opening comment?
<dael> rego: One option could be resolve we don't want focus-visible to match shadow host. Maybe we should explore alternative from emilio in the issue
<dael> Rossen_: For me that would be preferred path. I don't feel like conversation with Alice is resolved. emilio you can expand your proposal further
<dael> emilio: Can do
<dael> fantasai: My understanding is definition of this pseudo class is it will match when UA is doing something to style a focusable element as focused and will not match otherwise. If UI default focus was a dotted outline around everything if the author replicated that they should get same behavior as UA would give. All heuristics after example are suggestions of things UA should think about when they match focus-visible in general
<dael> fantasai: Meaning any UA in any case...we can improve the heuristics but they're not normative. Normative is if you style as focus-visible you match as focus-visible. We can do wahtever we want with heuristics but they're informative. We can't put heuristics that won't match actual definition of most browsers.
<dael> fantasai: I'm not sure what we're trying to do here. What's required is clear and this is just helpful hints when designing a UA.
<dael> emilio: When you focus and element inside a shadow tree a lot of state in the page reflects that. focus pseudo class and others. Issues is, for regular elements like DOM APIs, app dev returns shadow host. THat doesn't change if real active element is there. Doens't change what it is. Issue here is should we do same with css pseudo classes
<dael> emilio: I agree it could go either way. But it doesn't change if inner element should match focus-visible. If we spec it should bubble up and target dom apis all browsers should do
<Rossen_> ack fantasai
<dael> florian: Then are we saying it should style around the shadow host?
<dael> emilio: In order for bubble up to work you need to match is a special in your sheets. UA sheets match in all shadow scopes.
<dael> fantasai: Point of focus-visible was for UA to not have magic pseudo class that does something different
<dael> emilio: Right, this should apply to all retargetting like column-focus. Not magic like how it's not magic if you target inside the tree
<dael> florian: Does UA draw focus line around shadow root
<dael> emilio: Should not
<dael> florian: Then focus-visible should not. It's what focus-visible is for
<rego> about the heuristics, despite they're not normative, all implementations are following them, so they help to ensure interop on :focus-visible implementations
<dael> emilio: Right, but if you think in terms of built in control like date. You have inner element you focus. date is little editable fields for day, month, year. Inner focused is editable inside shadow root. In that case you want focus-visible to bubble so you get outline around text
<dael> emilio: Maybe add a way for shadow host to opt in. by default we shouldn't match
<tantek> regrets+
<dael> Rossen_: Don't feel we're ready to resolve. I suggest back to the issue and bring it back when we're ready
<dael> rego: Agree
<dael> rego: We should keep discussion on issue
<tantek> regrets+ Morgan

@astearns astearns removed the Agenda+ label Feb 17, 2021
@emilio
Copy link
Collaborator

emilio commented Feb 17, 2021

For reference, what I mentioned during the discussion is that if we want to bubble :focus-visible to the host (which is unclear to me) we should make it so that UA stylesheets don't match them both inside and outside the shadow root.

I think probably just do the simple thing is better though.

tabatkins added a commit that referenced this issue Feb 18, 2021
@robdodson
Copy link
Contributor

Hey @emilio, regarding this last comment:

I think probably just do the simple thing is better though.

Which solution were you proposing?

@emilio
Copy link
Collaborator

emilio commented Mar 16, 2021

I think that meant "matching :focus-visible only on the actual element, not on its shadow host(s)".

@mrego
Copy link
Member Author

mrego commented Mar 16, 2021

My plan here is to write a test to check that behavior, and include some text related to that in a PR for HTML spec describing when the UA should show a focus ring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
selectors-4 Current Work
Projects
None yet
Development

No branches or pull requests

8 participants