The current icon isn't very big, because it was designed for use in toolbars. We should have a more high-resolution one, and/or decide if this icon fits with the other icons in the panel. Stephen, this probably needs your help (or that of someone else who can do the icon-wrangling for us). (Filed based on bug 878544 comment 4, but I noticed this myself when looking at bookmark toolbar items + panel stuff)

Looks hi-res to me. Is this asset still needed?

(In reply to mmaslaney from comment #1) > Created attachment 8345462 [details] > Screen Shot 2013-12-10 at 12.15.21 PM.png > > Looks hi-res to me. Is this asset still needed? That's the bookmarks menu button, not the bookmarks toolbar item.

2 things to note : This icon should also be updated in the "bookmarks sidebar" and the library. The text is cropped a bit weirdly in customization mode. It should appear on two lines.

(In reply to Guillaume C. [:ge3k0s] from comment #5) > 2 things to note : > > This icon should also be updated in the "bookmarks sidebar" and the library. Why? The icon isn't changing outside the panel/palette. It'll be the same in toolbars as well. This bug is purely out of concern for the resolution of the icon when it's displayed at 36px^2. Certainly changing the icon everywhere doesn't need to happen as part of this patch. Furthermore, the flat icon will look out of place in the current bookmarks UI, so we'd have to refresh all of those icons, which is even more out of scope here.

So this patch does fix the alignment of the icon by making palette wrappers remove and re-add flex for the things they wrap. I chose not to fix the text wrapping in here because we're still figuring out what the desired endstate is for cropped single-line vs. multiline labels in bug 944947.

Updated with all platform themes.

Comment on attachment 8360417 [details] [diff] [review] Australis: Need a more high-res version of the bookmarks toolbar item placeholder icon for the palette/panel, Cancelling review because I should integrate the other icons for Windows.

So this adds the WinXP icon. I'm going to leave the win8 bit for Mike de Boer in bug 859751.

Comment on attachment 8360692 [details] [diff] [review] Australis: Need a more high-res version of the bookmarks toolbar item placeholder icon for the palette/panel, Review of attachment 8360692 [details] [diff] [review]: ----------------------------------------------------------------- Other than the little bug I found, I think this looks OK to me. Certainly better than what we had before. ::: browser/components/customizableui/src/CustomizeMode.jsm @@ +685,5 @@ > toolbarItem.checked = true; > } > > + if (aWrapper.hasAttribute("flex") && !toolbarItem.hasAttribute("flex")) { > + toolbarItem.setAttribute(aWrapper.getAttribute("flex")); This needs to be: toolbarItem.setAttribute("flex", aWrapper.getAttribute("flex"))

With my stupidity addressed (at least as far as this patch is concerned), remote: https://hg.mozilla.org/integration/fx-team/rev/4a5c84a4d166

So this fixes the styling of the search bar and the bookmarks bar. There's still a separate issue that only occurs if moving the bookmarks toolbar items from the palette to the menu panel, but stops as soon as they've been in the toolbar. Don't think that needs to hold up this bug, though.

Comment on attachment 8366666 [details] [diff] [review] Backed out changeset 4a5c84a4d166 () Sigh. Wrong bug! :-)