235300 - bookmarks toolbar items need an icon for the customize toolbar dialog

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, enhancement)

Description

[miahz]

User-Agent:       
Build Identifier: 

there is no icon for the personal bookmark toolbar items when in the customize
palette.

there is no indication that the bookmarks flex to fill the toolbar.

Reproducible: Always
Steps to Reproduce:
1. enter toolbar customize mode
2. drag bookmarks into palette
Actual Results:  
all you see is "Bookmarks" with no icon

Expected Results:  
an icon or some better visual representation of what the item is

Comment 1

[miahz]

Attachment: image illustrating bug, and potential remedies

1. current broken state in palette. no icon. same label as bookmarks sidebar
button.
2. fixed existing code. same label as bookmarks sidebar button. not apparent
that this is the personal bookmarks toolbar item.
3. restyled to use more descriptive label, with appearance more suggestive of
functionality.

4. current state on toolbar during customize. no indication of space taken by
the whole item.
5. how it would look restyled using same fix as "3" with borders showing size
of object similar to a flexible space.

Comment 2

[miahz]

Attachment: patch fixing existing code to show icon in palette

the code for the bookmark toolbar was already in
classic/skin/classic/browser/browser.css, it was just slighty incorrect.  this
one line patch corrects it as shown in "2" in the image in comment #1.

Comment 3

[miahz]

Attachment: patch restyling bookmarks toolbar items

this patch makes the changes as shown in "3" and "5" in the image attached to
comment #1.  it uses the same, more descriptive label in the palette as used on
the toolbar.  the standard "Bookmarks" palette item label is removed.  borders
are also added to illustrate the size of the bookmarks while on the toolbar.

i would recommend this patch be used.
- uses better label
- item appears similar both in palette and on toolbar
- eliminates potential confusion caused by this having the same label as the
bookmark sidebar button
- true size of item on toolbar is apparent with borders, which will help when
positioning items and should help prevent items from being "lost" when added
after the bookmarks

Comment 4

[Simon Paquet [:sipaq]]

Comment on attachment 142038 [details] [diff] [review]
patch fixing existing code to show icon in palette

Blake, do you like that approach?

Comment 5

[Simon Paquet [:sipaq]]

Comment on attachment 142040 [details] [diff] [review]
patch restyling bookmarks toolbar items

Blake, do you like that approach?

Comment 6

[Simon Paquet [:sipaq]]

Confirming. Assigning to miahz.

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 142040 [details] [diff] [review]
patch restyling bookmarks toolbar items

lets go ahead a file a separate bug about the styling against
[e-mail]

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 142038 [details] [diff] [review]
patch fixing existing code to show icon in palette

as noted, the styling should go to arvid for decisions, but in the absence of
that, lets get this in so we're not broken

Comment 9

[Mike Connor [:mconnor]]

checked in branch and trunk, thanks for the patch!

Please file a followup bug for Arvid to look at the styling here.

Comment 10

[Mike Connor [:mconnor]]

resolving FIXED, please note the followup bug here if/when filed.

Comment 11

[miahz]

thanks, mike.  something must be up with myrealbox email, cuz i haven't gotten
any mail in a couple days - i read about this checkin in the nightly build forum
thread.  i'll whip up the new bug as soon as i can.

Comment 12

[miahz]

It's broken again.  I just looked at it, and the fix is the exact same thing as
last time, though the patch might be off by a line or so.  How did the theme get
back-graded to where this was unfixed?

Needs to be reopened (i don't think i'm strong enough to change it).  And now
maybe the alternate second patch can be reconsidered.

Actually, just checked, and the bookmark folder icon image
(chrome://browser/skin/Bookmarks-folder.png) isn't in the theme anymore.  (Yet
another reason to go with the second approach - it uses the normal bookmark
icon.)  But that doesn't explain why the code backed up.

Comment 13

[Simon Paquet [:sipaq]]

*** Bug 206159 has been marked as a duplicate of this bug. ***

Comment 14

[Richard Song]

I can reproduce this bug in my system Windows XP. Here is my configuration:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3

Comment 15

[miahz]

This (along with a few other bugs) was tagged as fixed-aviary1.0, but is not
truly fixed.  browser/Bookmarks-folder.png was checked in, but it is corrupt or
something - it does not display and will not open in any image viewer/editor i
tried.

Still broken.

17th/Windows 2000 [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3)
Gecko/20041017 Firefox/1.0]

Comment 16

[Steffen Wilberg]

The icon is displayed just fine here:
http://lxr.mozilla.org/seamonkey/source/browser/themes/winstripe/browser/Bookmarks-folder.png
http://lxr.mozilla.org/seamonkey/source/browser/themes/pinstripe/browser/bookmarks/bookmark-folder.png

Marking fixed again.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

v.