1476301 - [FIX] Focus trap into webcomponents with shadowDOM

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Xavier Mouton-Dubosc]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180716100102

Steps to reproduce:

- Go on https://dascritch.net/vrac/.projets/audiowc/index.html (NOTE : <import> polyfilled) or download https://github.com/dascritch/cpu-audio/releases/tag/5.pre1
- Use →tab to move focus in the page


Actual results:

Once the focus is into a shadowDOM of the webcomponent, the focus cannot go outside forward or backward


Expected results:

The focus must continue to go forward

Comment 1

[Xavier Mouton-Dubosc]

I find out a part of the answer : 

The shadowRoot of my WebComponent has “2 tabindex="0"” declarations in the <template> tag  :

<div class="interface" tabindex="0">
	<div class="principal">
		<img class="poster nosmall" src="" alt="" />
		<div class="pageerror">
		</div>
		<div class="pagemain">
			<a class="control" tabindex="0">

If I put different positive numbers, I have less problems, excepted that the focus became really erratic.

Afterwards ( https://github.com/dascritch/cpu-audio/tree/39e9e18697afbf18d3ef6b782b74cd130ad8fbb6 ), I added some code to support a <menu>, with tabindex="0" added via innerHTML :

line.innerHTML = `<a href="#${self.audiotag.id}&t=${cuepoint}" tabindex="0">
			<strong>${cue.text}</strong>
			<span>${cuetime}</span>
		</a>`;

Funnily, the webcomponent is not reacting the same way, as you can focus backwards via [shift]+[tab], but you're still trapped in.

I can try to add programmatically element.tabindex, but it means a lot of undesired complexity.

Chrome works as desired. May be it is a bug due to their webcomponents.js polyfill ? It means my code really needs a native <link rel="import"> support.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Any chance for a minimal testcase attached to this bug?

Comment 3

[Xavier Mouton-Dubosc]

Yes, I will try to create it

Comment 4

[Xavier Mouton-Dubosc]

Attachment: Minimal testcase about focus trap into a customElement

Here is the minimal testcase.

When you move your focus with the [tab] key (or [shift]+[tab] for backwards), once the focus enters into a webcomponent, it cannot evade it.

Comment 5

[Xavier Mouton-Dubosc]

Attachment: test-focus-webcomponent.html but with style

Adding style for a better visibility

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Great, thanks. I'll try to look at this tomorrow

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

ok, hmm, I think I've fixed that issue already once before. And I thought I had added a testcase :/

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: shadowdomw_inline_host_focus.diff

On could argue this is a bug in FrameTraversal, since it deals with inline elements differently.

The key point in the test is that host is not div, but span.


(there is another sequential bug in shadow DOM, so I may need to tweak this code still a bit.)

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8998675 [details] [diff] [review]
shadowdomw_inline_host_focus.diff

Review of attachment 8998675 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsFocusManager.cpp
@@ +3599,5 @@
>        nsIContent* currentContent = frame->GetContent();
>        nsIContent* oldTopLevelHost = currentTopLevelHost;
> +      if (oldTopLevelHost != currentContent) {
> +        nsIContent* topLevel = GetTopLevelHost(currentContent);
> +        currentTopLevelHost = topLevel;

Nit: no need for the temporary `topLevel`.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Attachment: shadowdomw_inline_host_focus_2.diff

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/96daed2c2310
try to ensure sequential focusing can leave shadow trees, r=mrbkap

Comment 12

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/96daed2c2310

Comment 13

[Xavier Mouton-Dubosc]

I just checked and.... keyboard focus is still trapped in a little more complex page.
What i have missed ? 
https://dascritch.net/vrac/.projets/audiowc/index.html

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Could you open a new bug, please.
The patch in this bug fixed the issue shown by the testcase.

Comment 15

[Xavier Mouton-Dubosc]

Yes, i will.

But i still don't understand where the problem lies, and so how to reproduce it.

Comment 16

[張俊芝(ZHANG Junzhi)]

I also encountered a complicated situations where shadow dom focusable elements cannot be tabbed to, and right now I also haven't found the virutal cause where the issue lies. Uploading the whole document and styles seems an overkill

Comment 17

[Xavier Mouton-Dubosc]

Sorry. No more focus problems here.