Closed Bug 1476301 Opened 2 years ago Closed 2 years ago

[FIX] Focus trap into webcomponents with shadowDOM

Categories

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

63 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: xaviermd, Assigned: smaug)

References

Details

(Keywords: nightly-community)

Attachments

(3 files, 1 obsolete file)

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
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.
Component: Untriaged → DOM
Product: Firefox → Core
Priority: -- → P3
Any chance for a minimal testcase attached to this bug?
Flags: needinfo?(xaviermd)
Yes, I will try to create it
Flags: needinfo?(xaviermd)
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.
Adding style for a better visibility
Attachment #8998222 - Attachment is obsolete: true
Great, thanks. I'll try to look at this tomorrow
ok, hmm, I think I've fixed that issue already once before. And I thought I had added a testcase :/
Assignee: nobody → bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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.)
Attachment #8998675 - Flags: review?(mrbkap)
Summary: [WebComponents] Focus trap into webcomponents with shadowDOM → [FIX] Focus trap into webcomponents with shadowDOM
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`.
Attachment #8998675 - Flags: review?(mrbkap) → review+
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/96daed2c2310
try to ensure sequential focusing can leave shadow trees, r=mrbkap
https://hg.mozilla.org/mozilla-central/rev/96daed2c2310
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Could you open a new bug, please.
The patch in this bug fixed the issue shown by the testcase.
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Flags: needinfo?(xaviermd)
Resolution: --- → FIXED
Yes, i will.

But i still don't understand where the problem lies, and so how to reproduce it.
Flags: needinfo?(xaviermd)
Flags: needinfo?(xaviermd)
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
Component: DOM → DOM: Core & HTML

Sorry. No more focus problems here.

Flags: needinfo?(xaviermd)
You need to log in before you can comment on or make changes to this bug.