1171085 - [Control Center] Restore keyboard accessibility for identity panel

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Tim Taubert [:ttaubert] (inactive)]

Bug 1146269 removed the autofocus but we need to bring it back in. Otherwise keyboard navigation inside the panel is mostly not possible.

Comment 1

[Marco Mucci [:MarcoM]]

Hi Team, does bug 1171085 block the release of the initial user story (bug 1168883) or can bug 1171085 be dealt with later on?

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

It's not blocking the story but it should "block" the release. We shouldn't ship 41 without that fixed. It might maybe block bug 1168423? We need that to fix it.

Comment 3

[Marco Mucci [:MarcoM]]

(In reply to Tim Taubert [:ttaubert] from comment #2)
> It's not blocking the story but it should "block" the release. We shouldn't
> ship 41 without that fixed. It might maybe block bug 1168423? We need that
> to fix it.

Sounds good.  I'll make bug 1171085 a dependency of bug 1168423.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Picking this up, we can't currently navigate the control panel with the keyboard and we'd need to uplift the fix to 41.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1171085-Control-Center-Implement-keyboard-naviga.patch

This should look decent on all platforms and makes keyboard navigation work, including showing and hiding subviews.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Attachment: Keyboard navigation on OS X

Here's how it looks like on OS X.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1171085-Control-Center-Implement-keyboard-naviga.patch, v2

Small fix for the footer button on Linux.

Comment 9

[:Gijs (he/him)]

Comment on attachment 8627739 [details] [diff] [review]
0001-Bug-1171085-Control-Center-Implement-keyboard-naviga.patch, v2

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

This generally looks good (though conflicts now that tp-for-control-center landed), except that if I:

1) focus the "more information" button
2) click the |>| expander

I still see a focus outline on the more information button - but hitting space/enter does nothing.

Can you fix this? :-)

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1171085-Control-Center-Implement-keyboard-naviga.patch, v3

Thanks for finding that scenario you describe above, it's fixed in this patch:

1) When toggling subviews an the expand button isn't the focused element, then clear the window's focus.

2) Added <menulist> elements from the permissions section to the -moz-user-focus:ignored list when showing a subview.

3) This patch removes the blur handler as it interferes with (1).


What I didn't realize when landing bug 1146269 is that the blur handler (3) took care of closing the identity panel when the focus went away so that it's closed when switching tabs, opening a new tab, closing a tab, and similar actions. With no element having a focus by default that doesn't work anymore, and I think that the current behavior of not focusing anything until the user <TAB>s around is the right behavior.

Will file a follow-up to see what we can do about closing the popup when obviously the user expects it to close, e.g. when opening a new tab or switching tabs.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Tim Taubert [:ttaubert] from comment #10)
> Will file a follow-up to see what we can do about closing the popup when
> obviously the user expects it to close, e.g. when opening a new tab or
> switching tabs.

Filed bug 1180642.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

[Tracking Requested - why for this release]:

Need to bring back keyboard navigation for the identity panel.

Comment 13

[:Gijs (he/him)]

Why can't we focus the anchor instead? Then we can keep the blur handler...

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

(In reply to :Gijs Kruitbosch from comment #13)
> Why can't we focus the anchor instead? Then we can keep the blur handler...

That doesn't work unfortunately. If an element outside of the panel is focused the focus manager will not switch to any of the elements in the popup. We could fall back to focusing the "More Information" button by default but we'd have to clear its focus when showing a subview and then face the same problem again :/

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d4708e272a05

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d4708e272a05

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8629645 [details] [diff] [review]
0001-Bug-1171085-Control-Center-Implement-keyboard-naviga.patch, v3

Approval Request Comment
[Feature/regressing bug #]: bug 1146269
[User impact if declined]: Identity panel is not keyboard accessible.
[Describe test coverage new/current, TreeHerder]: No special test coverage.
[Risks and why]: Low risk.
[String/UUID change made/needed]: None.

Comment 18

[Kate Glazko]

Tracking, see Comment 2

Comment 19

[Kate Glazko]

Comment on attachment 8629645 [details] [diff] [review]
0001-Bug-1171085-Control-Center-Implement-keyboard-naviga.patch, v3

Has been on m-c for 3 days with no known issues, low-risk.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/616ec311f1e4

Comment 21

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 41b2, 42.0a2 (2015-08-20) Win 7.

Comment 22

[Paul Silaghi, QA [:pauly]]

Actually I'm unable to navigate with keyboard inside the panel on OS X 10.10.5, 42.0a2 (2015-08-23), 43.0a1 (2015-08-23). Thoughts?
Works fine on Win 7, Ubuntu 14.04.

Comment 23

[:Gijs (he/him)]

(In reply to Paul Silaghi, QA [:pauly] from comment #22)
> Actually I'm unable to navigate with keyboard inside the panel on OS X
> 10.10.5, 42.0a2 (2015-08-23), 43.0a1 (2015-08-23). Thoughts?
> Works fine on Win 7, Ubuntu 14.04.

You're testing on OS X and you haven't set the "full keyboard access" pref? (system Keyboard preferences, in the "Shortcuts" tab on yosemite)

Comment 24

[Paul Silaghi, QA [:pauly]]

Thank you. That was the problem.

Comment 25

[Petruta Horea [:phorea], Desktop QA]

Gijs, after setting "full keyboard access" on a macbook pro retina with OS X 10.9.5, I can only use "Space" to select an option and navigate further. "Enter" shouldn't have the same behavior?
  
On Win and Ubuntu "Enter" key can also be used. 

Thank you!

Comment 26

[:Gijs (he/him)]

(In reply to Petruta Rasa [QA] [:petruta] from comment #25)
> Gijs, after setting "full keyboard access" on a macbook pro retina with OS X
> 10.9.5, I can only use "Space" to select an option and navigate further.
> "Enter" shouldn't have the same behavior?
>   
> On Win and Ubuntu "Enter" key can also be used. 
> 
> Thank you!

I think enter should work, but that is not a bug in this patch. Please file a separate unrelated bug about this and CC me, and needinfo enndeakin about this behaviour.