Closed Bug 348719 Opened 14 years ago Closed 10 years ago

[Mac] Add site icons / favicons to bookmarks toolbar buttons

Categories

(Firefox :: Theme, enhancement, P5)

All
macOS
enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: waynegwoods, Unassigned)

References

Details

(Keywords: uiwanted)

Attachments

(24 obsolete files)

Since a theme refresh is being explored for Firefox 2, I thought this was worth revisiting.

I've created a patch to add icons to the bookmarks toolbar, and personally I think it looks much better with them. Maybe you guys will disagree. I think the icons break up what is otherwise a long bar of grey text, and make the individual button regions more defined. They're also practical, as they make an easier visual target of the button you're after. The only downside is they take up a little more room on the toolbar.

Comparison screenshots follow, then a patch.
Attached patch Patch v.1 (obsolete) — Splinter Review
Version 1 of the patch.

I added a 1 px margin around the icon so it doesn't go right to the edges of the button. I also added a 1 px padding-left to the text to separate it a little more (without distorting the icon), but it still looks OK without that extra space.

I disabled the folder icon for menu buttons, which is a departure from the Windows appearance, since we have the drop-down arrow already. So the css additions only apply to not([container]) items.
Finally, if you want to play with the css without compiling, here's a userChrome.css version of the patch to stick in the <profile>/chrome/ folder. Seems to work fine for both trunk and 1.8 branch.
(In reply to comment #2)
> I added a 1 px margin around the icon

I meant padding, not margin.
You should also test with livemarks, although I think those are containers too.
(In reply to comment #5)
> You should also test with livemarks, although I think those are containers too.

The livemarks look fine. I think you're right that they're treated as containers, so they shouldn't be affected.
This shows icons in the bookmarks bar at something less than their 16x16 native size.  Can this be done without scaling?

I feel like there's too much dead space between bookmark buttons now that there are icons separating them.  It's 2px now, does there need to be any dead space?

It looks a little bit weird to have the RSS icon on the right side when all other icons are on the left.  On the other hand, that's where the triangle for folder buttons belongs, and the RSS icon behaves much more like a folder.  Perhaps this RSS icon should also have a triangle, so it can stay on the right and its meaning is clear.

Regarding the sizing, a benefit of icons in the bookmarks bar is that it lets users have easily-identified textless bookmarks that consume less space.
For what it's worth, I've always liked the idea of icons in the bookmarks toolbar.  From what I understand, they were intentionally omitted from the original Pinstripe theme on Kevin's executive decision, probably because they're not native there in that Safari doesn't have them.

Camino does.
I like 'em!
(In reply to comment #7)
> This shows icons in the bookmarks bar at something less than their 16x16 native
> size.  Can this be done without scaling?

I'm not sure. If I remove the 1px padding I placed around the icon, it squashes. I tried simultaneously removing the top & bottom padding from toolbarbutton.bookmark-item, but that seemed to be no improvement. There seems to be something else restraining it vertically. I'll have another look tonight.

> I feel like there's too much dead space between bookmark buttons now that there
> are icons separating them.  It's 2px now, does there need to be any dead space?

I think you're right. I don't think there needs to be any space between the buttons... just a bit of internal padding to separate them, as it is on Windows.

> It looks a little bit weird to have the RSS icon on the right side when all
> other icons are on the left.  On the other hand, that's where the triangle for
> folder buttons belongs

That's something that's definitely done the opposite way to Windows. I think for consistency, both the RSS and drop menu icons would be better on the left, as well. But maybe that's a separate bug...

Anyway, I'll tinker with the css more, generate a new patch tonight and see what ya think.
Attached file userChrome version v.2 (obsolete) —
Duh, the reason the padding change wasn't taking properly was because I forgot the "!important".

Having removed the padding, the icon should be at full size. The icon looks better, but there's a couple of issues with this:

- Test using an icon that takes up the full 16 px height (such as the MozillZine one). The icon goes right to the top/bottom edges of the button, which I don't think looks as good as when it's framed by the button. The way around this could be to increase the button height by 2 px, which should be doable. If so, I assume new button images would have to be created for this?

- Test it on the 1.8 branch. You'll notice for large icons that the icon sticks out the bottom of the button by 1px. This seems to be because the buttons are distorted on the branch right now... because the bottom is squashed a bit. I assume that's a bug and not intended appearance?
Attachment #233773 - Attachment is obsolete: true
(In reply to comment #11)
> This seems to be because the buttons are
> distorted on the branch right now... because the bottom is squashed a bit. I
> assume that's a bug and not intended appearance?

Oh, I see now. It looks like the button isn't actually distorted, but the bottom line of the hover image is so light that it blends into the toolbar background and looks like it's squashed. So while the icon looks like it's sticking out from the button, it's really not. I filed bug 348837 for that.
Wayne, I agree with both of your comments.  The button's bottom probably needs to be bumped out by 1px to fix bug 348837 and make this feasible.  It would also look better if the favicon were enclosed within the button, but I'm not sure if that means adding 1px to the top (total 2px) or adding 1px to the top and bottom (total 3px).
Attached image Mockups (obsolete) —
The three hovered buttons here are the current theme with Wayne's favicons and a 16px high button, an 18px high button, and a 19px high button.  I did a very rough job here, it's only meant to help see whether the button should be grown by 2px or 3px to make the icon look like it's within the button.  Based on this, it looks like 2px is sufficient, but the height of the bookmarks toolbar should also be increased by 1px to keep the button from rubbing up against the separator above.
Not going to block on this, its really late in the cycle to explore yet another set of UI implications.
Flags: blocking-firefox2? → blocking-firefox2-
Thanks Mark!

I think the 18 px button will be enough, once bug 348837 is fixed. It really only needs 1 px either side to enclose it. And a simple margin adjustment should widen the toolbar.

Jay, do you think this is worthwhile trying? If so, the button icon could be adjusted for height at the same time it's adjusted for bug 348837. It could always be reduced again if it doesn't work out, and we could go for a conservative approach (either 14 px icons, or leave them at 16 px and fill the entire button height).
Or... if someone is able to produce 18 px high versions of the button images for the old theme, we could check something in to the trunk immediately for testing. I'd be happy to make them, but I suspect I'd botch it.
This is a modified version of the new Bookmark Toolbar hover images, made 2 px taller to accommodate favicons. Hope it works!
Attached image 18 px "fat" button (obsolete) —
(In reply to comment #18)
> This is a modified version of the new Bookmark Toolbar hover images, made 2 px
> taller to accommodate favicons. Hope it works!

It works :) I compiled a trunk version, but using those images (don't have resources to build branch). But I also had to modify the css immediately or the buttons go weird, as the theme assumes 16 px in a few places. I'll post a userChrome version of the changes next.

The one nit I had was that I think the buttons look more blunt on the ends now... which makes them look a lot fatter (screenshot attached).
Attached file userChrome version v.3 (for 18px only) (obsolete) —
Here's the chrome I used. With the 18px button images, I also had to set the button text, icon and dropmarkers to 18px, or they'd scale down the button image. So then I had to add a 1px padding around them to shrink them back to 16 px while leaving the background image at 18 px. If anyone knows a better way of doing this, please lemme know.

You should notice that the icon is enclosed by the tab image now... but only by 1px, so it's faint. I think it looks a bit better that way, but do you think it's worth it?
And is there any chance of slipping this in between beta 2 and rc1? Or is this too late now?
Wayne, could you prepare this as a patch to the in-tree css?  I don't know if it's too late or not, but it seems kind of silly to pursue a separate trunk version when the theme's days on the trunk are numbered.
OK, I'll do that shortly. One question, though...

Do we want to move the menu dropmarker icons to be on the LHS as well, for consistency?

If so, I'd recommend disabling the dropmarker entirely and relying on the default folder/live bookmarks icons, which is how it's done on Windows. This is really easy, but you may not like the change in images. Alternatively, if you want to keep the simplified dropmarker images, I think I'd have to modify the xml.

For live bookmarks, I prefer the icon to the dropmarker (like Windows), because it has a down arrow as part of the icon. The actual dropmarker (which we use now), however, has no arrow and so there's no suggestion that it's a popup menu at all.

For standard (non-livemark) menus, I think the opposite is the case, as our dropmarker is an arrow. The folder icon (such as Windows uses) has no arrow and doesn't suggest a menu. Ideally, I'd still like to go with the icon method, but modify the folder to have an arrow on it. But I'm not experienced enough to go messing with images.
Here's the patch. It requires concurrent check-in of the 18px-high buttons (which don't seem to be a complete set yet). I've left the dropmarkers where they were... on the RHS.
Attachment #233772 - Attachment is obsolete: true
Comment on attachment 235180 [details] [diff] [review]
Patch v.2 - for 18px buttons, but leaving dropmarkers alone

Wayne, are you going to request review from someone (probably mconnor), or do you want feedback first?
(In reply to comment #25)
> (From update of attachment 235180 [details] [diff] [review] [edit])
> Wayne, are you going to request review from someone (probably mconnor), or do
> you want feedback first?

Some feedback would be greatly appreciated! Thanks :) I was actually delaying review request until the full set of 18px button icons were submitted. I've posted on bug 348837 about that, but maybe it's not worth waiting? Neither can be checked in without the other, however.
Comment on attachment 235180 [details] [diff] [review]
Patch v.2 - for 18px buttons, but leaving dropmarkers alone

I guess I better just request a review anyway.
Attachment #235180 - Flags: review?(mconnor)
Attached file 18-pixel-high images (obsolete) —
Here is a full set of 18-pixel-high images that correspond to the latest 16-pixel-high ones from bug 348865, making sure not to clobber bug 343897 or bug 348837.  They're not perfectly smooth, because I'm not that skilled, but I think they'll serve your needs.
Attachment #234214 - Attachment is obsolete: true
(In reply to comment #28)
> Here is a full set of 18-pixel-high images that correspond to the latest
> 16-pixel-high ones from bug 348865, making sure not to clobber bug 343897 or
> bug 348837.  They're not perfectly smooth, because I'm not that skilled, but I
> think they'll serve your needs.

Thanks Pam :) I'll try them out in a build as soon as I can. Eyeballing them, they look like they should be good. Of note, the new "open" images you created do seem smoother and nicer than the corresponding "hover" images (less squared, as per comment 19... ). Are the hover images the ones that were already in attachment 234214 [details]? If so, any chance you could please smooth them the same way? :) I haven't tried building them in a browser yet, however, so maybe the difference is just an illusion of my picture viewer. :)

Pam, I tested them out and they look good, except that the bottom line of the "open" icons is still so pale that it looks like the button shrinks when it's pressed (i.e. bug 348837).

When magnified, the bottom line is definitely darker than the toolbar background, but this isn't obvious at normal magnification... it just blends in.
Patch update, since things had bitrotted in one segment. My code didn't need changing... it was just that surrounding changes in browser.css were throwing out the alignment.
Attachment #233983 - Attachment is obsolete: true
Attachment #234429 - Attachment is obsolete: true
Attachment #235180 - Attachment is obsolete: true
Attachment #237606 - Flags: review?(mconnor)
Attachment #235180 - Flags: review?(mconnor)
Attached image 22 px high toolbar background (obsolete) —
With the 18px buttons, the toolbar requires a 2px higher background, or its undergarments start to show. So this'll need to be checked in at the same time. It's just the same as the old one, but with the colour extended for 2 px more at the bottom. Already tested.
(In reply to comment #30)
> Pam, I tested them out and they look good, except that the bottom line of the
> "open" icons is still so pale that it looks like the button shrinks when it's
> pressed (i.e. bug 348837).

Okay, here's a set with a darker lower edge.  Since bug 348837 was marked as fixed, I'd gotten the impression that the current images were the right ones, but now I see what you're talking about and have fixed both the 16- and the 18-pixel versions.

> Of note, the new "open" images you created
> do seem smoother and nicer than the corresponding "hover" images (less squared,
> as per comment 19... ).

Both sets were from the images that were in CVS at the time.  Here are rounder "hover" images to better match the "open" ones, though.  (16-px versions of these have been submitted to bug 348837, which also has a set of comparison screenshots in attachment 238016 [details].)
Attachment #237212 - Attachment is obsolete: true
(In reply to comment #33)
> Created an attachment (id=238018) [edit]
> 18-pixel-high images: "open" darker border, "hover" rounder

They look brilliant. Thanks Pam!

Mike, are you willing to review this? I've been testing it for some time on my home builds. If you're worried about changes to the chrome this late, I can always work up a 16px version again. I think the 18px is a little nicer (border around the favicons), but even the 16px is better than nothing. Thanks.
Mark (Mentovai), did you ever try this out? :) I'd still love some feedback.

I'm worried that the silence on here is a sign that it's not going to make it in time. But I'm trying to stay hopeful... Though it'd be nice to at least know if I'm wasting my time keeping it updated.
Sorry, Wayne, I've been busy with the 'bag.  I think that this looks awesome.  The livemark icon is a pixel too low, I think.  Other than that, this is great.
(In reply to comment #36)
> The livemark icon is a pixel too low, I think.  Other than that, this is great.

Hmmm, I can't see any difference. For example, in the default "Latest Headlines" button, the livemark starts 1 px below, and 3 px to the right of the final 's' in both the official 2.0 nightly and the 18px button versions. Does it appear differently on yours?

Anyway, thanks for testing it! :)
Oh, well, in that case, my preference would be to move it up 1px in the 16px iconless version, too - no points docked for matching it!  :)
Attached image Dropmarker change idea (obsolete) —
That's easy enough to do :) I think it looks better, too.

But, do you think there's really any chance of this making it in before Fx 2 now? If there's a small chance, I'm worried that further adjustments will make it even less likely to be accepted. But if it's already too late, I'm happy to keep adjusting... 

Assuming I can keep experimenting... what do you think of this idea instead? (see screenshot attachment). Basically, enable "favicons" for containers as well, which makes the folder or rss icon appear to the left. Keep the dropmarker to the right to indicate that it's a menu, but use the same arrow for both folders and livemarks. This is really easy to do, and I think (i) it better matches the container buttons with the normal ones, and (ii) the dropmarker arrow for the livemarks is more intuitive than the current rss one. I'd use the folder-arrow or rss-arrow style icons instead, but as far as I can tell they were removed from Fx 2 when Places was disabled.

Anyway, I appreciate you taking the time to comment. I'm sure you're already flat out with Fx 2 looming :)
Comment on attachment 237606 [details] [diff] [review]
Patch v.3. For use with 18 px buttons

As per Mike Connor's comments on Planet, are you able to please review this, Asaf? Perhaps it's too late now, but even just a comment saying that much would be appreciated at this point. Thanks :)
Attachment #237606 - Flags: review?(mconnor) → review?(mano)
Assignee: jgoldman → w.woods
Comment on attachment 237606 [details] [diff] [review]
Patch v.3. For use with 18 px buttons

Few issues:
  1) the hover feedback is broken, screenshot coming.
  2) It looks to me like the items aren't middle-aligned (vertically)
  3) I'm not sure whether it makes sense to keep the live-bookmark icon on the other side, talk to beltzner/mconnor please.
  4) I think the iccn needs a bit more padding on the right side.
Attachment #237606 - Flags: review?(mano) → review-
Asaf, thanks for the review :)

One question, though, did you also apply the images for the 18px high buttons (attachment 238018 [details]) and the taller background (attachment 237682 [details])? The screenshot you showed me is usually what happens when the code is applied to the current 16px buttons, so hopefully that's the only reason you saw these two issues:
>   1) the hover feedback is broken, screenshot coming.
>   2) It looks to me like the items aren't middle-aligned (vertically)

I have had a patch working with 16 px high buttons in the past, but that doesn't leave the favicons any border. 18px definitely looks better.

As for the other two issues:

>   3) I'm not sure whether it makes sense to keep the live-bookmark icon on the
> other side, talk to beltzner/mconnor please.

I agree. Ideally I'd like to just have an icon on the left, of a folder or rss image with a little drop arrow over it. But since those aren't available (afaik the folder one doesn't exist, and the rss one I think was disabled with Places), I can set it to a regular folder/rss image on the left, and have the drop marker on the right. I think this looks pretty good... see the screenshot and later remarks in comment 39. Anyway, I'll speak to beltzner/mconnor about it, as you suggest. What's the best way of contacting them?

>   4) I think the icon needs a bit more padding on the right side.

Do you think the icon's too close to the label? I can put in another 1-2px padding. I just tried to keep it compact... the more space there is between the icon and the text, the less it looks like they belong together when not highlighted.

Thanks :)
Ugh, I didn't test with the new images, sorry.

Anyway, talked-about-it on IRC, we sorta agreed that the dropmarker used for folders should be replaced with a real folder icon (like we already do in the  bookmarks menu!), and then we would update the live bookmarks icon as well (again, maybe make it the same as the icon used in menus?).

Please do talk to beltzner before taking these suggestions.
Group: mozillaorgconfidential
(In reply to comment #44)
> Anyway, talked-about-it on IRC, we sorta agreed that the dropmarker used for
> folders should be replaced with a real folder icon (like we already do in the 
> bookmarks menu!), and then we would update the live bookmarks icon as well
> (again, maybe make it the same as the icon used in menus?).
> 
> Please do talk to beltzner before taking these suggestions.

Thanks Asaf. I'll get in contact with beltzner as soon as I can manage to be on irc at the same time.

I agree about the drop markers... using real icons fits better, and also matches the Windows way of doing it. The one thing I don't like about the Windows way is that it's not really intuitive that the folder icon is a drop-menu. It should have a drop arrow superimposed on it the same way that the toolbar RSS icon has. I'm not sure who can make good quality images that are suitable, but I'll post a screenshot shortly, that uses my own mockups.
The container icons are only rough mock-ups. Better ones can be made, but it should give the idea.

Compare with the screenshot in attachment 238732 [details], where dropmarkers are displayed independently of the icon in containers.
Haven't spoken to beltzner yet, but wanted to put these up before I did.

I still thought the proposed new 18px buttons looked a little blocky / cut-off at the ends. I worked on them a bit more and they now look more rounded. Hope you guys like them. For simplicity, the attached archive also includes the required new 22px background.

Screenshot next, to demonstrate.
Attachment #238018 - Attachment is obsolete: true
If the difference in roundness isn't immediately obvious, magnify the image a bit :) In practice, I think the bottom ones definitely look better.

Also in the bottom image, I've modified the icons for the folder/livemark buttons a bit more (outline around the arrow).
Attachment #241063 - Attachment is patch: false
Attachment #241063 - Attachment mime type: text/plain → image/png
Attached image Bookmark Icon Fault (obsolete) —
It seems that there are still two issues with the handling of bookmark icons. 
I uploaded a hopefully comprehensive screenshot here https://bugzilla.mozilla.org/attachment.cgi?id=242837

#1: The icons are not depicted in the tool bar at all. This is working in FF 1.5. As I usually delete the description field to end up with the icon in the bookmark toolbar I am gaining a lot of space. Just follow the upper red arrow. There should be several site icons to the left an right odf the text area named 'Netzzeitung'.

#2: For icons there are different formats out there. 

If you look to the red framed icon in the drop down menu you will see how it looks like in the address field as well as in the tabs. The lower red arrow points to the correspondent window of the bookmark manager. This icon seems to be cut somehow.

I am using a 2x2GHz G5 Mac. The Firefox version I downloaded earlier today is marked as 

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; de; rv:1.8.1) Gecko/20061010
HTH
Ulf
(In reply to comment #50)
> #1: The icons are not depicted in the tool bar at all. This is working in FF
> 1.5.

What theme were you using for Firefox 1.5? The default theme (Pinstripe) never had bookmark toolbar icons for Mac 1.5 (or 1.0). Very old builds of Firebird did, but they were disabled by the introduction of Pinstripe. So if you're seeing those icons, I assume you must have had a third-party theme (or something in your userChrome.css file that you've since deleted).

The icons obviously won't make it back in for Firefox 2. I'm still hoping to get a review of my patch, so maybe they can make it in a later branch release, or at least on the trunk to bake.

> #2: For icons there are different formats out there. 
> If you look to the red framed icon in the drop down menu you will see how it
> looks like in the address field as well as in the tabs. The lower red arrow
> points to the correspondent window of the bookmark manager. This icon seems to
> be cut somehow.

That's bug 175787, which was never fixed on the 1.8 branch. Basically, when the image is larger that 16x16, it's clipped in treeview situations, rather than scaled. It's fixed on the trunk, though.
(In reply to comment #51)
THX Wayne,
In fact I am using a third party theme (Breeze 1.5.3 to be exact) to get a more compact display. Now I downloaded Littlefox 1.7 which has an even more compact display and despite the different look the favicons reappeared in the bookmark tool bar. As the bookmark menu is showing the icons, some extensions like ScrapBook are using them as well, I don't see any reason why this information is cut off in the standard theme. To me it looks like a waste of space if you need to have to use text instead of icons. If you use more4 than 3 letters for each link, the icons use less space. In fact with the 'new' theme I gained some additional space.

I hope my vote helps for getting the processed and allready stored icon information into the standard theme as I was not aware about this being a theme issue after a lonnnnnng period of allready using Firefox.
(In reply to comment #52)
Download attachment 233983 [details], above, and move it to (or paste the contents into) userChrome.css in the chrome subdirectory of your Firefox user profile. That should give you the icons back until there's a theme fix. I can't guarantee how it'll look for the third-party theme, however... In the long run, you're better off asking them if they'll re-enable the icons in the theme.
See also bug 360089 about related problems in the bookmark toolbar; it's hard to visually distinguish items from each other, and the separator is invisible.
(In reply to comment #54)
> See also bug 360089 about related problems in the bookmark toolbar; it's hard
> to visually distinguish items from each other, and the separator is invisible.

Fixing this bug should go a long way towards distinguishing individual bookmarks. I've sent Mike Beltzner what I've done so far, and he said he'll be reviewing it when Firefox 2 lets him go.
Any status on this one? It'd be a huge win in usability.
(In reply to comment #56)
> Any status on this one? It'd be a huge win in usability.

Nothing new. Still just waiting for Beltzner to have time to review it.

In case anyone else wants to provide some feedback, the following is the info that I mailed Mike Beltzner, with text modified for Bugzilla. It's a summary of what I've done, and where I want to go with the bookmarks icons.

What's posted here currently is:
1. a patch (attachment 237606 [details] [diff] [review]) for enabling icons on the buttons at native 16x16px. But only for non-container buttons.
2. an archive (attachment 241602 [details]) of taller hover/open icons for the buttons and the bookmarks toolbar background (18px and 22px respectively) to accommodate the icons at native size. The icons *could* be enabled with the current 16px-high buttons, but they don't look as good when there's no extra button height to provide a border around the icon. Alternatively, we'd have to shrink the icons to 14x14px, which isn't as nice.

Asaf Romano has reviewed the code to date, and his main comment, which I agreed with, was that it'd look much better if the container/livemark dropmarker icons were also on the left, and were based on standard folder/livemark icons (as in Windows). It's this that he said I should seek advice about.

It's easy enough to remove the dropmarkers and enable icons for the containers. The one thing I don't like about the Windows way of doing it, however, is that the folder icon has no drop arrow... so it's not so intuitive that it's a menu. On Mac, this would also be the case, if we implemented it as-is.

I've investigated a couple of different options for this:

1. Enable the folder/livemark icons on the left, but also keep the dropmarker arrow on the right for containers. For livemarks, use the same drop arrow as the folder. A screenshot is in attachment 238732 [details].

2. Remove the dropmarker arrow from the right, and modify the icons on the left so that they that have a small dropmarker arrow superimposed on them, just like the livemark icon is on the toolbar in Windows. There's a suitable livemark icon in Places for that on Mac, but it's currently disabled. There's no folder equivalent at all. However, I made up a couple of icons purely as a proof of concept. I'm not a professional and someone else might do them better, though I think they look pretty good. A screenshot of how they look is in attachment 241063 [details].
The top toolbar in the image was the first set I created (using just plain black drop arrows). In the second set, the arrows have an outline and look better (It's actually a handy coincidence that the image shows that comparison, as I originally created it to compare the bookmark buttons hover images between the first 18 px-high ones submitted and the ones I just modified and submitted... the latter are more rounded, see comment 47 and comment 48).

I favour the second option (icons with arrows on them). I haven't submitted a patch for this, however, as I wanted to be sure I was going in the right direction first.

If anyone has any useful feedback on the direction I'm going, it'd be much appreciated. The two gripes that I anticipate so far are that: (i) someone will be upset that the bookmarks toolbar is 2px taller, and (ii) someone will be upset that they now have icons that they can't turn off. The bonuses, of course, are that (i) you can find buttons much more easily, (ii) you can delete the button titles and view buttons by icon alone, which allows you to fit many more buttons on the toolbar, and (iii) I think the majority will find it looks nicer, though I've taken no poll ;)
Well, this bug obviously went nowhere fast with the branch. Time to revisit it, I hope.

I've converted the patch to the trunk.

Since nobody had any feedback on my ideas in comment 58, I've gone ahead and added them to the new patch. Basically, the old drop arrows for bookmark toolbar menus/containers (folders and livemarks) are gone. Instead, I've added normal favicon-style images to the left, so they match the other buttons. Both container icons have a drop arrow superimposed on them, so it's obvious they're menus. Pretty much what's depicted in the lower image in attachment 241063 [details].

Corresponding image archive to follow.
Attachment #237606 - Attachment is obsolete: true
Attachment #256334 - Flags: review?(beltzner)
Attached file Image archive for use with patch v4 (obsolete) —
Attachment #241062 - Attachment is obsolete: true
Attachment #256336 - Flags: review?(beltzner)
Note we're soon (a3/a4) going to enable places-bookmarks on trunk.
(In reply to comment #61)
> Note we're soon (a3/a4) going to enable places-bookmarks on trunk.

Which will mean... adjusting the styling for a different file?

I suppose it's highly unlikely this will be reviewed first, but at least I've got it updated for trunk, now. Hopefully it won't be hard to adapt to a new file when the time comes.
Flags: blocking-firefox3?
Version: 2.0 Branch → Trunk
Blocking so mconnor and I are forced to make the ui-decision on this one way or another ...
Flags: blocking-firefox3? → blocking-firefox3+
Keywords: uiwanted
Target Milestone: --- → Firefox 3 beta1
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Attached patch Patch v5 - Updated for Places (obsolete) — Splinter Review
Updated to work with Places. We could probably debate the images and styling until Firefox 10, but I'd rather get this in sooner and let the rest be covered later by theme discussion :)
Attachment #256334 - Attachment is obsolete: true
Attachment #256336 - Attachment is obsolete: true
Attachment #276078 - Flags: review?(beltzner)
Attachment #256334 - Flags: review?(beltzner)
Attachment #256336 - Flags: review?(beltzner)
Attached file Image archive for use with patch v5 (obsolete) —
Updated images for use with the new patch (changed a couple of file names to match the Place naming style). Personally, I think that the button hover images are still marginally too blunt on the ends, but I improved them over what they were, and they can be improved more later.
Attachment #276080 - Flags: review?(beltzner)
Setting review requests as per suggestions on #developers, since beltzner is on holidays.

Please remember to use the image archive from v5 (attachment 276080 [details]) in conjunction with this patch.

And also please consider that the layout can be tweaked during the Mac theme refresh later, as long as the code is there.

Thanks in advance :)
Attachment #276078 - Attachment is obsolete: true
Attachment #279319 - Flags: ui-review?(mconnor)
Attachment #279319 - Flags: review?(mano)
Attachment #276078 - Flags: review?(beltzner)
Attachment #276080 - Flags: ui-review?(mconnor)
Attachment #276080 - Flags: review?(mano)
Attachment #276080 - Flags: review?(beltzner)
So, beltzner and I keep batting this around.  We're contemplating a theme shift to a more monochrome/native feel on Mac, and that'll make this look fairly out of place.  I do appreciate the work that's happening here, I'm just not sure its the right thing to do on Mac (as much as I think favicons are generally a win, and we show them in a lot more places now).
OK, thanks for breaking the silence. For Firefox 3 it sounds like you're planning a much more radical theme change than for Fx 2, and I'll look forward to seeing what you come up with (though I'd challenge the idea that native must be monochrome any more than Safari == HIG, but that's not a discussion for here). If you don't mind, I'll leave this open and see how it looks against the new theme, or maybe be considered as a (default-off) option?
Camino uses the unified theme and they have favicons on the bookmark toolbars. It looks really slick and in my opinion, a lot easier to pick out the individual bookmarks on the toolbar. I don't see any reason why Firefox cannot do the same, even if Firefox uses a monochrome theme.
Good point.
Camino doesn't have monochrome Leopard-style UI either.  Punting to M9 for the moment.
Target Milestone: Firefox 3 M8 → Firefox 3 M9
Comment on attachment 276080 [details]
Image archive for use with patch v5

canceling review requests for the time being.
Attachment #276080 - Flags: review?(mano)
I don't want to be a pain about this, but something to consider if/when you revisit it at 3M9... As a non-radical, non-threatening first step, would you consider the idea of a preference to hide/show the bookmarks toolbar icons (default on for PC, off for Mac)? It could even just be a hidden / advanced config pref. Even if you later decide that you want to show the icons in the new Mac theme, it wouldn't be a waste.. it could still be used for anyone who hates icons (on any platform), much as you can already hide the icons on the nav. toolbar. If you're not adverse to that, I'm more than happy to add it.
moving out bugs that don't need to block b1
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Attachment #276080 - Flags: ui-review?(mconnor)
Attachment #279319 - Flags: ui-review?(mconnor)
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Vlad and I were talking a bit about this today, he was mentioning that it wouldn't be that difficult to desaturate favicons using <canvas> so that the resulting image fits better with OSX apps. Adding him to cc.

Another way would be to change the opacity, I suppose.
(In reply to comment #75)
> Vlad and I were talking a bit about this today, he was mentioning that it
> wouldn't be that difficult to desaturate favicons using <canvas> so that the
> resulting image fits better with OSX apps. Adding him to cc.
> 
> Another way would be to change the opacity, I suppose.

Adjusting the saturation sounds like it could be a good way to go, providing it was something controllable in the chrome. It could even be useful by theme developers on other platforms. Another consideration would be to simply reduce the icon size, as the full-size icons stand out a lot more.
Priority: -- → P5
We're pretty sure we're not going to do this for Fx3
Flags: blocking-firefox3+
Target Milestone: Firefox 3 Mx → Firefox 3 M11
Duplicate of this bug: 426147
Duplicate of this bug: 501611
Blocks: 566034
(In reply to comment #76)
> Adjusting the saturation sounds like it could be a good way to go, providing it
> was something controllable in the chrome.

SVG filters can do it.
Assignee: w.woods → nobody
Component: Toolbars → Theme
QA Contact: toolbars → theme
Hardware: PowerPC → All
Target Milestone: Firefox 3 beta3 → ---
Attachment #276080 - Attachment is obsolete: true
Attachment #242837 - Attachment is obsolete: true
Attachment #241063 - Attachment is obsolete: true
Attachment #240940 - Attachment is obsolete: true
Attachment #279319 - Attachment is obsolete: true
Attachment #234011 - Attachment is obsolete: true
Attachment #234425 - Attachment is obsolete: true
Attachment #233771 - Attachment is obsolete: true
Attachment #237682 - Attachment is obsolete: true
Attachment #238732 - Attachment is obsolete: true
Attachment #240372 - Attachment is obsolete: true
This has been fixed by bug 566034. File new followup bugs if you wish to add more tweaks or complaining. :)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.