What were you doing?

1. Tab through options in the new Profiler toolbar panel

What happened?

Tab and space don't work, keyboard a11y is broken.

What should have happened?

Keyboard navigation works (does in addon actually).

I tried to add iframe.focus() and iframe.contentWindow.focus() calls in [1] but this just produced errors on the console when tabbing.
I also tried to understand the focus handling in [2] but couldn't find something that would work.
I believe we're the first ones to use iframes in customizable UI [3], and that we may have a bigger problem to solve here.
The stuff going on with extensions look super complicated but maybe I'm not just used to the code. I think we'll need some help from people that know this more than us.

Hey Jamie, do you know who could help us around topics related to focus handling in the Firefox interface, especially around the toolbar buttons?

[1] https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/devtools/client/performance-new/popup/menu-button.jsm#93-105
[2] https://searchfox.org/mozilla-central/source/browser/components/customizableui/PanelMultiView.jsm
[3] https://searchfox.org/mozilla-central/rev/153172de0c5bfca31ef861bd8fc0995f44cada6a/browser/components/customizableui/content/panelUI.inc.xul#628-630

(In reply to Julien Wajsberg [:julienw] from comment #1)

I tried to add iframe.focus() and iframe.contentWindow.focus() calls in [1] but this just produced errors on the console when tabbing.

This is definitely one part of what's needed here. What errors were you seeing?

I also tried to understand the focus handling in [PanelMultiView] but couldn't find something that would work.
...
I believe we're the first ones to use iframes in customizable UI [3], and that we may have a bigger problem to solve here.

In bug 1545766 D28442, I fixed this for extension panels, but they use a <browser>, not an <iframe>. Fixing this should hopefully just be a matter of adding a check for "iframe" (as well as "browser") here:
https://searchfox.org/mozilla-central/rev/ee806041c6f76cc33aa3c9869107ca87cb3de371/browser/components/customizableui/PanelMultiView.jsm#1421
and here:
https://searchfox.org/mozilla-central/rev/ee806041c6f76cc33aa3c9869107ca87cb3de371/browser/components/customizableui/PanelMultiView.jsm#1589
Ideally, that would also have a similar test to the one added in D28442.

Ah thanks, adding the checks in these locations fixes the error that was displayed (related to setFocus having an illegal argument IIRC).

But this still doesn't focus the inner iframe.

After opening the window, when using the arrow keys, I can scroll down the popup, so the focus is somewhat right, but not completely right.
As you can see on the diff, I tried various ways to focus, from calling it directly to waiting for a load event in the contentWindow before focusing.
I also tried to set a tabindex on document.body to see if this could help.

My last try, which is working but I'm not satisfied about is:

• replace the iframe with a browser (this makes it a lot slower to display the first time too).
• add an unconditional return in keyNavigation.
• no need to add a manual focus.

Of course this is not the proper solution (esp because of the unconditional return), but a good lead to understand what could be changed.

For some reason, whenever I set focus to the iframe programmatically, this breaks the behaviour of the tab key, even with PanelMultiView key handling disabled. I don't yet understand why, nor can I reproduce it with a distilled test case. I'll dig into it further.

Bug 1545766 (D28442) tweaked PanelMultiView keyboard navigation to behave as expected for embedded browser elements.
This patch extends this to handle iframe elements such as used in the builtin Profiler panel.
In addition, it avoids setting tabindex="-1" on iframe and browser elements, since this breaks tabbing behavior in iframe elements (and possibly causes issues in browser elements as well).
iframe and browser elements are already focusable, so this isn't needed anyway.

Thanks for the fix Jamie !

https://hg.mozilla.org/mozilla-central/rev/33473b615fa7

Thanks for the fix James! Sorry I didn't realize this when I initially landed this code.