Closed Bug 586056 Opened 10 years ago Closed 10 years ago

Unsorted Bookmarks entry in menu of Bookmarks button in Personal Toolbar has no special icon

Categories

(SeaMonkey :: Bookmarks & History, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b1

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

References

Details

Attachments

(1 file)

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.
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?
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #466170 - Flags: superreview?(neil)
Attachment #466170 - Flags: review?(neil)
Attachment #466170 - Flags: approval-seamonkey2.1a3?
Comment on attachment 466170 [details] [diff] [review]
add icon definition [Checkin: comment 10]

Too late for a plus patch has not been reviewed yet.
Attachment #466170 - Flags: approval-seamonkey2.1a3?
Why do we only have Unsorted Bookmarks in the Bookmarks button and not the menu?
(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.
(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?
(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.
(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 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.
Attachment #466170 - Flags: superreview?(neil)
Attachment #466170 - Flags: superreview+
Attachment #466170 - Flags: review?(neil)
Attachment #466170 - Flags: review+
(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 on attachment 466170 [details] [diff] [review]
add icon definition [Checkin: comment 10]

http://hg.mozilla.org/comm-central/rev/c2aa1b4c38d6
Attachment #466170 - Attachment description: add icon definition → add icon definition [Checkin: comment 10]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b1
You need to log in before you can comment on or make changes to this bug.