1400551 - Using Persona/Theme/Light Theme makes all buttons icons in Bars/Toolbars too dark after landing patch from bug #1384686

Status: VERIFIED INVALID

(Firefox :: Theme, defect)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR:
1. Install Virtualfox Persona/Theme/Light Theme ( https://addons.mozilla.org/en-US/firefox/addon/virtualfox/ )
and see that all button icons color is changed to the black
while is should stay the same, like without any Theme

It's also inconsistent now with buttons icons color in Address Bar.

Regression caused by:
Bug #1384686

Regression pushlog:
https://hg.mozilla.org/mozilla-central/rev/7de50f562f6f542a1adbce4a39d533a898ea9afc

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: good build with Theme - proper icon color.png

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: good build with Theme - proper icon color (500%).png

Comment 3

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bad build with Theme - wrong icon color.png

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bad build with Theme - wrong icon color (500%).png

Comment 5

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bad build without Theme - proper icon color.png

Comment 6

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: bad build without Theme - proper icon color (500%).png

Comment 7

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

According to
> (Tim Nguyen :ntim in bug #1384686 comment #3 wrote)
> > The spec says icons shouldn't be full black:
> > http://design.firefox.com/photon/visual/icons.html#color

Comment 8

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm really in favor to standardizing all icons colors and make them all black (or white, depending on used theme), like it's in menus and submenus, as they're more visible per higher contrast to background.

Comment 9

[Dão Gottwald [:dao]]

(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #8)
> I'm really in favor to standardizing all icons colors and make them all
> black (or white, depending on used theme),

They're intentionally different with a lightweight theme applied, as lightweight themes have noisy backgrounds and need more contrast than the default theme.

Comment 10

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

(In reply to Dão Gottwald [::dao] from comment #9)
> (In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see
> your comment/reply/question/etc.) from comment #8)
> > I'm really in favor to standardizing all icons colors and make them all
> > black (or white, depending on used theme),
> 
> They're intentionally different with a lightweight theme applied, as
> lightweight themes have noisy backgrounds and need more contrast than the
> default theme.

So shouldn't also color of the buttons icons in the Address Bar be consistent too and be darker or brighter depending on used Theme?

Comment 11

[Dão Gottwald [:dao]]

The address bar is more opaque, so I don't think the same tradeoff with regards to contrast applies.

Comment 12

[Richard Marti (:Paenglab)]

Attachment: test-webext-theme.xpi

The icons are not only a bit darker, they use the text color as fill color.

Test with my test theme which makes the tab- and toolbar text red. And with this the icons are red too. Is this intended?

Comment 13

[Dão Gottwald [:dao]]

(In reply to Richard Marti (:Paenglab) from comment #12)
> Created attachment 8909836 [details]
> test-webext-theme.xpi
> 
> The icons are not only a bit darker, they use the text color as fill color.
> 
> Test with my test theme which makes the tab- and toolbar text red. And with
> this the icons are red too. Is this intended?

That's expected for now. I think there are plans to allow themes to customize the fill color separately from the text color.