Some items in the Site Identity panel are not focusable/navigatable if you click the Site Identity icon and navigate to them

VERIFIED FIXED in Firefox 68

Status

()

defect
P1
normal
VERIFIED FIXED
4 months ago
2 months ago

People

(Reporter: itiel_yn8, Assigned: Gijs)

Tracking

(Blocks 1 bug, Regression, {regression})

unspecified
Firefox 68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 wontfix, firefox68 verified)

Details

Attachments

(1 attachment)

STR:

  1. Restart Nightly or start it from fresh
  2. Open https://bugzilla.mozilla.org/home
  3. Click the Site Identity icon in the URL bar (the {i) icon)
  4. Using Tab/Arrow keys try to navigate to "Trackers", "Cookies", "Standard (cog)" and the permissions section's cog

AR:
You can't.

ER:
You should be able to.

This doesn't reproduce if you Ctrl+L > Shift+Tab and Enter, and after doing so you wouldn't be able to reproduce the STR above. Restarting Nightly will make this reproducible.

I think this occurs because we no longer use PanelMultiView keyboard navigation in the Site Identity panel as per the short-term workaround in bug 1522092. Unfortunately, that means PanelMultiView doesn't run the initialisation code which makes these controls focusable. (It does run that code if you activate the panel from the keyboard, though, because PanelMultiView has to focus the first control in that case.)

This will probably be fixed by bug 1477673, where we will fix PanelMultiView's key nav code and then switch Site Identity to use it again.

Unfortunately, this probably means this bug is a regression introduced by bug 1522092. Easiest way to work that out would be to test with Firefox 66.

Depends on: 1477673

(In reply to James Teh [:Jamie] from comment #1)

Unfortunately, this probably means this bug is a regression introduced by bug 1522092. Easiest way to work that out would be to test with Firefox 66.

Ug. Nope, because we uplifted that patch to 66. So we'd need to test with 65 (or just revert the patch and test with a local build).

I confirmed that this was regressed by bug 1522092. :(

Blocks: 1522092
Keywords: regression
Priority: -- → P1

Assigning to Jamie so it can be closed after bug 1477673 gets fixed and is confirmed to fix this issue too.

Assignee: nobody → jteh
Regressed by: 1522092
See Also: → 1539976

bug 1477673's fix bounced so still continuing to wait on that.

The STR can still be reproduced after bug 1477673, though the issue is a bit different now.
You can now open all of the items in the panel, but they're not focusable (except for the 2 cog buttons), meaning there's no visual representation of them being selected.
And again, you must first click the Site Identity icon in the URL bar the first time in Nightly's session to reproduce this.

Flags: needinfo?(jteh)

(In reply to Itiel from comment #6)

The STR can still be reproduced after bug 1477673, though the issue is a bit different now.
You can now open all of the items in the panel, but they're not focusable (except for the 2 cog buttons),

Are you still doing this with the tab key? If they're not focusable (and thus not getting focus), how are you opening them?

I can't reproduce this. Even if I click the Site identity icon, tab then gets me to all the controls. That said, I can't see the screen, so perhaps they're somehow getting focus without there being visual indication. I don't know why that would be, though.

Can you provide complete and updated STR (including exact actions and expected results at each point) for clarity? Thanks.

Flags: needinfo?(jteh)

(In reply to James Teh [:Jamie] from comment #7)

Are you still doing this with the tab key?

Doesn't matter if you're tabbing or using the space key.

If they're not focusable (and thus not getting focus), how are you opening them?

I meant visually focused. Behind the scenes they are actually focused.

That said, I can't see the screen, so perhaps they're somehow getting focus without there being visual indication.

Yes, that's the case.

Can you provide complete and updated STR (including exact actions and expected results at each point) for clarity? Thanks.

Sure!

  1. Restart Nightly (or be in a session that you didn't open the Site Identity panel by keyboard navigation means)
  2. Open bugzilla.mozilla.org
  3. Click the Site Identity icon
  4. Use the ArrowDown or Tab key once to navigate to the "Show connection details" (>) button -- the button is not outlined or changing to a hover state. The same issue reproduces for all buttons in this panel except for the 2 cog buttons
  5. Even though it seems nothing is focused/selected, hit Enter -- will work as expected
  6. Close the Site Identity panel
  7. You can repeat steps 3-6 as many times as you'd like, as long as you don't open the Site Identity panel by keyboard navigation means
  8. Ctrl+L > Shift+Tab > Enter, and repeat step #4 -- this will now work as expected until the next restart to Nightly.

Hope that's clear enough :)

Flags: needinfo?(jteh)

Thanks. Since focus is being set correctly, I have no idea why it isn't being reflected visually. Given that I can't see, this is not something I can investigate. Gijs, can you help find someone to assist with this? See comment 8. (Note that a11y does report focus, so this seems to be purely visual.)

Flags: needinfo?(jteh) → needinfo?(gijskruitbosch+bugs)

This doesn't reproduce on mac. This is caused by how we track "should we show focus rings" on Windows/Linux. The elements that have working visual feedback use :focus and the elements that are broken use :-moz-focusring.

Generally speaking, I believe :-moz-focusring is preferred, to avoid showing the focused styling for users who navigate the UI by clicking things, and where showing what to them is effectively a "hover" style when the item is not actually hovered by the cursor (after pressing a button once, for instance) is confusing.

I think the bug here is that tabbing around the panel somehow doesn't trip the window-global "This user uses keyboard navigation" state flag, whereas using shift-tab + enter to open the identity popup from the navbar does do that.

It looks like the panelmultiview code that sets focused elements in response to keypresses might need to pass FLAG_BY_KEY, based on https://searchfox.org/mozilla-central/rev/d33d470140ce3f9426af523eaa8ecfa83476c806/dom/base/nsGlobalWindowInner.cpp#4023-4028 which is used in ther window's "do we need to show focus rings" tracking code.

Flags: needinfo?(gijskruitbosch+bugs)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6fea732af766
pass along whether a focus change was tripped by a keypress to ensure :-moz-focusring works as designed, r=Jamie
Backout by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/291b093ea26a
Backed out changeset 6fea732af766 for browser_PanelMultiView_keyboard.js failures CLOSED TREE

So the spacebar insertion instead wipes the field, indicating its contents were selected. That's why the test fails. Why only on mac? "home" on a mac does not, in fact, move focus to the start of the line (cmd-arrow does). It only happened to work because the test wasn't selecting the input before. Now that we're moving focus with the keyboard, that's totally what it does because the HTML input code checks for the last focus movement method when deciding whether to select field contents, and it makes things sad. But only on mac, because on linux/windows dispatching 'home' fixed things.

Attachment #9058906 - Attachment description: Bug 1539984 - pass along whether a focus change was tripped by a keypress to ensure :-moz-focusring works as designed, r?Jamie → Bug 1539984 - pass along whether a focus change was tripped by a keypress to ensure :-moz-focusring works as designed, r=Jamie
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4ffdb79d8075
pass along whether a focus change was tripped by a keypress to ensure :-moz-focusring works as designed, r=Jamie
Assignee: jteh → gijskruitbosch+bugs
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

I'm tempted to suggest we let this ride with 68 (ie wontfix on 67) as I don't think the steps in comment #0 (never using tab/shift-tab and then clicking to open the identity panel and then starting to use tab/shift-tab) are all that common. Itiel / Johann, could you share if that seems like the right decision to you?

Flags: needinfo?(jhofmann)
Flags: needinfo?(itiel_yn8)

I agree, seems like an edge-case. :)

Flags: needinfo?(jhofmann)

(In reply to Johann Hofmann [:johannh] from comment #18)

I agree, seems like an edge-case. :)

Same here.
Additionally, this may give me (or others) enough time to find unforeseeable regression(s) that may be caused by this fix.

Flags: needinfo?(itiel_yn8)

Thanks!

Fixed on latest Nightly.

Marking VERIFIED FIXED based on comment 21.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.