Closed Bug 625325 Opened 13 years ago Closed 13 years ago

Reorder some items in the Bookmarks menu

Categories

(Firefox :: Menus, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 6

People

(Reporter: faaborg, Assigned: steffen.wilberg)

References

(Blocks 1 open bug)

Details

(Keywords: user-doc-needed)

Attachments

(1 file, 1 obsolete file)

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
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.
(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.
>- When hovering it displays the contents of unsorted bookmarks just like any
>other menu entry.

I've filed follow up bug 626852 for that
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).
(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
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/
Attached patch patch v1 (obsolete) — Splinter Review
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.
Assignee: nobody → steffen.wilberg
Status: NEW → ASSIGNED
Attachment #510045 - Flags: review?(dao)
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.
Attachment #510045 - Flags: review?(dao) → review-
Attached patch patch v2Splinter Review
"bookmarksToolbarSeparator" and "bookmarksMenuItemsSeparator",
"menu_unsortedBookmarks",
unsortedBookmarksCmd.label
Attachment #510045 - Attachment is obsolete: true
Attachment #523382 - Flags: review?(dao)
I also removed the unused bookmarkAllCmd.label:
http://mxr.mozilla.org/mozilla-central/search?string=bookmarkAllCmd.label
So that is not used on the tab context menu to Bookmark All Tabs...?
> 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
Attachment #523382 - Flags: review?(dao) → review+
http://hg.mozilla.org/mozilla-central/rev/4bbc17c443c9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 6
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.
Status: RESOLVED → VERIFIED
Flags: in-litmus?
(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.
Depends on: 661250
Keywords: user-doc-needed
Depends on: 680295
No longer depends on: 680295
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: