Closed Bug 870545 Opened 7 years ago Closed 6 years ago

Prevent hiding the nav-bar from the context menu / toolbar menu

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 28

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 2 obsolete files)

We shouldn't let users accidentally hide the navigation bar.
Blocks: australis
No longer blocks: 860814
Attached patch Patch (obsolete) — Splinter Review
Something like this?

I think we should probably have a further migration blip to add the non-removable UI items if they're not currently in the navbar, which will prevent the toolbar being completely empty or something of that sort, but I'd argue we shouldn't do that until we've made them non-removable.
Attachment #749205 - Flags: review?(jaws)
Status: NEW → ASSIGNED
Comment on attachment 749205 [details] [diff] [review]
Patch

As far as I can tell, you just need to drop the toolbarname attribute.
Attachment #749205 - Flags: review-
(In reply to Dão Gottwald [:dao] from comment #2)
> Comment on attachment 749205 [details] [diff] [review]
> Patch
> 
> As far as I can tell, you just need to drop the toolbarname attribute.

As far as I could tell, I can't because it's used in various other places as well (eg. the customizeToolbar toolkit code (do we still need that?), and accessibility).
(In reply to :Gijs Kruitbosch from comment #3)
> (In reply to Dão Gottwald [:dao] from comment #2)
> > Comment on attachment 749205 [details] [diff] [review]
> > Patch
> > 
> > As far as I can tell, you just need to drop the toolbarname attribute.
> 
> As far as I could tell, I can't because it's used in various other places as
> well (eg. the customizeToolbar toolkit code (do we still need that?), and
> accessibility).

That it's used doesn't mean it's needed. Accessibility code just exposes the name, but there's no requirement that all toolbars should have names. Toolkit doesn't require the toolbarname attribute either. According to <https://developer.mozilla.org/en-US/docs/XUL/Attribute/toolbarname>, the primary point of the attribute is for listing the toolbar in menus.
(In reply to Dão Gottwald [:dao] from comment #4)
> (In reply to :Gijs Kruitbosch from comment #3)
> > (In reply to Dão Gottwald [:dao] from comment #2)
> > > Comment on attachment 749205 [details] [diff] [review]
> > > Patch
> > > 
> > > As far as I can tell, you just need to drop the toolbarname attribute.
> > 
> > As far as I could tell, I can't because it's used in various other places as
> > well (eg. the customizeToolbar toolkit code (do we still need that?), and
> > accessibility).
> 
> That it's used doesn't mean it's needed. Accessibility code just exposes the
> name, but there's no requirement that all toolbars should have names.
> Toolkit doesn't require the toolbarname attribute either. According to
> <https://developer.mozilla.org/en-US/docs/XUL/Attribute/toolbarname>, the
> primary point of the attribute is for listing the toolbar in menus.

I asked in #accessibility, and they told me the exposure of the name is important once there is more than one toolbar:

<Gijs> So how important is exposing toolbar names for a11y?
<Gijs> do screenreaders or other tools announce/use those, and do users rely on them when they move around the UI using accessibility tools?
<marcoz> Gijs: Hi! The toolbar names are important if there is more than one of them. You can have a general toolbar, a Formatting toolbar, a Utilities toolbar, for example. In MS Office, esp with the ribbons, which map to toolbars, it is quite important to know which one the user is moving through.
<Gijs> marcoz: so if we were to get rid of the toolbarname for the navigation toolbar (with back/forward/url field/search box, etc.), that would be a problem?
<marcoz> I think of toolbars simply as grouping containers similar to groupbox elements in XUL or fieldset/legend elements in HTML, only that here, the items are of more similar types than in regular groupings.
<marcoz> Gijs: Yes, the names are the differenciators. It is useful to know when transitioning from the navigation toolbar to the tabs toolbar, for example.
<marcoz> Gijs: When you throw the Social toolbars in there, things become more involved even. You'll want to know as a user which toolbar you're navigating into. Sighted people often have different indicators, and screen reader users need some, too, esp in complex UIs.

So I don't think removing it outright is the right solution. Nevertheless, here is a patch that does that instead.

If the first patch I provided is not deemed the Right fix, we may instead want to separate the provisioning of the a11y information and the decision of whether to show this in a menu in some other way, eg. by having a separate attribute for the latter.

Finally, I realized just now that both of these patches remove the context menu from the nav-bar and I didn't mention that before. I figured it didn't make sense to have this context menu (which shows up intermittently anyway as 90% of the space is taken up by elements with their own menus) on a toolbar that you then couldn't remove using that menu, but I recognize there's room for disagreement on that front...
Attachment #749252 - Flags: review?(jaws)
According to DOM Inspector's Application Accessible, if we drop the toolbarname attribute we can use aria-label to still set the "Navigation Toolbar" name for accessibility purposes.

You can then keep the context="toolbar-context-menu" attribute so that the context menu will be shown when users right-click on the toolbar. I'd like to keep this behavior since users have learnt this over time and it's common muscle memory by now.
Attached patch PatchSplinter Review
Thanks Jared, that's a good point. Updated patch attached...
Attachment #749205 - Attachment is obsolete: true
Attachment #749252 - Attachment is obsolete: true
Attachment #749330 - Flags: review?(jaws)
Attachment #749330 - Flags: review?(jaws) → review+
http://hg.mozilla.org/projects/ux/rev/56db3a7a5f94
Whiteboard: [fixed-in-ux]
Blocks: 851704
No longer blocks: 851704
Duplicate of this bug: 851704
Depends on: 879678
https://hg.mozilla.org/mozilla-central/rev/56db3a7a5f94
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-ux]
Target Milestone: --- → Firefox 28
Blocks: 941377
(In reply to Jared Wein [:jaws] from comment #6)
...
> context menu will be shown when users right-click on the toolbar. I'd like
> to keep this behavior since users have learnt this over time and it's common
> muscle memory by now.

Yes! Keep this function! It's more than 'muscle memory', it is !Important to gain page real estate when designing, reading, viewing Any page... Less chrome, toolbar, menu and More 'page'.. 

Please leave, put back the 'Option' to us, users.

Thank You.
Landis.
Keywords: verifyme
Verified as fixed on the 02/02 Nightly. Can't be verified on Aurora since Australis was backed out there.
Status: RESOLVED → VERIFIED
Keywords: verifyme
bug 1083721 request to reintroduce the capability to disable Navigation bar.
You need to log in before you can comment on or make changes to this bug.