Add delegatesFocus init to attachShadow and flag to ShadowRoot

[rakina]

Fixes #367 (The DOM part of whatwg/html#2013)

This adds delegatesFocus to ShadowRootInit, used in attachShadow.
Also adds a "delegates focus flag" on shadow roots, that later can be referenced by the focus algorithms in the HTML specs.

Related PRs:

• :focus selector behavior with delegatesFocus Add shadow host consideration for :focus selector html#4731
• Sequential focus navigation Update sequential focus navigation to include shadow trees html#4735

---

Preview | Diff

[rniwa]

Looks sane to me. This only defines a hook for the HTML specification to use it?

[rakina]

Looks sane to me. This only defines a hook for the HTML specification to use it?

Yep, since the focus algorithms are defined there. I'm currently working on the HTML changes.

[rakina]

Thanks for the approval! (also can someone with merge access merge this?)

[domenic]

As the editor, @annevk is the one that should merge; I'm mostly doing a drive-by approval. We may also want to delay merging until the corresponding HTML-side change is ready, to ensure they integrate well.

[rakina]

As the editor, @annevk is the one that should merge; I'm mostly doing a drive-by approval. We may also want to delay merging until the corresponding HTML-side change is ready, to ensure they integrate well.

I see. Will link to the HTML spec PRs here when it's ready then. Thanks!

[annevk]

Since this is a new field, should we use a boolean instead (basically, remove the word flag and instead of set/unset we use set to false/true)? And yeah, it'd be great to land this together with the HTML change and tests (if any new tests are needed).

[annevk]

Another thing I realized is that we could clean this up further by having a default value in the dictionary. I suspect we might need to leave the default value outside the dictionary as well for shadow roots that are not allocated via the method (i.e., user agent shadow roots) though? Either way, if the dictionary has a default value the constructor step can simply assign that value to the internal slot.

[rakina]

Another thing I realized is that we could clean this up further by having a default value in the dictionary. I suspect we might need to leave the default value outside the dictionary as well for shadow roots that are not allocated via the method (i.e., user agent shadow roots) though? Either way, if the dictionary has a default value the constructor step can simply assign that value to the internal slot.

Yeah you're right! Changed :)

[annevk]

Thanks! I'll leave this open per earlier discussion.

[domenic]

@annevk I think we have enough HTML spec changes that are ready to merge (whatwg/html#4731 and whatwg/html#4735 almost) that we should merge this. Any objections?

We'll hold off on filing implementer bugs for delegatesFocus until all related PRs merge.

[rniwa]

I don't think it makes senes to merge these changes unless there is some progress being made in at least one more implementation.

[domenic]

@rniwa, per the working mode, the requirement is not two implementations, but two implementations that support the feature. Among other benefits, this allows people to implement off of a complete spec, instead of looking at multiple different branches and pull requests.

Previously you have expressed support for delegatesFocus, and I am pretty sure (but don't recall specific instances) that the same holds for Mozilla. Do you now feel that maybe WebKit might switch from supportive to opposed?

[rniwa]

@rniwa, per the working mode, the requirement is not two implementations, but two implementations that support the feature. Among other benefits, this allows people to implement off of a complete spec, instead of looking at multiple different branches and pull requests.

We do support the general use cases behind it. Given how complex all these changes are, I don't really understand what delegatesFocus truly does or what kind of code changes we'd end up having to make, or even the proposed change is implementable in WebKit. Given that, I don't think I can say we support the PR as is until the actual implementation effort starts happening in WebKit.

[domenic]

OK. Please let us know when you have time to review them; it would be much appreciated.

[rniwa]

What's the behavior of the focus ring for a shadow host with delegatesFocus? It's not drawn?

[rniwa]

Alright, I've left all the relevant comments after implementing what's being currently proposed & spec'ed in https://webkit.org/b/166484. The remaining issues / questions I have are:

• What happens to focus rings?
• WPT test in HTML: Handle shadow host with delegatesFocus=true in Element.focus(), click and sequential focus navigation web-platform-tests/wpt#18035 seems to have bugs.
• What does the first focusable area mean when focusing a shadow host with delegatesFocus set to true? Does it consider any elements that are focusable during sequential navigation? e.g. anchors vs. buttons.

[domenic]

@rakina did you see #768 (comment) ? I'll try to answer as best I can.

What happens to focus rings?

These seem to be largely user-agent defined, according to the CSSWG. https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo. Any changes would probably be done there, but I'm unsure what changes we would make, given how much leeway that document gives user agents. Perhaps we could add another bullet point to the non-normative suggestions?

@rakina can probably explain what Chrome does, perhaps as a starting point for such a non-normative suggestion.

WPT test in web-platform-tests/wpt#18035 seems to have bugs.

Looks like this got stalled a bit, I'll poke it.

What does the first focusable area mean when focusing a shadow host with delegatesFocus set to true? Does it consider any elements that are focusable during sequential navigation? e.g. anchors vs. buttons.

This is the subject of great discussion in whatwg/html#4796 (comment) and WICG/webcomponents#830 . Complicated :(. As I stated in the PR it might be best to merge with a big red box pointing to the open issue.

[rniwa]

These seem to be largely user-agent defined, according to the CSSWG. https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo. Any changes would probably be done there, but I'm unsure what changes we would make, given how much leeway that document gives user agents. Perhaps we could add another bullet point to the non-normative suggestions?

I think we need to at least decide what happens if UA can decide to make it obvious that a shadow host has the focus even if the focus had been delegated to its shadow tree's content. Or would we follow the normal convention so that the focus ring is only drawn in the inner most element with the actual focus?

What does the first focusable area mean when focusing a shadow host with delegatesFocus set to true? Does it consider any elements that are focusable during sequential navigation? e.g. anchors vs. buttons.

This is the subject of great discussion in whatwg/html#4796 (comment) and WICG/webcomponents#830 . Complicated :(. As I stated in the PR it might be best to merge with a big red box pointing to the open issue.

I don't think we should merge this or that PR until we figure out which element gets the focus delegated given that's this feature is all about. As things stand, I can't really implement / land this feature in WebKit because the behavior is not well defined.

[rakina]

Oops, I missed this thread. Thanks @domenic!

These seem to be largely user-agent defined, according to the CSSWG. https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo. Any changes would probably be done there, but I'm unsure what changes we would make, given how much leeway that document gives user agents. Perhaps we could add another bullet point to the non-normative suggestions?

The default focus ring is drawn for all the activeElement in every shadow tree/document. If a shadow host that delegates focus has a focused element within its shadow tree, the focus ring is drawn for both the focused element in the shadow tree and the host.

I'm not really sure I understand :focus-visible, but I think in Blink it's like the above with the added requirement of being focused sequentially or through focus() (in other words, not through click), or if it's editable/can have user keyboard input/etc.

Looks like this got stalled a bit, I'll poke it.

I'll change shadow-utils a bit directly on Blink and update the tests once we figure out what's the desired behavior for focus delegation is.

[rniwa]

The default focus ring is drawn for all the activeElement in every shadow tree/document. If a shadow host that delegates focus has a focused element within its shadow tree, the focus ring is drawn for both the focused element in the shadow tree and the host.

It seems like that should be spec'ed somewhere then. It doesn't look like the current CSS selectors spec allows that for now.

[domenic]

It's a little unclear to me why speccing the focus ring is required. It is not web-observable (right?). I thought it was just part of how browsers present their UI.

[rniwa]

It's a little unclear to me why speccing the focus ring is required. It is not web-observable (right?). I thought it was just part of how browsers present their UI.

I mean, it's definitely not a blocker but we should update the spec because while it doesn't specify when UA MUST show a focus ring but it does specify it MUST NOT implicitly.

[rakina]

It seems like that should be spec'ed somewhere then. It doesn't look like the current CSS selectors spec allows that for now.

Oh sorry I was mistaken in my post earlier: we don't draw the focus rings for every activeElement. Instead I think we draw the focus ring for everything that matches the :focus selector (so hosts with delegatesFocus gets the focus ring if there's anything focused in its shadow scope), which is already updated in whatwg/html#4731. Is there any other place that needs an update?

[rniwa]

Instead I think we draw the focus ring for everything that matches the :focus selector (so hosts with delegatesFocus gets the focus ring if there's anything focused in its shadow scope), which is already updated in whatwg/html#4731.

Ah, okay. The focus ring issue is taken care of then. It might be worth adding a non-normative note about that.

[domenic]

Given the consensus we reached at TPAC, let's start merging things as they become ready. This one is the foundation of all the others, and is solid, so I'll start here!