881937 - The Australis panel menu should be keyboard accessible

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Johan C]

The new panel menu for Australis should be accessible by keyboard.

It would be great if you could open the menu with a keyboard shortcut; and even more so if you could navigate it (i.e. using the arrow keys and enter/return).

Comment 1

[Justin Dolske [:Dolske]]

Hmm. The Firefox button (which the panel sorta replaces) wasn't keyboard accessible. The reasoning there was that it duplicated the (accessible) menubar contents, and as UI intended for mouse users it wasn't interesting to make it keyboard accessible. But I think that while the panel menu still duplicates some menubar entries, it can also contain arbitrary addon items. And so it's roughly equivalent to the navbar needing to be accessible.

So P1, but I could be convinced otherwise.

Comment 2

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #1)
> Hmm. The Firefox button (which the panel sorta replaces) wasn't keyboard
> accessible. The reasoning there was that it duplicated the (accessible)
> menubar contents, and as UI intended for mouse users it wasn't interesting
> to make it keyboard accessible. But I think that while the panel menu still
> duplicates some menubar entries, it can also contain arbitrary addon items.
> And so it's roughly equivalent to the navbar needing to be accessible.
> 
> So P1, but I could be convinced otherwise.

I do not believe the navbar is keyboard-accessible, and I therefore don't think this should be a P1.

I'd like to implement this, but I'm not sure how hard it would be. For starters, though, what would we want the shortcut for the menu to be? F10 and "Alt" are taken for the actual menubar on Windows, and we probably shouldn't change that.

Comment 3

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #2)
> I'd like to implement this, but I'm not sure how hard it would be. For
> starters, though, what would we want the shortcut for the menu to be? F10
> and "Alt" are taken for the actual menubar on Windows, and we probably
> shouldn't change that.

Zhenshuo, do you have any ideas for the keyboard bindings here and/or know who else I should ask about them?

Comment 4

[Zhenshuo Fang (:fang) - Firefox UX Team]

(In reply to :Gijs Kruitbosch from comment #3)
> Zhenshuo, do you have any ideas for the keyboard bindings here and/or know
> who else I should ask about them?

This SUMO page contains most of (not sure it's all of) the keyboard shortcuts https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly

Comment 5

[:Gijs (he/him)]

(In reply to Zhenshuo Fang (:fang) - Firefox UX Team from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > Zhenshuo, do you have any ideas for the keyboard bindings here and/or know
> > who else I should ask about them?
> 
> This SUMO page contains most of (not sure it's all of) the keyboard
> shortcuts
> https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-
> tasks-quickly

After a quick discussion with fang on IRC, we're currently planning to use ctrl-m on Windows/Linux, and cmd-shift-m on Mac (where cmd-m is taken for the OS's minimize function).

Comment 6

[:Gijs (he/him)]

Attachment: Add shortcut to toggle the menupanel

First step: add a global shortcut. This obviously isn't enough by itself, but it's a start... Mike, do you think you can review this?

Comment 7

[Mike de Boer [:mikedeboer]]

I don't know man, sure looks like the most complicated piece of code I've seen... like ever!

Comment 8

[Mike de Boer [:mikedeboer]]

Comment on attachment 776388 [details] [diff] [review]
Add shortcut to toggle the menupanel

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

Looks good, apart from a few small remarks!

::: browser/base/content/browser-sets.inc
@@ +400,5 @@
> +#ifdef XP_MACOSX
> +         key="&toggleMenuPanelMac.key;" modifiers="accel,shift"/>
> +#else
> +         key="&toggleMenuPanel.key;" modifiers="accel"/>
> +#endif

Not necessary atm (see my other comment in browser.dtd)

@@ +408,5 @@
>  #else
>  #define NUM_SELECT_TAB_MODIFIER accel
>  #endif
>  
> +

nit: please remove this newline.

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +343,5 @@
> +     toggleMenuPanel. However, the modifier is different on mac (cmd+shift rather than ctrl)
> +     and so you may need to pick a different key to avoid conflicts with other shortcuts. -->
> +<!ENTITY toggleMenuPanel.key          "m">
> +<!ENTITY toggleMenuPanelMac.key       "m">
> +

since the key is the same now, regardless of the modifiers, I'd like to see this added only when it's needed. I other words: please remove this line.

Comment 9

[Mike de Boer [:mikedeboer]]

Comment on attachment 776388 [details] [diff] [review]
Add shortcut to toggle the menupanel

On second thought, Gijs kindly reminded me that the modifier key difference is significant and warrants the additional property to localize. r=me.

Comment 10

[:Gijs (he/him)]

Comment on attachment 776388 [details] [diff] [review]
Add shortcut to toggle the menupanel

Pushed with nits fixed, https://hg.mozilla.org/projects/ux/rev/2b7346629b68

Leaving bug open to work on the rest of making this work with the keyboard...

Comment 11

[:Gijs (he/him)]

Attachment: focus the first focusable item

So this is one part of fixing this further... bring focus into the panel iff it was opened using the shortcut (otherwise we'd break cut/copy/paste because we take focus out of the area where you'd be pasting...)

Comment 12

[:Gijs (he/him)]

Attachment: tabindex all the things

And this is another part... tabindex all the things!

3 Caveats:
(1) we still need to fix styling (but we have a separate bug about updating that per the design anyway, I'm pretty sure, and I don't think it should block progress here - we can fix the :focus styles in one go);
(2) we probably need something fancy to deal with  subviews correctly (escape should probably go back to the main view, and you shouldn't be able to tab 'out of' the subview), and
(3) I've not made the edit-controls focusable because I suspect they wouldn't work... perhaps only cut/copy would work? Paste definitely wouldn't. :-\

Comment 13

[:Gijs (he/him)]

Comment on attachment 777794 [details] [diff] [review]
focus the first focusable item

Thinking about this one more, maybe I could just toggle the noautofocus attribute on the panel, that'd be cleaner. Will check this before re-requesting review.

Comment 14

[:Gijs (he/him)]

Attachment: focus the first focusable item

Yup, this seems to work even better, and looks a lot nicer, too.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 777802 [details] [diff] [review]
tabindex all the things

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

::: browser/components/customizableui/src/CustomizableUI.jsm
@@ +534,5 @@
>        this.ensureButtonContextMenu(widgetNode, aArea == CustomizableUI.AREA_PANEL);
> +      if (widgetNode.localName == "toolbarbutton" && aArea == CustomizableUI.AREA_PANEL) {
> +        widgetNode.setAttribute("tabindex", "0");
> +      } else {
> +        widgetNode.removeAttribute("tabindex");

Should we also remove this attribute when removing the widget from an area?

::: browser/components/customizableui/src/CustomizableWidgets.jsm
@@ +318,5 @@
>        buttons.forEach(function(aButton) {
>          let btnNode = aDocument.createElementNS(kNSXUL, "toolbarbutton");
>          setAttributes(btnNode, aButton);
> +        if (inPanel)
> +          btnNode.setAttribute("tabindex", "0");

You'll need to do this for the edit controls as well.

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Also, I imagine this keyboard access stuff will be an iterative thing, but subviews should probably have their first item focused after opening...

Comment 17

[:Gijs (he/him)]

Attachment: tabindex all the things

I believe you're actually right on both counts; updated patch! :-)

Comment 18

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 778503 [details] [diff] [review]
tabindex all the things

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

I *think* this is all of the places where this needs to go. I guess we'll find out if we missed any. Thanks Gijs!

Comment 19

[:Gijs (he/him)]

Comment on attachment 778452 [details] [diff] [review]
focus the first focusable item

https://hg.mozilla.org/projects/ux/rev/539f516783b0

Comment 20

[:Gijs (he/him)]

Comment on attachment 778503 [details] [diff] [review]
tabindex all the things

https://hg.mozilla.org/projects/ux/rev/3e84d97864bd

Comment 21

[Dão Gottwald [:dao]]

(In reply to Justin Dolske [:Dolske] from comment #1)
> Hmm. The Firefox button (which the panel sorta replaces) wasn't keyboard
> accessible. The reasoning there was that it duplicated the (accessible)
> menubar contents, and as UI intended for mouse users it wasn't interesting
> to make it keyboard accessible. But I think that while the panel menu still
> duplicates some menubar entries, it can also contain arbitrary addon items.
> And so it's roughly equivalent to the navbar needing to be accessible.

As Gijs mentioned, the nav bar isn't keyboard accessible. Add-ons always had to add themselves to the menu bar for full keyboard accessibility.

> So P1, but I could be convinced otherwise.

I think we should wontfix this. If we just had buttons in the panel, this would be a no-brainer, but with the sub view model and custom widgets that may not work like buttons at all, it becomes a lot harder to design and implement a sane and predictable keyboard model. A half-baked model is worse than just saying "use the menu bar", as it would be inconsistent and cause frustration every time users run into cases that don't work.

The idea that the panel menu should be keyboard accessible also constraints us in how we present the panel menu to mouse users -- see bug 900428 comment 1.

Comment 22

[Justin Dolske [:Dolske]]

We discussed this at the Australis meeting today...

* From an a11y perspective (Gijs), we already have keyboard methods for all the functions in the menu panel. That meets minimum requirements, and thus makes this not-P1. Turns out he has already talked with Marco Zehe about this, too.

* In a general sense, keyboard navigation is desirable for the people who use it, and so we should figure out here to what degree we want this panel to be KB navigable so such users have an expected/acceptable UX. (Ditto for the side point that we don't want to treat a11y/AT as a separate-but-equal path.)

* Asa noted that perhaps in the long-term (long-long-term :) the panel could replace the menubar, and so thinking about how that might end up could help guide what we start doing now. Zhenshuo's going to mull this over.

Comment 23

[Justin Dolske [:Dolske]]

(In reply to Dão Gottwald [:dao] from comment #21)

> > And so it's roughly equivalent to the navbar needing to be accessible.
> 
> As Gijs mentioned, the nav bar isn't keyboard accessible. Add-ons always had
> to add themselves to the menu bar for full keyboard accessibility.

Shorlander mentioned that most OSen have a pref to enable app toolbar KB navigation, but it was unclear from a quick test in the meeting if that was actually the case or how well it worked.

But it's sounding like I was wrong on this point.

> I think we should wontfix this. If we just had buttons in the panel, this
> would be a no-brainer, but with the sub view model and custom widgets that
> may not work like buttons at all, it becomes a lot harder to design and
> implement a sane and predictable keyboard model. A half-baked model is worse
> than just saying "use the menu bar", as it would be inconsistent and cause
> frustration every time users run into cases that don't work.

What makes it hard? (Honest question, I don't have an easy solution in mind ;). Can you say a bit more about what makes is complex? I assume you're talking about things like where focus goes if you have a searchbox widget in the menu panel?

I just want to make sure that we're not throwing out kbnav without having fully considered it. (And since other popups -- awesomebar results, bookmarks, downloads -- are navigable.)

Comment 24

[Dão Gottwald [:dao]]

(In reply to Justin Dolske [:Dolske] from comment #22)
> We discussed this at the Australis meeting today...
> 
> * From an a11y perspective (Gijs), we already have keyboard methods for all
> the functions in the menu panel. That meets minimum requirements, and thus
> makes this not-P1. Turns out he has already talked with Marco Zehe about
> this, too.
> 
> * In a general sense, keyboard navigation is desirable for the people who
> use it, and so we should figure out here to what degree we want this panel
> to be KB navigable so such users have an expected/acceptable UX. (Ditto for
> the side point that we don't want to treat a11y/AT as a separate-but-equal
> path.)

Here's what I experienced when playing around with this for a few minutes: When activating the history button with the keyboard, a sub view opened but it didn't have focus. I could tab around mostly blindly until the sub view would receive focus, but then I saw no way to close the sub view. I then tried a different but similar widget: I activated the bookmarks button with the keyboard and got a popup rather than a sub view. When dismissing the popup, focus behavior became wonky: Tab seemed to work but arrow keys randomly stopped moving focus.

So I don't think what we've got here meets a minimum standard. And IMHO, as mentioned in comment 21, half-baked support is worse than pushing keyboard users towards the menu bar which has a very straightforward and predictable keyboard access model.

It's also still unclear why this should be a minimum requirement in the first place, as the situation without those patches was pretty much the same compared to pre-Australis.

> * Asa noted that perhaps in the long-term (long-long-term :) the panel could
> replace the menubar, and so thinking about how that might end up could help
> guide what we start doing now. Zhenshuo's going to mull this over.

That's a logical thing to do as soon as we have a keyboard model that's as good as the menu bar's. We don't have that yet, neither designed nor implemented, and I don't think it should block Australis. Of course you didn't say it should, but what you said seems to imply that it would be ok to ship with keyboard access for the panel menu even though it would be incomplete. I'd disagree with that - we shouldn't send users in directions that may lead to dead ends and make UI interaction less efficient.

(In reply to Justin Dolske [:Dolske] from comment #23)
> What makes it hard? (Honest question, I don't have an easy solution in mind
> ;). Can you say a bit more about what makes is complex? I assume you're
> talking about things like where focus goes if you have a searchbox widget in
> the menu panel?

Apart from the issues mentioned above, what landed here also only sets the tabindex attribute on buttons -- for custom widgets (such as our own zoom or edit controls), it relies on the widget provider to do this. Add-ons will surely miss this.

Comment 25

[Dão Gottwald [:dao]]

Oh, and this lacks access keys, a key feature for regular and efficient keyboard use. :/

Comment 26

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #24)
> Here's what I experienced when playing around with this for a few minutes:
> When activating the history button with the keyboard, a sub view opened but
> it didn't have focus. I could tab around mostly blindly until the sub view
> would receive focus, but then I saw no way to close the sub view. I then
> tried a different but similar widget: I activated the bookmarks button with
> the keyboard and got a popup rather than a sub view. When dismissing the
> popup, focus behavior became wonky: Tab seemed to work but arrow keys
> randomly stopped moving focus.

So, the subviews thing is known (see also comment 16, comment 12) and I agree should be addressed.

At the time when I worked on this, the bookmark widget didn't exist yet; if it's broken we should fix that. It should be opening a subview initially, AIUI, no matter how it's activated - but I'll ask mconley to be sure. Regarding where focus goes when you close a menupopup, that might be a bug in our focus handling generally although improved focus styling in the menupanel will help make this less painful.

Generally, I think opening a subview should focus its first item, and focus should be constrained to the subview until you hit escape/backspace/left-arrow to close the subview. I'm not entirely sure if each of these needs to work, I'd need to play around and see what seems natural. I don't think this should be hard to implement, in principle.

> So I don't think what we've got here meets a minimum standard. And IMHO, as
> mentioned in comment 21, half-baked support is worse than pushing keyboard
> users towards the menu bar which has a very straightforward and predictable
> keyboard access model.

I think this is where we disagree. I don't think this is ready to ship on release today (much like most of Australis) don't get me wrong. However, I don't think it's OMGWTFBACKITOUT quality, either. I'd like to fix this in an iterative fashion, and as we have the menubar as 'backup' for keyboard users, I think that's fine. We're not forcing people to use this, and as it is the menupanel button isn't in the taborder, nor is the shortcut discoverable (that part we may want to fix somehow or other if we end up sticking with this, possibly on the first run page and/or tooltip?). I think that's a fine compromise in terms of being able to work on it but not forcing people to use something which is not perfect just yet. If we find we can't get it to a minimum standard on the nightly cycle, I agree we should back this out, but I think doing that now because it's not perfect yet would be premature.

> It's also still unclear why this should be a minimum requirement in the
> first place, as the situation without those patches was pretty much the same
> compared to pre-Australis.

I agree that if necessary, we can ship without this, hence asking for it not to be P1. But I feel it would be a significant win if we could ship with decent keyboard support.

Note that Stephen pointed out to the meeting that on OS X, at least, you can make toolbars tabbable in ordinary Mac apps, so us not supporting that is technically a bug (which I'm sure will be filed somewhere already).

> > * Asa noted that perhaps in the long-term (long-long-term :) the panel could
> > replace the menubar, and so thinking about how that might end up could help
> > guide what we start doing now. Zhenshuo's going to mull this over.
> 
> That's a logical thing to do as soon as we have a keyboard model that's as
> good as the menu bar's. We don't have that yet, neither designed nor
> implemented, and I don't think it should block Australis. Of course you
> didn't say it should, but what you said seems to imply that it would be ok
> to ship with keyboard access for the panel menu even though it would be
> incomplete. I'd disagree with that - we shouldn't send users in directions
> that may lead to dead ends and make UI interaction less efficient.

First: no, this definitely doesn't block Australis.

Second, regarding what we ship, the reason the menubar is predictable is in part because it has a constrained structure. You can do more crazy things in the menupanel (e.g. text boxes, context menus) than you can in the menu on most OSen (hello, OS X) and that makes life harder for having a 'predictable' focus model (for instance, if I'm in a search box in the menupanel, and hit the up arrow, should I go to the beginning of the field or the item above it? I'd lean towards the former, but it's not completely well-defined - just like focus in the help menu in OS X isn't, for the same reason).

I don't think we're currently in a shippable state, but I don't think we need to be as 'perfect' as the menubar.

> (In reply to Justin Dolske [:Dolske] from comment #23)
> > What makes it hard? (Honest question, I don't have an easy solution in mind
> > ;). Can you say a bit more about what makes is complex? I assume you're
> > talking about things like where focus goes if you have a searchbox widget in
> > the menu panel?
> 
> Apart from the issues mentioned above, what landed here also only sets the
> tabindex attribute on buttons -- for custom widgets (such as our own zoom or
> edit controls), it relies on the widget provider to do this. Add-ons will
> surely miss this.

I feel quite strongly that this shouldn't block us here. We should ensure, to the best of our abilities, that builtin stuff works, and that it's relatively easy for add-ons to play along if they have complex items, but if they do have such items and don't play nicely in the taborder, that goes in the general bucket of "sometimes add-ons do imperfect things; tough" - goodness knows how many add-ons don't follow platform conventions everywhere or are 'imperfect' in various other ways. I don't think this should be an exception where we give up on doing things just because add-ons might make things work less-than-perfectly. For things that genuinely bust keyboard accessibility, we have AMO reviewers.

(In reply to Dão Gottwald [:dao] from comment #25)
> Oh, and this lacks access keys, a key feature for regular and efficient
> keyboard use. :/

I promise to review your patches and buy you beers for them! ;-)
It's not actually hard to add them (we did extra work in bug 880918 to make them work!), I've just been bogged down in perf work so I haven't worked on this recently; the subview stuff would worry me more (in terms of complexity) if I did, though...

(Mike, needinfo'ing you about the bookmarks button not opening a subview - did I miss something?)

Comment 27

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #26)
> I think this is where we disagree. I don't think this is ready to ship on
> release today (much like most of Australis) don't get me wrong. However, I
> don't think it's OMGWTFBACKITOUT quality, either. I'd like to fix this in an
> iterative fashion, and as we have the menubar as 'backup' for keyboard
> users, I think that's fine. We're not forcing people to use this,

Well, what again is the argument for providing this at all? That the panel may contain stuff not exposed elsewhere in a keyboard-accessible manner? In that case, the menu bar wouldn't be a good backup anymore. Secondly, while it's good to know one has a backup, we shouldn't need it. Saying that people don't need to use this is really a poor excuse for shipping something half-baked. Broken record: If we expose a less efficient path to users, it will make some users less efficient. Choice isn't always beneficial or helpful.

> and as it
> is the menupanel button isn't in the taborder, nor is the shortcut
> discoverable (that part we may want to fix somehow or other if we end up
> sticking with this, possibly on the first run page and/or tooltip?).

s/may/should/

If we think this is an important, worthwhile and matured feature, we should expose it properly. If we think otherwise, we shouldn't ship it. And if maybe not exposing it is already part of the plan, then frankly we shouldn't spend time on it.

> Note that Stephen pointed out to the meeting that on OS X, at least, you can
> make toolbars tabbable in ordinary Mac apps, so us not supporting that is
> technically a bug (which I'm sure will be filed somewhere already).

Screen readers can do something similar, afaik. I think it's mostly a hack for keyboard-accessible menus not providing all necessary options (especially on OS X where applications always have a menu bar).

> Second, regarding what we ship, the reason the menubar is predictable is in
> part because it has a constrained structure. You can do more crazy things in
> the menupanel (e.g. text boxes, context menus) than you can in the menu on
> most OSen (hello, OS X) and that makes life harder for having a
> 'predictable' focus model (for instance, if I'm in a search box in the
> menupanel, and hit the up arrow, should I go to the beginning of the field
> or the item above it? I'd lean towards the former, but it's not completely
> well-defined - just like focus in the help menu in OS X isn't, for the same
> reason).

Spot on. Any complete keyboard focus model for the menu panel will inevitably be more complex than for the menu bar. There's an inherent strength of traditional menus and an inherent weakness of a keyboard-enabled complex panel. I think this is where we should pause for a moment and think hard whether we really want the latter.

> > Apart from the issues mentioned above, what landed here also only sets the
> > tabindex attribute on buttons -- for custom widgets (such as our own zoom or
> > edit controls), it relies on the widget provider to do this. Add-ons will
> > surely miss this.
> 
> I feel quite strongly that this shouldn't block us here. We should ensure,
> to the best of our abilities, that builtin stuff works, and that it's
> relatively easy for add-ons to play along if they have complex items, but if
> they do have such items and don't play nicely in the taborder, that goes in
> the general bucket of "sometimes add-ons do imperfect things; tough" -
> goodness knows how many add-ons don't follow platform conventions everywhere
> or are 'imperfect' in various other ways. I don't think this should be an
> exception where we give up on doing things just because add-ons might make
> things work less-than-perfectly. For things that genuinely bust keyboard
> accessibility, we have AMO reviewers.

This is missing some important context. We provide no straightforward API to make this work, and we can't, because custom widgets can have all kinds of interesting structures where "add the tabindex attribute here and there" may not be a sufficient solution.
Secondly, if we already say this is kind of redundant and keyboard users (already a minority) may not use it, and if add-on authors add relevant hooks to the menu bar (like they already should), it becomes less clear why this should be a requirement for AMO.

> (In reply to Dão Gottwald [:dao] from comment #25)
> > Oh, and this lacks access keys, a key feature for regular and efficient
> > keyboard use. :/
> 
> I promise to review your patches and buy you beers for them! ;-)
> It's not actually hard to add them (we did extra work in bug 880918 to make
> them work!),

It actually is hard, I'm afraid. Access keys in menus work differently than they do elsewhere. They don't require the user to hit Alt. This way keyboard users can navigate really fast through menus. On top of that, bug 880918 is again only relevant for buttons. For custom widgets, there may not even be a label.

Comment 28

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #27)
> (In reply to :Gijs Kruitbosch from comment #26)
> > I think this is where we disagree. I don't think this is ready to ship on
> > release today (much like most of Australis) don't get me wrong. However, I
> > don't think it's OMGWTFBACKITOUT quality, either. I'd like to fix this in an
> > iterative fashion, and as we have the menubar as 'backup' for keyboard
> > users, I think that's fine. We're not forcing people to use this,
> 
> Well, what again is the argument for providing this at all? That the panel
> may contain stuff not exposed elsewhere in a keyboard-accessible manner? In
> that case, the menu bar wouldn't be a good backup anymore. Secondly, while
> it's good to know one has a backup, we shouldn't need it. Saying that people
> don't need to use this is really a poor excuse for shipping something
> half-baked. Broken record: If we expose a less efficient path to users, it
> will make some users less efficient. Choice isn't always beneficial or
> helpful.

You're conflating two issues. The argument for having this is to provide mouse users and keyboard users an ability to use the same route through the UI as much as possible. There is already choice for mouse users (who can use the menubar just the same - although probably less efficiently than just using toolbar buttons!) too, it seems weird to be arguing that now having a choice is somehow wrong. That is orthogonal to how far along iterating on this feature is.

> > and as it
> > is the menupanel button isn't in the taborder, nor is the shortcut
> > discoverable (that part we may want to fix somehow or other if we end up
> > sticking with this, possibly on the first run page and/or tooltip?).
> 
> s/may/should/
> 
> If we think this is an important, worthwhile and matured feature, we should
> expose it properly. If we think otherwise, we shouldn't ship it. And if
> maybe not exposing it is already part of the plan, then frankly we shouldn't
> spend time on it.

I don't think it's useful to nitpick the exact words I used to phrase this. The first-run page is not part of this bug, and discussing it here seems pointless. I'm sure UX will give us their opinion on how to expose this properly if we implement it to a satisfactory extent. I'm happy to ask for feedback here in terms of the tooltip of the menu button containing the shortcut, too, if you think that belongs in here.

> > Second, regarding what we ship, the reason the menubar is predictable is in
> > part because it has a constrained structure. You can do more crazy things in
> > the menupanel (e.g. text boxes, context menus) than you can in the menu on
> > most OSen (hello, OS X) and that makes life harder for having a
> > 'predictable' focus model (for instance, if I'm in a search box in the
> > menupanel, and hit the up arrow, should I go to the beginning of the field
> > or the item above it? I'd lean towards the former, but it's not completely
> > well-defined - just like focus in the help menu in OS X isn't, for the same
> > reason).
> 
> Spot on. Any complete keyboard focus model for the menu panel will
> inevitably be more complex than for the menu bar. There's an inherent
> strength of traditional menus and an inherent weakness of a keyboard-enabled
> complex panel. I think this is where we should pause for a moment and think
> hard whether we really want the latter.

You're assuming that just because it's not trivial, that makes it a weakness. Arguments are fine, stop energy is not. Clearly, OS X copes with having a textbox in their menu, and Windows copes with context menus on their menus - I see no reason, a priori, that we can't. That it's not as trivial as a traditional menu shouldn't be an argument to give up before we've even started.

> This is missing some important context. We provide no straightforward API to
> make this work, and we can't, because custom widgets can have all kinds of
> interesting structures where "add the tabindex attribute here and there" may
> not be a sufficient solution.

"may"

You're straw-manning a hypothetical thing with an "interesting structure" where adding tabindices isn't enough. The web has functioned using tabindices and event handlers for some years now, and although recent aria work changed a lot of how stuff is exposed to AT in terms of semantics, they didn't change any ability to add behaviour. Why not? Because it wasn't necessary. If the web can already do this, certainly add-ons can too.

> Secondly, if we already say this is kind of redundant and keyboard users
> (already a minority) may not use it,

My point wasn't that it shouldn't be used, it's that while we're iterating, it isn't in the way of anything.

> and if add-on authors add relevant
> hooks to the menu bar (like they already should)

But don't always. Also not an AMO requirement. Add-ons that only add toolbarbuttons are actually *more* available to keyboard users now that the menu is keyboard-accessible.


> > (In reply to Dão Gottwald [:dao] from comment #25)
> > > Oh, and this lacks access keys, a key feature for regular and efficient
> > > keyboard use. :/
> > 
> > I promise to review your patches and buy you beers for them! ;-)
> > It's not actually hard to add them (we did extra work in bug 880918 to make
> > them work!),
> 
> It actually is hard, I'm afraid. Access keys in menus work differently than
> they do elsewhere. They don't require the user to hit Alt. This way keyboard
> users can navigate really fast through menus.

I had not thought of that. Still, that sounds fixable.

> On top of that, bug 880918 is
> again only relevant for buttons. For custom widgets, there may not even be a
> label.

If there isn't a keyboard shortcut to accomplish what such widgets do already (such as in the case of the zoom controls which, technically, don't have a label, even if their image is really sort of text...), I think that's not OK, and we should fix such widgets (or, add-on authors would have to). There is a separate bug on file about exposing the keyboard shortcuts for the individual items.

Note that add-ons do already add regular menuitems without accesskeys, or with accesskeys that conflict. I really don't think this alone is a reason to forgo having keyboard access here.

Comment 29

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #28)
> You're conflating two issues. The argument for having this is to provide
> mouse users and keyboard users an ability to use the same route through the
> UI as much as possible.

I wasn't aware of or forgot about you referring to this principle. Where is this from? It seems pretty bogus to me. Mouse and keyboard users don't generally go through the same routes and it's not clear to me why they should. Think of toolbar buttons as a prime example. What works well for one group may make no sense at all or at least be inefficient for the other.

> There is already choice for mouse users (who can use
> the menubar just the same - although probably less efficiently than just
> using toolbar buttons!) too, it seems weird to be arguing that now having a
> choice is somehow wrong. That is orthogonal to how far along iterating on
> this feature is.

The menu bar is basically hidden and not mouse-accessible for most of our users. Users can enable it, but then it's just a traditional menu bar with a straightforward mouse-interaction model. I don't think we'd invest in providing a mouse-accessible menu bar, but we basically get it for free.

> > > and as it
> > > is the menupanel button isn't in the taborder, nor is the shortcut
> > > discoverable (that part we may want to fix somehow or other if we end up
> > > sticking with this, possibly on the first run page and/or tooltip?).
> > 
> > s/may/should/
> > 
> > If we think this is an important, worthwhile and matured feature, we should
> > expose it properly. If we think otherwise, we shouldn't ship it. And if
> > maybe not exposing it is already part of the plan, then frankly we shouldn't
> > spend time on it.
> 
> I don't think it's useful to nitpick the exact words I used to phrase this.
> The first-run page is not part of this bug, and discussing it here seems
> pointless.

I didn't mean to advocate for the first-run page idea. I actually don't think it's a good one. It would only target a tiny minority and we have no reason to believe that most users in that minority actually read the first-run page. I tooltip wouldn't quite cut it either for keyboard-focused users either. My point was, if we think this is a worthwhile and important feature, we should expose it properly, whatever that means.

> > > Second, regarding what we ship, the reason the menubar is predictable is in
> > > part because it has a constrained structure. You can do more crazy things in
> > > the menupanel (e.g. text boxes, context menus) than you can in the menu on
> > > most OSen (hello, OS X) and that makes life harder for having a
> > > 'predictable' focus model (for instance, if I'm in a search box in the
> > > menupanel, and hit the up arrow, should I go to the beginning of the field
> > > or the item above it? I'd lean towards the former, but it's not completely
> > > well-defined - just like focus in the help menu in OS X isn't, for the same
> > > reason).
> > 
> > Spot on. Any complete keyboard focus model for the menu panel will
> > inevitably be more complex than for the menu bar. There's an inherent
> > strength of traditional menus and an inherent weakness of a keyboard-enabled
> > complex panel. I think this is where we should pause for a moment and think
> > hard whether we really want the latter.
> 
> You're assuming that just because it's not trivial, that makes it a
> weakness. Arguments are fine, stop energy is not.

I didn't say "not trivial" but "complex", as in: less straightforward, harder to predict and understand. I do think such an interaction model is a weak one.

> You're straw-manning a hypothetical thing with an "interesting structure"
> where adding tabindices isn't enough. The web has functioned using
> tabindices and event handlers for some years now, and although recent aria
> work changed a lot of how stuff is exposed to AT in terms of semantics, they
> didn't change any ability to add behaviour. Why not? Because it wasn't
> necessary. If the web can already do this, certainly add-ons can too.

On the web, interactive HTML widgets can be focused by default and where people cook up their own widgets, lacking accessibility is actually a common problem. In the browser, toolbar widgets generally cannot be focused by default, with the exception of text fields, so the burden on add-on authors would be significantly higher than it is for web content authors.

> > Secondly, if we already say this is kind of redundant and keyboard users
> > (already a minority) may not use it,
> 
> My point wasn't that it shouldn't be used, it's that while we're iterating,
> it isn't in the way of anything.

So when you said we wouldn't be forcing people to use this, you were referring to development cycles rather than when we'd release it? But you were also advocating for choice, and replacing the menu bar isn't part of the plan yet, so my point remains that keyboard users may actually not use this and it becomes less clear why it should be enforced by AMO.

> > and if add-on authors add relevant
> > hooks to the menu bar (like they already should)
> 
> But don't always. Also not an AMO requirement.

Maybe we should make it one? The good news is that the SDK could provide a sane API for this, if it doesn't already.

> > On top of that, bug 880918 is
> > again only relevant for buttons. For custom widgets, there may not even be a
> > label.
> 
> If there isn't a keyboard shortcut to accomplish what such widgets do
> already (such as in the case of the zoom controls which, technically, don't
> have a label, even if their image is really sort of text...), I think that's
> not OK, and we should fix such widgets (or, add-on authors would have to).
> There is a separate bug on file about exposing the keyboard shortcuts for
> the individual items.

We were talking about access keys (for interacting with the panel), not keyboard shortcuts (for invoking commands without the panel -- something that isn't going to be available for all toolbar widgets).

> Note that add-ons do already add regular menuitems without accesskeys, or
> with accesskeys that conflict. I really don't think this alone is a reason
> to forgo having keyboard access here.

Missing access keys is sometimes a feature, like in the Bookmarks and History menus, and sometimes it's just not a problem, because if you're lucky, you can use a menu item label's first letter as the implied access key. Other times it's indeed a problem that's unfortunately hard to solve, but of course that's no reason to create more problems that are hard to solve. My argument wasn't that people may forget to add access keys or that there may be conflicts, but that custom widgets may not even have a label where an access key could be exposed.

Comment 30

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Gijs Kruitbosch from comment #26)

> (Mike, needinfo'ing you about the bookmarks button not opening a subview -
> did I miss something?)

It's strange that the keyboard opened the popup instead of a subview. Yes, that's a bug. But I suppose its priority is conditioned on the decision on keyboard access made in this bug.

Comment 31

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #29)
> (In reply to :Gijs Kruitbosch from comment #28)
> > You're conflating two issues. The argument for having this is to provide
> > mouse users and keyboard users an ability to use the same route through the
> > UI as much as possible.
> 
> I wasn't aware of or forgot about you referring to this principle. Where is
> this from? It seems pretty bogus to me.

Apologies. I did bring this up in the call, and it was originally mentioned to me by Marco, when applied to AT / non-AT users. "Open the menupanel" (or, right now, "Open the Firefox menu") isn't something they could do, AIUI. The same goes for keyboard users. Conversely, if the menu is hidden, for mouse users it is hard to discover how they're supposed to get it back (someone boasted about having 'managed' to put it back, on our user research sessions...).

> Mouse and keyboard users don't
> generally go through the same routes and it's not clear to me why they
> should. Think of toolbar buttons as a prime example. What works well for one
> group may make no sense at all or at least be inefficient for the other.

I don't buy your first assertion. Toolbar buttons aren't focusable; the menubar, in cases where it is invisible by default, is highly inaccessible to mouse users. Users often *can't* go through the same route right now. The menubar, when it is visible, is actually one of the few things both "types" of users use. (although I don't think the divide is that strong, but for the purpose of this argument let's leave it)

> The menu bar is basically hidden and not mouse-accessible for most of our
> users. Users can enable it, but then it's just a traditional menu bar with a
> straightforward mouse-interaction model. I don't think we'd invest in
> providing a mouse-accessible menu bar, but we basically get it for free.

I'm not sure about our current usage numbers, but the menubar won't be hidden on XP or OS X by default, and our recent user research showed quite some users had put it back even on later versions of Windows where we'd hidden it in favour of the Firefox button (do we have FHR/telemetry on this? Would be helpful, perhaps). If we didn't have a mouse-accessible menu bar, a couple of features wouldn't be mouse-accessible at all, I think? (the view menu, for instance, seems to be missing from the Firefox menu)

> I didn't mean to advocate for the first-run page idea. I actually don't
> think it's a good one. It would only target a tiny minority and we have no
> reason to believe that most users in that minority actually read the
> first-run page. I tooltip wouldn't quite cut it either for keyboard-focused
> users either. My point was, if we think this is a worthwhile and important
> feature, we should expose it properly, whatever that means.

Sure, although I was talking about the first-run experiments Blair and UX are working on... I think they may be more, how to say, ... attention-grabbing (?) than the in-tab first-run page. In any case, again, separate discussion. :-)

> > You're assuming that just because it's not trivial, that makes it a
> > weakness. Arguments are fine, stop energy is not.
> 
> I didn't say "not trivial" but "complex", as in: less straightforward,
> harder to predict and understand. I do think such an interaction model is a
> weak one.

Clearly, a less straightforward, harder to predict and understand interaction model is weaker than a model that is less so. We disagree about how much (less straightforward, harder to predict and understand - and therefore...) weaker this model is, and if it's sufficiently weak that there's no sense in investigating having it at all. I think we should iterate and find out if we can make it good enough that we can keep it. I have the impression you feel it is never going to work well anyway and trying is a waste of time. Is that a fair characterization?

> In the browser, toolbar widgets generally cannot be focused by
> default, with the exception of text fields, so the burden on add-on authors
> would be significantly higher than it is for web content authors.

I don't understand what you mean here, and how it impacts the debate on whether or not we should be doing this. Sorry - can you elaborate?

> > > Secondly, if we already say this is kind of redundant and keyboard users
> > > (already a minority) may not use it,
> > 
> > My point wasn't that it shouldn't be used, it's that while we're iterating,
> > it isn't in the way of anything.
> 
> So when you said we wouldn't be forcing people to use this, you were
> referring to development cycles rather than when we'd release it? 

Yes. (although I don't plan on literally 'forcing' them to use it when released either, until we make an actual decision to remove the ordinary menubar, which I don't expect to happen in the Australis v1 timeframe)


> > > and if add-on authors add relevant
> > > hooks to the menu bar (like they already should)
> > 
> > But don't always. Also not an AMO requirement.
> 
> Maybe we should make it one? The good news is that the SDK could provide a
> sane API for this, if it doesn't already.

It's not trivial. Add-on localization is a little messy, and if you add an accesskey, I can pretty much guarantee you that it'll conflict with some existing accesskey in a non-English locale. Anyway, that's the subject of a separate bug. Either of us can file it, if you do, please feel free to CC me; it sounds like this is something about we should at least think more.

> > > On top of that, bug 880918 is
> > > again only relevant for buttons. For custom widgets, there may not even be a
> > > label.
> > 
> > If there isn't a keyboard shortcut to accomplish what such widgets do
> > already (such as in the case of the zoom controls which, technically, don't
> > have a label, even if their image is really sort of text...), I think that's
> > not OK, and we should fix such widgets (or, add-on authors would have to).
> > There is a separate bug on file about exposing the keyboard shortcuts for
> > the individual items.
> 
> We were talking about access keys (for interacting with the panel), not
> keyboard shortcuts (for invoking commands without the panel -- something
> that isn't going to be available for all toolbar widgets).

Sure, but I think that if you can use a keyboard shortcut to do what the custom widget does, it is OK if it does not have an access key, as the keyboard shortcut would be faster than opening the panel and so on anyway. For that argument to work, the shortcut should be discoverable. Hence my bringing that up.

> > Note that add-ons do already add regular menuitems without accesskeys, or
> > with accesskeys that conflict. I really don't think this alone is a reason
> > to forgo having keyboard access here.
> 
> Missing access keys is sometimes a feature, like in the Bookmarks and
> History menus, and sometimes it's just not a problem, because if you're
> lucky, you can use a menu item label's first letter as the implied access
> key. Other times it's indeed a problem that's unfortunately hard to solve,
> but of course that's no reason to create more problems that are hard to
> solve. My argument wasn't that people may forget to add access keys or that
> there may be conflicts, but that custom widgets may not even have a label
> where an access key could be exposed.

I agree that it's not a trivial problem. I replied to the non-labeled concern above (ie, things should have an accesskeyed label unless there are discoverable keyboard shortcuts). I'll note briefly that this lack of accesskeys and items' first letters being used instead actually lessens the predictability and straightforwardness of the accesskey work in menus, as those letters lack a visual affordance that tells you you're going to be able to use that item by hitting that letter. Not great. (but hey, platform convention, what can you do)

Comment 32

[:Gijs (he/him)]

I'm unsure why this was marked fixed-in-ux; there is ongoing discussion + work to be done if we decide that we want this. If you think that needs follow up bugs, they should be filed and stuck in the dep tree for this bug.

Comment 34

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/2b7346629b68
https://hg.mozilla.org/mozilla-central/rev/539f516783b0
https://hg.mozilla.org/mozilla-central/rev/3e84d97864bd