Last Comment Bug 586056 - Unsorted Bookmarks entry in menu of Bookmarks button in Personal Toolbar has no special icon
: Unsorted Bookmarks entry in menu of Bookmarks button in Personal Toolbar has ...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- minor (vote)
: seamonkey2.1b1
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
:
Mentors:
Depends on: SMPlacesBMarks
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-10 13:11 PDT by Jens Hatlak (:InvisibleSmiley)
Modified: 2010-08-21 23:22 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
add icon definition [Checkin: comment 10] (2.78 KB, patch)
2010-08-15 14:51 PDT, Jens Hatlak (:InvisibleSmiley)
neil: review+
neil: superreview+
Details | Diff | Splinter Review

Description Jens Hatlak (:InvisibleSmiley) 2010-08-10 13:11:22 PDT
Now with Places-based bookmarks, the Unsorted Bookmarks entry in the Bookmarks Manager has a special icon, but the same entry in the menu of the Bookmarks button in the Personal Toolbar only shows the standard folder icon.
Comment 1 Jens Hatlak (:InvisibleSmiley) 2010-08-15 14:51:53 PDT
Created attachment 466170 [details] [diff] [review]
add icon definition [Checkin: comment 10]

AFAICS the icon definition for Unsorted Bookmarks inside the Bookmarks Manager is #BMB_unsortedBookmarksFolderMenu:
http://mxr.mozilla.org/comm-central/source/mozilla/browser/themes/winstripe/browser/browser.css#1654

OTOH, our icon definition for Personal Toolbar inside the drop-down menu of the Bookmarks button is #BMB_bookmarksToolbarFolderMenu:
http://mxr.mozilla.org/comm-central/source/suite/themes/classic/communicator/bookmarks/bookmarks.css#105

In the latter file there is already a line for #unsortedBookmarksFolderMenu which is just missing the extra #BMB_bookmarksToolbarFolderMenu.

Requesting 2.1a3 approval for simple and safe fix for a new feature (UX polish).

BTW: Do we have a bug somewhere to find a common name for "Bookmarks Toolbar" in the Bookmarks Manager vs. "Personal Toolbar" in the button drop-down menu?
Comment 2 Ian Neal 2010-08-15 15:09:48 PDT
Comment on attachment 466170 [details] [diff] [review]
add icon definition [Checkin: comment 10]

Too late for a plus patch has not been reviewed yet.
Comment 3 neil@parkwaycc.co.uk 2010-08-15 15:12:08 PDT
Why do we only have Unsorted Bookmarks in the Bookmarks button and not the menu?
Comment 4 Jens Hatlak (:InvisibleSmiley) 2010-08-15 22:51:27 PDT
(In reply to comment #3)
> Why do we only have Unsorted Bookmarks in the Bookmarks button and not the
> menu?

[Just in case anyone thought I could answer that: ] Sorry, no idea, I didn't follow the Places-Bookmarks implementation that closely. Would be another bug anyway.
Comment 5 Jens Hatlak (:InvisibleSmiley) 2010-08-17 14:37:54 PDT
(In reply to comment #1)
> BTW: Do we have a bug somewhere to find a common name for "Bookmarks Toolbar"
> in the Bookmarks Manager vs. "Personal Toolbar" in the button drop-down menu?

Filed bug 588191.

KaiRo, can you enlighten us why Unsorted Bookmarks is missing from the menu as Neil noted? Should we file a bug for that as well?
Comment 6 Robert Kaiser 2010-08-17 17:57:01 PDT
(In reply to comment #5)
> KaiRo, can you enlighten us why Unsorted Bookmarks is missing from the menu as
> Neil noted? Should we file a bug for that as well?

I probably just copied some logic from Firefox bookmarks toolbar button and bookmarks menu. I'm not sure if it's really needed/wanted in either on our side or none, but you are right that we probably should make it consistent.
Comment 7 Jens Hatlak (:InvisibleSmiley) 2010-08-19 10:06:39 PDT
(In reply to comment #6)
> I'm not sure if it's really needed/wanted in either on our side
> or none, but you are right that we probably should make it consistent.

Filed bug 588807.
Comment 8 neil@parkwaycc.co.uk 2010-08-21 15:54:05 PDT
Comment on attachment 466170 [details] [diff] [review]
add icon definition [Checkin: comment 10]

Sorry for the delay on this.

There's no reason for us to penalise keyboard users; we should totally have the unsorted bookmarks folder accessible from the main menu too.
Comment 9 Jens Hatlak (:InvisibleSmiley) 2010-08-21 23:16:07 PDT
(In reply to comment #8)
> There's no reason for us to penalise keyboard users; we should totally have the
> unsorted bookmarks folder accessible from the main menu too.

I assume that you confused this bug with bug 588807 (see comment 2 there) when you wrote that but that the reviews still apply to this patch, right? ;-)
Comment 10 Jens Hatlak (:InvisibleSmiley) 2010-08-21 23:21:32 PDT
Comment on attachment 466170 [details] [diff] [review]
add icon definition [Checkin: comment 10]

http://hg.mozilla.org/comm-central/rev/c2aa1b4c38d6

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