1503304 - toolbar_text doesn't always fall back to textcolor

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P1)

Description

[Dão Gottwald [:dao]]

See https://phabricator.services.mozilla.com/D9998#inline-42760

Comment 1

[Dão Gottwald [:dao]]

Bumping the priority because we shouldn't have to set this optional color in the first place but it seems the fallback is broken e.g. when enabling Windows 10 accent colors in the titlebar.

Comment 2

[Dão Gottwald [:dao]]

Attachment: Bug 1503304 - Make toolbar_text fall back to textcolor. r?ntim

Comment 3

[Tim Nguyen :ntim]

Hi Dão, do you have a screenshot of what the current patch is fixing ?

Comment 4

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #3)
> Hi Dão, do you have a screenshot of what the current patch is fixing ?

The problem is that despite :-moz-lwtheme we sometimes set a text color on the root element that is different from --lwt-text-color. I think we only do this in compacttheme.css right now but I'm not sure.

Comment 5

[Tim Nguyen :ntim]

I think it makes more sense to set --lwt-text-color/--lwt-accent-color to -moz-win-accentcolor/-moz-win-accentcolortext when relevant and set toolbar_text to the right value.

What do you think ?

Comment 6

[Dão Gottwald [:dao]]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #5)
> I think it makes more sense to set --lwt-text-color/--lwt-accent-color to
> -moz-win-accentcolor/-moz-win-accentcolortext when relevant

It doesn't sound like this would change anything with regards to this bug, and also make it impossible to potentially use the original theme colors in a different context that isn't the title bar / window background.

> and set toolbar_text to the right value.

We shouldn't have to set toolbar_text when we already set textcolor...

Comment 7

[Tim Nguyen :ntim]

I was thinking my suggestion was better in terms of converting the themes to the theme API later on, but I guess it doesn't matter much for now and we can do that later.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ea314ab963f
Make toolbar_text fall back to textcolor. r=ntim

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6ea314ab963f

Comment 10

[Mikael Håkansson]

Found this bug with mozregression.

This bug causes the find bar (ctrl+f) to use the accentcolor (for background) and textcolor from a theme (example: https://addons.mozilla.org/sv-SE/firefox/addon/furfox-tail-twister/), which can make close button, text and so on the same color as the background color.

This makes the find bar more or less unusable with all themes I have tried (~10).

Adding below to userChrome.css fixes the problem locally for me.

:root:-moz-lwtheme {
--toolbar-color: inherit !important;
}

Comment 11

[Dão Gottwald [:dao]]

(In reply to Mikael Håkansson from comment #10)

This is filed as bug 1506913.