1198415 - Search suggestions opt-in notification in the urlbar has low contrast, is hard to read

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Robin Andersen [:tecgirl]]

Attachment: nightly-poor-contrast.png

In a fresh install of Nightly 43.0a1, I was given this tooltip when entering into the URLbar. The light grey background makes it hard to read.

Comment 1

[Drew Willcoxon :adw]

Robin, what OS are you using?  Do you use some kind of high-contrast mode or other non-standard OS theme?

Comment 2

[Drew Willcoxon :adw]

Or a Firefox theme?  I couldn't reproduce this on Windows 7 with the built-in high-contrast OS themes or on OS X.  I only tested with the default Firefox theme though.

Comment 3

[Drew Willcoxon :adw]

Oh, it's lightweight themes.

Comment 4

[Drew Willcoxon :adw]

Attachment: use -moz-DialogText

Not sure which color we should be using exactly, but this works.  Tested on Windows 7 in default and high-contrast themes and on OS X.

Comment 5

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #1)
> Robin, what OS are you using?  Do you use some kind of high-contrast mode or
> other non-standard OS theme?

Comment 6

[Marco Bonardo [:mak] (away 22-26 Sept)]

If I'm not wrong, the autocomplete has background-color: -moz-Field; so likely this should use -moz-FieldText?

Comment 7

[Drew Willcoxon :adw]

Attachment: use -moz-FieldText

That works too.  It does look like autocomplete has a -moz-Field background, so -moz-FieldText would be the right color to use, but FWIW there's no visual difference that I can tell between -moz-FieldText and -moz-DialogText on Windows or OS X with non-standard Firefox and OS themes.

Comment 8

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/fx-team/rev/9d4b08fb58f0

Comment 9

[Drew Willcoxon :adw]

Robin, I'll leave the needinfo on you just to verify that you're using a non-default Firefox theme.

Comment 10

[Robin Andersen [:tecgirl]]

I am using the default theme.

Comment 11

[Drew Willcoxon :adw]

Hmm, interesting.  I can't explain then why your fresh install, as you say, looks like that, but the patch should fix it since it specifies a specific text color.  What OS and OS theme are you using?

Please try the new Nightly whenever this patch makes its way there and let us know whether it's still a problem.

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9d4b08fb58f0

Comment 13

[Dão Gottwald [:dao]]

Sorry, I'm backing this patch out because it made the text unreadable with the Ubuntu default theme: https://hg.mozilla.org/integration/fx-team/rev/d1ae9771dd06

Comment 14

[Drew Willcoxon :adw]

Attachment: Windows, OS X: -moz-FieldText; Linux: MenuText

OK, here are the colors used on Windows, OS X, and Ubuntu (without the backed-out patch):

Opt-in background
-----------------
Windows, OS X: urlbarSearchSuggestionsNotification.inc.css (as expected)
Ubuntu: A dark brown color.  I couldn't find where this comes from.

Opt-in foreground
-----------------
Windows, Ubuntu: panel { color: MenuText } (popup.css)
OS X: window { color: -moz-DialogText } (global.css)

Richlistbox item background
---------------------------
Windows, OS X: Transparent "browser style", don't know where this comes from.
Ubuntu: A dark brown color.  I couldn't find where this comes from.

Richlistbox item foreground
---------------------------
Windows, OS X: richlistbox { color: -moz-FieldText } (richlistbox.css)
Ubuntu: .autocomplete-richlistitem { color: MenuText } (autocomplete.css)

Ubuntu styles both the opt-in and richlistbox item background colors with a dark brown, even though urlbarSearchSuggestionsNotification.inc.css hardcodes a background color for the opt-in.  I couldn't find where that color comes from using the browser toolbox.  So on Ubuntu, we should use the same color as the richlistbox items, MenuText.  MenuText also happens to be the color that the opt-in inherits.

MenuText happens to be black on Windows and OS X too, so we could use that everywhere.  But maybe it's better to keep using -moz-FieldText, like the richlistbox items on those platforms, so that's what this patch does.

I tested MenuText on Ubuntu with lightweight themes too and it looks fine.  The panel's colors don't change at all.

Also tested on Fedora and it looks fine.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 8663820 [details] [diff] [review]
Windows, OS X: -moz-FieldText; Linux: MenuText

(In reply to Drew Willcoxon :adw from comment #14)
> Opt-in background
> -----------------
> Windows, OS X: urlbarSearchSuggestionsNotification.inc.css (as expected)
> Ubuntu: A dark brown color.  I couldn't find where this comes from.

From the panel: http://hg.mozilla.org/mozilla-central/annotate/197af2fb7e29/toolkit/themes/linux/global/popup.css#l18

> Ubuntu styles both the opt-in and richlistbox item background colors with a
> dark brown, even though urlbarSearchSuggestionsNotification.inc.css
> hardcodes a background color for the opt-in.

That background color is mostly transparent (7% opacity).

> color comes from using the browser toolbox.  So on Ubuntu, we should use the
> same color as the richlistbox items, MenuText.  MenuText also happens to be
> the color that the opt-in inherits.

So if it's already inherited, why did you add a rule for it in this patch?

>--- a/browser/themes/shared/urlbarSearchSuggestionsNotification.inc.css
>+++ b/browser/themes/shared/urlbarSearchSuggestionsNotification.inc.css
>@@ -1,10 +1,11 @@
> #PopupAutoCompleteRichResult > hbox[anonid="search-suggestions-notification"] {
>   border-bottom: 1px solid hsla(210, 4%, 10%, 0.14);
>+  color: -moz-FieldText;

Please move this to autocomplete.css where -moz-field is set as a background. (Except for Linux where -moz-field is set but not used.)

Comment 16

[Drew Willcoxon :adw]

Attachment: patch v3

(In reply to Dão Gottwald [:dao] from comment #15)
> From the panel:
> http://hg.mozilla.org/mozilla-central/annotate/197af2fb7e29/toolkit/themes/
> linux/global/popup.css#l18

The background color?  From the -moz-appearance?

> That background color is mostly transparent (7% opacity).

Oh, right.

> So if it's already inherited, why did you add a rule for it in this patch?

I thought it was necessary to override changes made by themes, which is the point of this bug.  But I tested just now without specifying a color at all, and that only seems to be a problem on OS X, where the inherited color is -moz-DialogText.  So lightweight themes can set -moz-DialogText but not MenuText?

Comment 17

[Drew Willcoxon :adw]

Comment on attachment 8663865 [details] [diff] [review]
patch v3

Let's see who gets to it first.

Comment 18

[Dão Gottwald [:dao]]

Comment on attachment 8663865 [details] [diff] [review]
patch v3

(In reply to Drew Willcoxon :adw from comment #16)
> Created attachment 8663865 [details] [diff] [review]
> patch v3
> 
> (In reply to Dão Gottwald [:dao] from comment #15)
> > From the panel:
> > http://hg.mozilla.org/mozilla-central/annotate/197af2fb7e29/toolkit/themes/
> > linux/global/popup.css#l18
> 
> The background color?  From the -moz-appearance?

yep

> > So if it's already inherited, why did you add a rule for it in this patch?
> 
> I thought it was necessary to override changes made by themes, which is the
> point of this bug.  But I tested just now without specifying a color at all,
> and that only seems to be a problem on OS X, where the inherited color is
> -moz-DialogText.  So lightweight themes can set -moz-DialogText but not
> MenuText?

Lightweight themes can't change those colors at all, plus Robin said she was using the default theme.

We still don't even know what OS she is on, so who knows if this patch will help in her case... but r+ because this change makes sense either way.

Comment 19

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/fx-team/rev/1e7f9effc62b

(In reply to Dão Gottwald [:dao] from comment #18)
> Lightweight themes can't change those colors at all, plus Robin said she was
> using the default theme.

True, although FWIW I was able to reproduce by using lightweight themes.

Comment 20

[Marco Bonardo [:mak] (away 22-26 Sept)]

please remember we need to uplift this.

Comment 21

[Drew Willcoxon :adw]

Robin, which OS are you using, and are you using a non-standard OS theme?

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1e7f9effc62b

Comment 23

[Marco Bonardo [:mak] (away 22-26 Sept)]

Comment on attachment 8663865 [details] [diff] [review]
patch v3

Approval Request Comment
[Feature/regressing bug #]: Search suggestions in the awesomebar
[User impact if declined]: The opt-in notification bar is unreadable with some theme combinations
[Describe test coverage new/current, TreeHerder]: Nightly
[Risks and why]: low risk, trivial css change
[String/UUID change made/needed]: none

Comment 24

[Drew Willcoxon :adw]

Robin, you didn't actually answer my questions.  Which OS are you using, and are you using a non-standard OS theme?

Comment 25

[Robin Andersen [:tecgirl]]

(In reply to Drew Willcoxon :adw from comment #24)
> Robin, you didn't actually answer my questions.  Which OS are you using, and
> are you using a non-standard OS theme?

I am on Mac OS Yosemite 10.10.5 (14F27), and not using a non-standard OS theme.

Comment 26

[Drew Willcoxon :adw]

Thanks.  One more thing: now that this patch is on Nightly, could you please test again with the latest Nightly and let us know whether it's fixed?

Comment 27

[Robin Andersen [:tecgirl]]

LGTM

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8663865 [details] [diff] [review]
patch v3

Verified in Nightly; let's uplift to aurora.

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/53f7553a1c8b