892959 - Broken icons in customization palette / menupanel on Linux

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[:Gijs (out for now; he/him)]

Attachment: Screenshot

Someone noted in #ux today that Linux is currently fairly significantly broken when icons are in the customization palette. Screenshot attached that was taken from the channel messages they left (unfortunately they left immediately afterwards, so no time to chat):

[2013-07-12 14:05:59] <testman> Hi. Here is screenshot of UX on my Fedora 18 x86_64 running Gnome 3: http://shrani.si/f/3X/oQ/HWRs6z5/screenshot-from-2013-07-.png
[2013-07-12 14:06:58] <testman> Just want to notice that some of icons are not showing up properly.
[2013-07-12 14:07:42] <testman> Also, Downloads "arrow" icon dissapears when clicked and does not appear again.

The screenshot is recent, judging from the customize and help buttons in the panel, which landed a couple of days ago.

I think this is P1 seeing as this is technically still a tier-1 platform, and this isn't shippable.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: correct icons for built-in widgets

Matt: please bounce this off your queue to someone else if not appropriate/ no time. I flagged you for r, because bug 892959 is kinda similar, just different platform.

On another note, I'm really unhappy about the fact that I had to expand the selectors for built-in widgets this much to make the GTK icon only appear when the button is inside the toolbar.

I can also change the selectors to

```css
#home-button[customizableui-areatype="toolbar"] {
  list-style-image: url("moz-icon://stock/gtk-home?size=toolbar");
}
```

Which looks a LOT cleaner, but I don't know if that attribute is always there (AFAICS it is).

Comment 2

[Mike de Boer [:mikedeboer]]

btw, I kept the current style, instead of the usage of an attribute as proposed in comment 1 to illustrate how I got here. Consider this patch an intermediate representation of the final fix :P

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: correct icons for built-in widgets

Went with suggestion in comment 1 after discussion on IRC.

Comment 4

[:Gijs (out for now; he/him)]

Comment on attachment 775650 [details] [diff] [review]
correct icons for built-in widgets

Review of attachment 775650 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/linux/browser.css
@@ -648,3 @@
>    list-style-image: url("moz-icon://stock/gtk-print?size=toolbar");
>  }
> -:not(toolbarpaletteitem) > #print-button[disabled="true"] {

Why get rid of the :not(toolbarpaletteitem) everywhere?

Comment 5

[Mike de Boer [:mikedeboer]]

(In reply to :Gijs Kruitbosch from comment #4)
> Why get rid of the :not(toolbarpaletteitem) everywhere?

Because we want stock icons to appear ONLY in the toolbar. In the palette (toolbarpaletteitem case) AND the menu-panel we want to show the Australis icons.

Of course this will change when we start using SVG icons instead of the stock icon sets, but this is intended to resolve the current situation until we have that (see bug 874674).

Comment 6

[:Gijs (out for now; he/him)]

Comment on attachment 775650 [details] [diff] [review]
correct icons for built-in widgets

Review of attachment 775650 [details] [diff] [review]:
-----------------------------------------------------------------

Hrm. Still not a fan, but I guess this'll do as a stopgap. IMO we should ideally sort this out properly before landing though.

Comment 7

[Mike de Boer [:mikedeboer]]

Thanks!

https://hg.mozilla.org/projects/ux/rev/0c53c2cf98b5

Comment 8

[:Gijs (out for now; he/him)]

https://hg.mozilla.org/mozilla-central/rev/0c53c2cf98b5