1879415 - Firefox Translation UI shows a solid-orange square, where it should have text showing the language that we translated to

Status: VERIFIED FIXED

(Firefox :: Translations, defect, P3)

Description

[Daniel Holbert [:dholbert]]

STR:

1. Load a page that Firefox offers to translate. I'm using:
   https://archive.is/2024.01.30-203406/https://www.luzernerzeitung.ch/wirtschaft/kriminalitaet-die-zahnbuersten-greifen-an-das-sind-die-aktuellen-cybergefahren-und-so-koennen-sie-sich-schuetzen-ld.2569480
2. Accept Firefox's offer to translate the page (you may have to click the translation button in the location bar)

ACTUAL RESULTS:
After the translation completes, Firefox's location bar shows an orange translation-icon next to a solid orange square. (If you zoom in on a screenshot and look very very closely, you can barely make out that it has the text en, but it's nearly impossible to see -- it's slightly-darker-orange text on a slightly-brighter-orange background. My eyedropper tool says the text is rgb(225,80,31), and the background is rgb(233,84,32).)

EXPECTED RESULTS:
The solid-orange square should have text on top of it, to show you what language we translated into (en in my case)

mozregression says this is a regression from bug 1869299, specifically this range which just has one commit:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=34533b3add99fe26cbd60a7fd8d0c60966e5fd32&tochange=89da90df398f5aee595e14748b0241ede2a9fd4d

(The one commit is https://hg.mozilla.org/integration/autoland/rev/89da90df398f5aee595e14748b0241ede2a9fd4d )

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot of expected rendering (before the regression)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot of actual/buggy rendering (after the regression)

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1869299

:emilio, since you are the author of the regressor, bug 1869299, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Daniel Holbert [:dholbert]]

I'm using the "System Theme -- Auto" (the default).
If I change to any of our non-default themes (e.g. Light or Dark), then the issue goes away.

It looks like the styling here comes from this rule:
https://searchfox.org/mozilla-central/rev/54c9b4896fdc1e858cd4942f306d877f1f3d195e/browser/themes/shared/urlbar-searchbar.css#659-661

#translations-button-locale {
  background-color: var(--toolbarbutton-icon-fill-attention);
  color: var(--toolbar-field-background-color);

With the default theme, that maps to these values, as shown in Browser Console "rules" pane for this element:

#translations-button-locale {
  background-color: AccentColor;
  color: light-dark(rgba(0, 0, 0, .05), rgba(0, 0, 0, .3));

Notably the light version of the color there (what I'm getting) is almost entirely transparent (rgba(0, 0, 0, .05)), which is why it's not showing up.

Comment 5

[Daniel Holbert [:dholbert]]

That nearly-transparent color was indeed added in the regressing commit, here:
https://hg.mozilla.org/integration/autoland/rev/89da90df398f5aee595e14748b0241ede2a9fd4d#l1.40

https://searchfox.org/mozilla-central/rev/54c9b4896fdc1e858cd4942f306d877f1f3d195e/browser/themes/linux/browser.css#35

--toolbar-field-background-color: light-dark(rgba(0, 0, 0, .05), rgba(0, 0, 0, .3));

The issue here seems to be a set of assumptions-mismatches:
(A) It looks like this commit 89da90df398f5aee595e14748b0241ede2a9fd4d is assuming that this nearly-transparent color is OK as a background-color because it'll only be used for backgrounds (and presumably we're assuming that, as a background, it'll be layered on top of some other background-color that provides sufficient contrast).

(B) The translations code is assuming that it can swap the background/foreground colors here and still have reasonable contrast (note in the first code-snippet in comment 4, we've got a fill color-variable as our background, and a background color-variable as our foreground).

These assumptions are in conflict, which is what's causing this issue.

Comment 6

[Daniel Holbert [:dholbert]]

Note that we do have other partially-transparent background-colors variables, so assumption (A) doesn't seem entirely out of place. Here's a searchfox link to find some of them:
https://searchfox.org/mozilla-central/search?q=--.*background-color.*rgba&path=&case=true&regexp=true

So, focusing on assumption (B)... Greg, see above -- the translations UI seems to be using a background color as if it were a foreground color, which is troublesome now that the background-color in question is almost entirely transparent. Perhaps we can change things such that we aren't using that approach? (Alternately, if this background/foreground swapping is common & expected-to-work, maybe there's another guaranteed-not-to-be-transparent background color we should be using as our foreground color here?)

Comment 7

[Daniel Holbert [:dholbert]]

platform-wise: I'm seeing this on Ubuntu 22.04, with a "light" system theme (which remains the default I think?) and a fresh Firefox profile (default configuration). So this might be affecting ~all Ubuntu users.

It reproduces similarly with Ubuntu's "dark" system-theme, too, but it's not quite as bad -- you can barely see the en text there, since per the above-quoted light-dark expression, we use not-quite-as-transparent-of-a-color (alpha=0.3) when you've got a dark system theme. So we end up with a slightly-darker-orange "en" over a bright orange background.

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: screenshot of actual/buggy rendering (after the regression) with dark system theme

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1879415 - Fix translation locale background. r=#theme,gregtatum

Using a background for text color is not great.

Provide a text color to draw on top of the toolbarbutton-icon-attention
color.

Comment 10

[Bob Hood [:bhood]]

Emilio, can we please get Priority/Severity settings applied?

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62f1f3f7fe02
Fix translation locale background. r=gregtatum

Comment 12

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/62f1f3f7fe02

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox123 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9379164 [details]
Bug 1879415 - Fix translation locale background. r=#theme,gregtatum

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivial-ish CSS fix.
• String changes made/needed: none
• Is Android affected?: No

Comment 15

[Ciprian Georgiu, Desktop QA]

I have reproduced this issue with Ubuntu 20 x64 on an affected Firefox 123.0, using STR from comment 0.

The issue is verified as fixed on latest Nightly 124.0a1 with Ubuntu 20 x64.

Comment 16

[Pascal Chevrel:pascalc]

Comment on attachment 9379164 [details]
Bug 1879415 - Fix translation locale background. r=#theme,gregtatum

Low risk, approved for our planned dot release next week, thanks.

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/0a04037bcef0

Comment 18

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-release/rev/0a04037bcef0

Comment 19

[Ciprian Georgiu, Desktop QA]

This is also verified as fixed on Firefox 123.0.1 with Ubuntu 20.04 x64.