Closed
Bug 566034
Opened 15 years ago
Closed 14 years ago
Favicons on OS X bookmarks toolbar
Categories
(Firefox :: Theme, defect)
Firefox
Theme
Tracking
()
RESOLVED
FIXED
Firefox 4.0b3
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta3+ |
People
(Reporter: jhill, Assigned: Dolske)
References
(Blocks 1 open bug)
Details
Attachments
(3 files, 2 obsolete files)
(deleted),
patch
|
dao
:
review+
mossop
:
approval2.0+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
limi
:
ui-review+
|
Details |
(deleted),
image/png
|
Details |
(Note: this is filed as part of the “Paper Cut” bugs — we assume that there may be multiple existing bugs on this. Please make them block this bug, and we will de-dupe if they are indeed exactly the same. Thanks!)
Recommendation:
People are very sad about this, and I agree
The bookmarks toolbar should have an option to display either text, icons, or both. The fact we have to jump through hoops (chrome.css etc) to set this is sad.
Updated•15 years ago
|
Updated•14 years ago
|
Component: General → Theme
QA Contact: general → theme
Assignee | ||
Comment 1•14 years ago
|
||
This seems to do the trick.
Assignee: nobody → dolske
Attachment #459689 -
Flags: review?
Assignee | ||
Comment 2•14 years ago
|
||
Attachment #459690 -
Flags: ui-review?(limi)
Assignee | ||
Updated•14 years ago
|
Attachment #459690 -
Attachment is patch: false
Attachment #459690 -
Attachment mime type: text/plain → image/png
Comment 3•14 years ago
|
||
Comment on attachment 459689 [details] [diff] [review]
Patch v.1
>-.bookmark-item[livemark] > .toolbarbutton-menu-dropmarker {
>- list-style-image: url("chrome://browser/skin/places/livemarkFolder.png");
This makes livemarkFolder.png unused, so actually remove it please.
> }
>
>-.bookmark-item > .toolbarbutton-icon {
>- display: none !important;
>-}
>
nit: remove another line here
Comment 4•14 years ago
|
||
How would you hide the icons, then? Selecting "text only" in the customization panel would apply to the other toolbars, too, wouldn't it?
In the screenshot the icons are squeezed vertically. Can we please not do that, or at least squeeze them horizontally too so we keep the aspect ratio?
Comment 5•14 years ago
|
||
The idea that we'd expose an option for this seems odd. Users who don't want the icons can use userChrome.css, an extension or theme. Note that hiding them is easier than restoring them, making the first two options more viable than they are right now.
Comment 6•14 years ago
|
||
(In reply to comment #5)
> The idea that we'd expose an option for this seems odd.
That's what this bug was filed about, no?
Comment 7•14 years ago
|
||
Maybe, although the main concern seems to be that the bookmarks lack favicons, and dolske's patch addresses exactly that.
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #4)
> How would you hide the icons, then? Selecting "text only" in the customization
> panel would apply to the other toolbars, too, wouldn't it?
I'm not sure that's really a useful configuration option, checked with faaborg and he agrees. [I'd love to know if anyone actually runs the browser that way, and why.]
The basic idea behind this bug is that favicons very important for visual recognition/identification, and so bookmark toolbar items should show them. Removing them may have matched Safari's style, but we're explicitly wanting to break with that in the name of improved UI.
I considered making the icons look more muted (by adjusting opacity ala tabs, or perhaps with SVG/canvas to desaturate them), but I'm not sure that will end up looking good, and it can be investigated in followup.
> In the screenshot the icons are squeezed vertically. Can we please not do that,
> or at least squeeze them horizontally too so we keep the aspect ratio?
Oops, yeah. Not intentional. Will fix in updated patch.
Assignee | ||
Comment 9•14 years ago
|
||
Huh, to get rid of the squished favicon it seems I need to change .toolbarbutton-text to using vertical-align:baseline (instead of center), and force .toolbarbutton-icon to have min-height:16px. Either alone does nothing, I'm not entirely sure why isn't being squished, some kind of pixel-snapping bug when it's centered vertically?
This results in a correct icon but the bookmark is too fat vertically (http://grab.by/5DH6), so I'll probably need to fix up some padding and such next.
Assignee | ||
Comment 10•14 years ago
|
||
Hmm, turns out the weirdness was caused by the new Bookmarks button (right side of bookmarks toolbar). Seems trying to cram a 20x20 -moz-image-region into whatever space it had made things unhappy.
Attachment #459689 -
Attachment is obsolete: true
Attachment #459690 -
Attachment is obsolete: true
Attachment #461112 -
Flags: review?
Attachment #459689 -
Flags: review?
Attachment #459690 -
Flags: ui-review?(limi)
Assignee | ||
Updated•14 years ago
|
Attachment #461112 -
Flags: review? → review?(dao)
Assignee | ||
Comment 11•14 years ago
|
||
Attachment #461113 -
Flags: ui-review?(limi)
Comment 12•14 years ago
|
||
Comment on attachment 461112 [details] [diff] [review]
Patch v.2
> #bookmarks-menu-button.bookmark-item {
>+ -moz-image-region: rect(2px, 178px, 18px, 162px);
> list-style-image: url("chrome://browser/skin/Toolbar.png");
> }
Are the clipped pixels completely transparent?
Assignee | ||
Comment 13•14 years ago
|
||
(In reply to comment #12)
> Are the clipped pixels completely transparent?
Yes. The object of interest in the sheet dark grey (16w x 14h), with a a 1px white shadow/emboss on the bottom. Outside of that 16x15 region it's entirely transparent. Attached is a screenshot of that button being hovered to show the shadow and alignment.
Updated•14 years ago
|
Attachment #461112 -
Flags: review?(dao) → review+
Comment 14•14 years ago
|
||
Comment on attachment 461113 [details]
Screenshot of patch v.2
We'll have to look at making the icons a bit better-looking, but that shouldn't block ui-r. :)
Attachment #461113 -
Flags: ui-review?(limi) → ui-review+
Assignee | ||
Comment 15•14 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/0f6f1e759cd1
Will file a followup for looking at updating the stock folder icons.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b3
Comment 16•14 years ago
|
||
Comment on attachment 461112 [details] [diff] [review]
Patch v.2
post-hoc a=me
Attachment #461112 -
Flags: approval2.0+
Updated•14 years ago
|
blocking2.0: --- → beta3+
Comment 17•14 years ago
|
||
With this patch checked in my bookmarks toolbar has blown up to more than twice it's normal size. I see though, through the "Screenshot of patch v.2" that this is not the intention, and by creating a new profile with the default settings. I suspect that an add-on is to blame, so I'll do some "research" into this.
Comment 18•14 years ago
|
||
Now this is interesting. I tried disabling all my add-ons and plugins, but it had no effect. Then I figured it might be because I keep more bookmarks in the toolbar than most people (it's about 140, I think, I need to put them in folders), and put all of them in a folder. Sure enough, the bookmark toolbar shrunk down to it's original size. To spare you the rest, I figured out that a certain bookmark, with a certain favicon caused this problem.
The address I had bookmarked was:
"http://stanford.edu/~jlebar/moz/respkg"
It now immediately redirects to a mozilla.org page, but when I had visited it before, it had the favicon of the Stanford University.
It reappears for me when I bookmark "http://www.stanford.edu", so try to reproduce it yourselves and see if you can find a solution.
Comment 19•14 years ago
|
||
Ok, now it seems pretty obvious what causes the problem. The Stanford University favicon is a 32x32 (http://www.stanford.edu/favicon.ico) icon, which is displayed squished together in the bookmarks toolbar. Since the link I originally had bookmarked was too far in among the bookmarks to appear in the bookmarks toolbar itself, it appeared in the menu besides where everything looks correct.
Assignee | ||
Comment 20•14 years ago
|
||
I filed bug 583616 for the issue in comments 17-19.
(For any other issues, please file new bugs and mark them as blocking this one.)
You need to log in
before you can comment on or make changes to this bug.
Description
•