User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

Tabbing through a popup window's content does not cycle back to the browser controls and url after the end of the content is reached if window.open is used with the toolbar disabled (value of 0 instead of 1). I'm assuming the tabbing logic in the browser is expecting the tab to be visible, but that isn't always the case, so it fails to find it and prevents the full cycle from being completed. We noticed it when doing ADA testing for our products.

Actual results:

User has to shift + tab to get to earlier content.

Expected results:

User should have been able to tab through all content and then be placed back at the top.

It looks like you can't directly access the html file I uploaded as an example. You will have to download it.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1391a8e0183535b6f56586c844d2bbcc543e8237&tochange=3204779395ce6e90bfb39558484641a2d63d2fa7

Regressed by;
3204779395ce6e90bfb39558484641a2d63d2fa7 Yura Zenevich — Bug 1506510 - add/fix keyboard focus styling for notification and blocked permission icon buttons. r=dao
19ec728351acee967f424397f9f3e1b11e11c0f3 Yura Zenevich — Bug 1506510 - add keyboard focus styling for toolbar/urlbar buttons. r=dao

Actually a regression from bug 1436086, but looks like bug 1506510 because that bug enabled bug 1436086 by default.

Thanks for narrowing that down, Jamie. Do you think you'll have time to investigate this?

I can try to look into this, though it probably won't be until the new year.

This is happening because focus moves from the browser to the first focusable tabstop, which is the one at the start of the navbar.

Then we hit the focus handling code in toolbarKeyNav. More or less the following then happens:

1. we determine we're moving focus backwards, because oldFocus.compareDocumentPosition(aEvent.target) & Node.DOCUMENT_POSITION_PRECEDING is true. This is because, of course, the browser is at the end of the document and the tabstop is at the beginning. The comparison can't "tell" that this is because we're wrapping around. We don't normally hit this because when the tabstrip is visible, focus moves to the tabstrip without ever hitting our tabstop code (buttons in the tabstrip are not keyboard navigable).
2. we look for a thing to move focus to by calling walker.nextNode() - this is already a bit odd because we're not taking the "backwards" bit into account; shouldn't we call previousNode() ? It looks like maybe we rely on this because we skip backward some more (manually) if we shift-tab anyway, because the this._isFocusMovingBackward && oldFocus && this._isButton(oldFocus) case will normally be true... But because there are no buttons in the toolbar because this is a popup window with toolbar=0, we don't find any buttons until we hit the next tabstop (in the url bar, for the identity button etc.). That (correctly) fails the _isButton check, so then we give up and rely on the focus manager to move focus some more.
3. except we ask it to rewindFocus because in (1) we decided we were moving backwards. This puts focus back where it was before, which is obviously not helpful.

We can't rely on the last element being the browser, because if the user opens the find bar or the devtools that won't be true, and in any case people could also shift-tab from the browser to the toolbar...

However, we can probably check if old focus was outside of the toolbox, and now focus is inside the toolbox (this is trivially true if we hit this code as that's where the event listener is), and we end up without a button / with a non-button to focus.

The priority flag is not set for this bug.
:Gijs, could you have a look please?

For more information, please visit auto_nag documentation.

Hi Gijs, is this on track to land in time for Fx73?

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)

Hi Gijs, is this on track to land in time for Fx73?

Probably not given we're what, halfway through the beta cycle at this point? The issue is a bit older, and this is a bit of an edgecase because it's popups, and I have had some other more urgent stuff on my plate. At the moment I have a working patch but I was hoping to write an automated test because the patch is pretty finnicky.

Any update on this bug? Thanks

(In reply to Pascal Chevrel:pascalc from comment #11)

Any update on this bug? Thanks

I need to finish writing the test, and couldn't do that away from my windows machine, plus I've had other things on my plate. Hopefully sometime soon.

https://hg.mozilla.org/mozilla-central/rev/11e7af8d4d71

Is this something we should consider for Beta uplift?

(In reply to Ryan VanderMeulen [:RyanVM] from comment #15)

Is this something we should consider for Beta uplift?

Jamie's summary was

This is pretty intense, but I don't think there's a simpler way to handle it

so I dunno. :-)

I added a test, so I'm moderately confident this won't mess up loads of things, so maybe? Jamie, what do you reckon?

I found it intense in that it took me quite a while to digest and follow how it worked. However, it is isolated to a very specific area of the code and it shouldn't trigger under other circumstances, plus it has a test, which significantly lowers the risk.

It's probably okay for uplift, but I'm not super convinced it's essential for 74. I imagine there aren't many users who would tab through an entire page to get to the browser chrome, though I don't have any actual data on that.

Makes sense, let's not risk it then. :-)

I have managed to reproduce the issue using Fx73.0a1.
The issue is verified fixed using Fx75.0b6 and Fx76.0a1 on Windows 10 and Ubuntu 18.04. Tabbing now correctly re-cycles through all the content and Firefox options.