Last Comment Bug 625325 - Reorder some items in the Bookmarks menu
: Reorder some items in the Bookmarks menu
Status: VERIFIED FIXED
: user-doc-needed
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: Firefox 6
Assigned To: Steffen Wilberg
:
Mentors:
: 624663 (view as bug list)
Depends on: 661250
Blocks: 607226
  Show dependency treegraph
 
Reported: 2011-01-13 03:07 PST by Alex Faaborg [:faaborg] (Firefox UX)
Modified: 2011-08-20 02:55 PDT (History)
9 users (show)
hskupin: in‑litmus?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (4.22 KB, patch)
2011-02-05 13:20 PST, Steffen Wilberg
dao+bmo: review-
Details | Diff | Review
patch v2 (5.71 KB, patch)
2011-03-31 12:16 PDT, Steffen Wilberg
dao+bmo: review+
Details | Diff | Review

Description Alex Faaborg [:faaborg] (Firefox UX) 2011-01-13 03:07:58 PST
For consistency with the organization of the Firefox button and navigation toolbr bookmarks menu control, the traditional bookmarks menu should be slightly reorganized on all platforms:

Show All Bookmarks
----------------------
Bookmark This Page
Subscribe to This Page
----------------------
Bookmarks Toolbar
----------------------
[User's bookmarks menu items]
----------------------
Unsorted Bookmarks <--(added, opens the library window)

The command "Bookmark all Tabs" is being removed over in bug 607227
Comment 1 Pim Schellart 2011-01-13 03:45:56 PST
Just wondering if it is not possible/preferable to have "Unsorted Bookmarks" have dual function.
- When clicked it opens the library window.
- When hovering it displays the contents of unsorted bookmarks just like any other menu entry.

It would be even better if one could immediately drag bookmarks from the unsorted bookmarks entry to other folders.
Comment 2 Siddhartha Dugar [:sdrocking] 2011-01-14 08:51:19 PST
(In reply to comment #1)
> Just wondering if it is not possible/preferable to have "Unsorted Bookmarks"
> have dual function.
> - When clicked it opens the library window.
> - When hovering it displays the contents of unsorted bookmarks just like any
> other menu entry.

This should be same as the split menu used for "Bookmarks" in the main Firefox button menu.
Comment 3 Alex Faaborg [:faaborg] (Firefox UX) 2011-01-18 15:45:31 PST
>- When hovering it displays the contents of unsorted bookmarks just like any
>other menu entry.

I've filed follow up bug 626852 for that
Comment 4 Luke Iliffe (Harlequin99) 2011-01-19 12:25:44 PST
*** Bug 624663 has been marked as a duplicate of this bug. ***
Comment 5 Julo 2011-01-20 12:47:41 PST
I've just open bug 625325 about toolbar bookmark menu as didn't find this one. 

Main idea there was saving space before 1st real bookmark as from Firefox 4 many users will use browser without bookmark toolbar. This means they will access bookmarks via url bar suggestions if they remember part or url (or content) or via toolbar bookmark menu. As I think they will more frequently look for bookmarks in 2nd case and not for bookmark management functionality (well, in fact my experience) I consider current layout not ideal.

Currently before 1st real bookmark entry there are 5 fixed menu items and 4 separators. I would extract just following minor suggestions from bug 625325:
 - hide "Subscribe to This Page" if no feed is available to subscribe (currentlly such item is disabled, just taking space)
 - remove 1 separator between "Show All Bookmarks" and "Bookmark This Page" as in Firefox v3.6

(I would maybe personally move 3 of them to own "Organize" submenu or allow users to remove from toolbar button, but this would be probably too much).
Comment 6 Julo 2011-01-20 12:49:10 PST
(In reply to comment #5)
> I've just open bug 625325 about toolbar bookmark menu as didn't find this one. 
correction: this is bug 627325
Comment 7 Alex Faaborg [:faaborg] (Firefox UX) 2011-01-24 11:26:01 PST
The UX team is very eager to get this bug landed over the next few days in order to make Beta 11.  If anyone can get a patch for this bug posted soon, we will push hard for reviews and approval (even though this isn't blocking).

You can view all of the related bugs to clean up the traditional menu bar here: http://areweprettyyet.com/4/traditionalMenu/
Comment 8 Steffen Wilberg 2011-02-05 13:20:44 PST
Created attachment 510045 [details] [diff] [review]
patch v1

Implements comment 0. Code for the "Unsorted Bookmarks" menu item is copied from browser-appmenu.inc (except the id).

I'm leaving making "Unsorted Bookmarks" a split-menu to bug 626852, as that needs to happen in the app menu in menu button as well, and I'm not convinced of that idea yet.
Comment 9 Dão Gottwald [:dao] 2011-03-26 09:24:16 PDT
Comment on attachment 510045 [details] [diff] [review]
patch v1

>+      <menuseparator id="beforeBookmarksToolbarFolderSeparator"/>

>+      <menuseparator id="beforeBookmarksMenuItemsSeparator"/>

I slightly dislike these ids...

>+      <menuitem id="unsortedBookmarks"

This needs a more distinctive id, e.g. menu_unsortedBookmarks.

>+                label="&appMenuUnsorted.label;"

Should either add a new entity or rename this one so that it doesn't look appmenu-specific.

>+                class="menuitem-iconic"/>

Why? You're not setting an icon.
Comment 10 Steffen Wilberg 2011-03-31 12:16:17 PDT
Created attachment 523382 [details] [diff] [review]
patch v2

"bookmarksToolbarSeparator" and "bookmarksMenuItemsSeparator",
"menu_unsortedBookmarks",
unsortedBookmarksCmd.label
Comment 11 Steffen Wilberg 2011-03-31 12:17:40 PDT
I also removed the unused bookmarkAllCmd.label:
http://mxr.mozilla.org/mozilla-central/search?string=bookmarkAllCmd.label
Comment 12 [not reading bugmail] 2011-03-31 13:25:11 PDT
So that is not used on the tab context menu to Bookmark All Tabs...?
Comment 13 Steffen Wilberg 2011-03-31 13:30:14 PDT
> So that is not used on the tab context menu to Bookmark All Tabs...?
No, that's bookmarkAllTabs.label:
http://mxr.mozilla.org/mozilla-central/search?string=bookmarkAllTabs.label

We also have addCurPagesCmd.label, which is used in the Bookmarks menu:
http://mxr.mozilla.org/mozilla-central/search?string=addCurPagesCmd.label
Comment 15 Henrik Skupin (:whimboo) 2011-05-02 01:15:12 PDT
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110501 Firefox/6.0a1

The Litmus test will have to be updated when we have a branch for Firefox 6.
Comment 16 Simona B [:simonab] 2011-05-06 05:19:12 PDT
(In reply to comment #0)

> The command "Bookmark all Tabs" is being removed over in bug 607227

Logged bug 655239 as the option is still present on Mac.

Note You need to log in before you can comment on or make changes to this bug.