Closed Bug 571782 Opened 11 years ago Closed 11 years ago

Make "New Tab" a split menu button that includes "New Tab" and "New Window" options

Categories

(Firefox :: Menus, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 4.0b4

People

(Reporter: u88484, Assigned: u88484)

References

Details

Attachments

(4 files, 2 obsolete files)

The first entry in the new Firefox menu button is "New Window" although quite a few bugs have been filed and fixed or filed and have patches waiting review in the few years to promote tabs (e.g. tab related menu buttons being listed before windows, tab related preferences being listed before new windows preferences).

The same should be for Firefox menu with having "New Tab" as the first menu entry and I would suggest not even having a "New Window" entry at all.
I am also with the Bug Filer , its 2010 anyways , and IE6 share is declining!
On a default UI setup, there is no need to have the new tab option on the menu, because it's already there on the GUI. That said, I believe menus should be redundant with the GUI and maybe we'll want to account for those users who DON'T have the new tab button on their GUIs anyway (for whatever reason).
We are trying very very hard to eliminate redundancy.  Any control that gets a main window button doesn't also have a menu entry (with I believe the singular exception of bookmarks).
(In reply to comment #3)
> We are trying very very hard to eliminate redundancy.  Any control that gets a
> main window button doesn't also have a menu entry (with I believe the singular
> exception of bookmarks).

Makes sense but the new tab menu entry is currently really high on the list of common menu actions (http://blog.mozilla.com/metrics/2010/04/14/menu-item-usage-study-an-update-to-the-initial-analysis/).  I don't think there is any data after the introduction of the new tab button on the tab bar though.  Maybe a short test pilot test is needed and wouldn't be that hard to put together quickly.
I believe that this is primarily due to the fact that we didn't have a new tab button for several releases, and people built up common paths.  Now this will admittedly break that path, but now is really the best time since we are making a number of changes, and clicking the new tab button is faster than going into a menu to get the same functionality.
also of course people who really want to maintain the same UI paths will likely switch to the legacy menu.
-> wontfix
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WONTFIX
(In reply to comment #5)
> I believe that this is primarily due to the fact that we didn't have a new tab
> button for several releases, and people built up common paths.  Now this will
> admittedly break that path, but now is really the best time since we are making
> a number of changes, and clicking the new tab button is faster than going into
> a menu to get the same functionality.

I see in your blog post (http://blog.mozilla.com/faaborg/2010/07/18/details-about-the-firefox-button/) in this screenshot (http://people.mozilla.com/~faaborg/files/20100718-firefoxButtonDetails/firefoxButtonHeatmap-i4.png) that the 'new tab' entry to available.  Do you want to reopen this bug or should I create a fresh clean bug?
Reopened (as Kurt notes I changed my mind on redundancy vs. breaking people's paths)
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Feel free to update the summary
Assignee: nobody → supernova00
Status: REOPENED → ASSIGNED
Summary: Promote tabs (not windows) in the Firefox menu button → Add a 'New' split menu button to the Firefox button that includes "New Tab" and "New Window" options
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #460032 - Flags: ui-review?(faaborg)
Attachment #460032 - Flags: review?(dao)
Attached patch Patch v2 (obsolete) — Splinter Review
This patch will also need l10n review at some point.
Attachment #460032 - Attachment is obsolete: true
Attachment #460033 - Flags: ui-review?(faaborg)
Attachment #460033 - Flags: review?(dao)
Attachment #460032 - Flags: ui-review?(faaborg)
Attachment #460032 - Flags: review?(dao)
could you attach a screen shot for quick ui-r
"New" at the top of the hierarchy seems strange. (Because it's not a noun?)
"New" as an entry point makes a lot of sense for traditional text editors where the superordinate term would be "File", but we don't have that here. "New" also usually doesn't act as a command there but merely spawns a sub menu. Dunno if this changed in recent MS Office releases.
>"New" also usually doesn't act as a command there but merely spawns a sub menu.

idea was for it to create a new tab when hit (similar to how "print" is both an action and a thing that contains a sub menu).  Unfortunately if we fully qualify the name, it doesn't make sense to group new window inside of it.  In the mockup, the first action for each sub-menu is carried out when you hit the main item

New
Start private browsing
Print
Customize (loads add-ons manager)
Sync (starts a sync)
etc.
(In reply to comment #13)
> could you attach a screen shot for quick ui-r

I'll give it a try as I've never built my own builds before.  Although as easy as this patch is, I could just hack my nightly build with the changes.
Attachment #460720 - Flags: ui-review?(faaborg)
Any reason why this submenu is slightly lower than the edge of the parent menu, can it be lined up?
(In reply to comment #18)
> Any reason why this submenu is slightly lower than the edge of the parent menu,
> can it be lined up?

Hmm, it is three pixels lower (along with the text not lining up like with most submenus) but that is for another bug.  Print's also doesn't line up but Customize and History's do.
Can New Tab be used instead of New? This would be consistent with Start Private Browsing -> Start Private Browsing | Clear Recent History. I'm not sure the sub menu shouldn't repeat the top-level item in that case, though.
>Can New Tab be used instead of New?

yeah, I couldn't really decide which approach made more sense.  I'm now leaning towards more literal top menu labels, even though they contain sub menu commands. So for instance "addo-ons" instead of "customize" (with moving the toolbar customization over to options), etc.
Comment on attachment 460720 [details]
Screenshot with patch v2 changes applied

this is fine, two caveats that we can either fix now or later are switching the main item text to "New Tab" and displaying the keyboard shortcuts in the submenu.
Attachment #460720 - Flags: ui-review?(faaborg) → ui-review+
If we can get this landed quickly that would be ideal, otherwise we'll make these changes over in bug 583386
Attached patch Patch v3Splinter Review
(In reply to comment #22)
> Comment on attachment 460720 [details]
> Screenshot with patch v2 changes applied
> 
> this is fine, two caveats that we can either fix now or later are switching the
> main item text to "New Tab" and displaying the keyboard shortcuts in the
> submenu.

Switched to "New Tab".
Attachment #460033 - Attachment is obsolete: true
Attachment #461716 - Flags: review?(dao)
Attachment #460033 - Flags: ui-review?(faaborg)
Attachment #460033 - Flags: review?(dao)
Comment on attachment 461716 [details] [diff] [review]
Patch v3

>-      <menupopup id="appmenu-popup">
>-        <menuitem id="appmenu_newNavigator"
>-                  label="&newNavigatorCmd.label;"
>-                  command="cmd_newNavigator"/>
>+      <menupopup id="appmenu-popup">	  

You're adding spaces and tabs at the end of this line. Don't do this.

>+        <hbox flex="1" class="split-menuitem">
>+          <menuitem id="appmenu_new"

Please change this id to appmenu_newTab.

>+                    class="split-menuitem-item"
>+                    flex="1"
>+                    label="&appMenuNewCmd.label;"

Use &tabCmd.label; here, don't add appMenuNewCmd.label to the dtd.

>+                    command="cmd_newNavigatorTab"/>
>+          <menu class="split-menuitem-menu">
>+            <menupopup>
>+              <menuitem id="appmenu_newTab"

I'd propose appmenu_newTab_sub as the id here, although I still think New Tab -> New Tab is strange and we should avoid repeating the item in the sub menu.
Attachment #461716 - Flags: review?(dao) → review+
Attached patch Patch v4Splinter Review
(In reply to comment #25)
> Comment on attachment 461716 [details] [diff] [review]
> Patch v3
> 
> >-      <menupopup id="appmenu-popup">
> >-        <menuitem id="appmenu_newNavigator"
> >-                  label="&newNavigatorCmd.label;"
> >-                  command="cmd_newNavigator"/>
> >+      <menupopup id="appmenu-popup">	  
> 
> You're adding spaces and tabs at the end of this line. Don't do this.
> 
Removed.

> >+        <hbox flex="1" class="split-menuitem">
> >+          <menuitem id="appmenu_new"
> 
> Please change this id to appmenu_newTab.
> 
Done.

> >+                    class="split-menuitem-item"
> >+                    flex="1"
> >+                    label="&appMenuNewCmd.label;"
> 
> Use &tabCmd.label; here, don't add appMenuNewCmd.label to the dtd.
>
Done.

> >+                    command="cmd_newNavigatorTab"/>
> >+          <menu class="split-menuitem-menu">
> >+            <menupopup>
> >+              <menuitem id="appmenu_newTab"
> 
> I'd propose appmenu_newTab_sub as the id here, although I still think New Tab
> -> New Tab is strange and we should avoid repeating the item in the sub menu.

Done.  I agree but I can't think of anything else to propose that makes sense.  Calling it just "New" and having it open a new tab upon clicking seems weird since new doesn't describe the exact command that will be run once clicked.

I assume I can carry the review flag over so I'm requesting approval2.0?
Attachment #461785 - Flags: approval2.0?
Attachment #461785 - Flags: approval2.0? → approval2.0+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e601f6dcd81a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Summary: Add a 'New' split menu button to the Firefox button that includes "New Tab" and "New Window" options → Make "New Tab" a split menu button that includes "New Tab" and "New Window" options
Target Milestone: --- → Firefox 4.0b4
Verified fixed with Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre

Do we have any automated tests for those entries or at all for the Firefox button menu?
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Flags: in-litmus?
In javascript popups, "New Tab" in the root menu is not disabled, while in the submenue it is disabled. I guess the second is correct, because tabs that are opened are not displayed in the popup.
(In reply to comment #28)
> Verified fixed with Mozilla/5.0 (Windows NT 6.1; rv:2.0b4pre) Gecko/20100810
> Minefield/4.0b4pre
> 
> Do we have any automated tests for those entries or at all for the Firefox
> button menu?
No tests were added or changed in bug 570795 that originally implemented this app button and same for this bug (so far unless someone else adds them).

(In reply to comment #29)
> In javascript popups, "New Tab" in the root menu is not disabled, while in the
> submenue it is disabled. I guess the second is correct, because tabs that are
> opened are not displayed in the popup.
Can you file a new bug about that so someone can decide what to do about it.
Depends on: 586212
The text of the "New Tab" menu item is inconsistent with the other split-menuitem, "Print".  Print doesn't appear in the submenu of itself, so I don't see why New Tab does.  Part of the problem is the behavior of split-menuitem.  You're assuming that people will know that they can click the label of a menu and an action will be performed.  I've worked in menu code on Windows for years, and I've never seen something like this before.
Windows 7 has it on the start menu. I think MS Office has it too.

Although I do think "Print" should be part of the "Print" menu too, just like "New Tab" is part of the "New Tab" menu. But it's not like it matters much to me.
(In reply to comment #31)
> The text of the "New Tab" menu item is inconsistent with the other
> split-menuitem, "Print".  Print doesn't appear in the submenu of itself, so I
> don't see why New Tab does.  Part of the problem is the behavior of
> split-menuitem.  You're assuming that people will know that they can click the
> label of a menu and an action will be performed.  I've worked in menu code on
> Windows for years, and I've never seen something like this before.

(In reply to comment #32)
> Windows 7 has it on the start menu. I think MS Office has it too.
> 
> Although I do think "Print" should be part of the "Print" menu too, just like
> "New Tab" is part of the "New Tab" menu. But it's not like it matters much to
> me.

That will be fixed in bug 583386
(In reply to comment #32)
> Windows 7 has it on the start menu. I think MS Office has it too.
Although in Windows 7, hovering over any part of the menu item will open the submenu, whereas you have to hover over the arrow in the current Firefox nightlies. (Is there a bug to change that behavior?)
You need to log in before you can comment on or make changes to this bug.