1771313 - Paginator background is translucent with the alpenglow light theme

Status: VERIFIED FIXED

(Toolkit :: Printing, defect)

Description

[Anca Soncutean, Desktop QA]

Attachment: screenshot issue.png

Affected versions

• Fx 101
• Fx 102.0a1

Affected platforms

• Windows 10
• Ubuntu 20.04
• macOS 12.1

Steps to reproduce

1. Set Alpenglow theme at the browser level from about:addons#themes
2. Open any page
3. Open Print Preview (CTRL + P)
4. Observe the Paginator

Expected result

• The paginator background is opaque.

Actual result

• The paginator is translucid, the elements are hard to distinguish.

Regression range

• Not a regression, reproducible since paginator implementation.

Comment 1

[Daniel Holbert [:dholbert]]

I can reproduce this, but only if I have a "light" OS theme (usually the default).

In Ubuntu 22.04, this is configurable in the Ubuntu settings app, "Appearance" tab on the left. After going there, if I choose "Light", then I can reproduce this bug. If I choose "Dark", then the paginator has a dark-purple background.

The component in question has this as its background:

background-color: var(--toolbar-bgcolor);

https://searchfox.org/mozilla-central/rev/21e50ef42340c421c5ae2df29f2509d17339c239/toolkit/components/printing/content/printPagination.css#32

And in this case, the Browser Toolbox tells me that --toolbar-bgcolor is a partially-transparent color, rgba(255, 255, 255, 0.76) (actually 0.075999 but I suspect it's 0.76 with some floating-point imprecision).

I don't know where that color comes from, but it seems like we should either make that an opaque color (in the theme), or we need to add some sort of plain backdrop behind it in this piece of UI (separating it from the content).

Looks like sfoster has hg-blame on that line. Over to him for thoughts (and reclassifying to Toolkit|Printing which I think is where we're tracking bugs in the UI components of this print dialog; though ultimately this might want to move to Firefox|Theme or something like that).

Comment 2

[Daniel Holbert [:dholbert]]

Browser toolbox tells me that this --toolbar-bgcolor value is "Inherited from html#main-window`, which I think means it comes from the theme.

And, aha, it looks like it's set here:

    "colors": {
[...]
      "toolbar": "hsla(0, 0%, 100%, .76)",

https://searchfox.org/mozilla-central/rev/21e50ef42340c421c5ae2df29f2509d17339c239/browser/themes/addons/alpenglow/manifest.json#35

...vs. further down, for "dark_theme", it has:

      "toolbar": "hsla(254, 46%, 21%, .96)",

https://searchfox.org/mozilla-central/rev/21e50ef42340c421c5ae2df29f2509d17339c239/browser/themes/addons/alpenglow/manifest.json#92

(Still slightly transparent, but darker & mostly-opaque.)

Also: I noticed we have one definition of this CSS variable in our source with a much-more transparent color (alpha of 0.4):

:root:-moz-lwtheme {
  --toolbar-bgcolor: rgba(255,255,255,.4);

https://searchfox.org/mozilla-central/rev/21e50ef42340c421c5ae2df29f2509d17339c239/browser/themes/shared/browser-shared.css#113
I don't know under what circumstances this value with a=0.4 would get used, but if it ever does, that would presumably an even-more-severe version of this bug. And in any case: this is all a signal that we need to account for the possibility that this CSS variable can be semi-transparent (since we have 3 examples here from Mozilla's own theme CSS, the 3 lines I quoted above in this comment).

So I think we need to add an explicit background color behind the --toolbar-bgcolor: here, or we need to use a different CSS variable.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: screenshot showing the actual toolbar with alpenglow and os light theme (to demonstrate why toolbar-bgcolor is transparent)

Side note: it looks like this CSS Variable is intentionally/correctly transparent, when applied up on the actual Firefox toolbar. It seems like the point of the transparency is to let the alpenglow toolbar/titlebar background-image shine through the grayish toolbar, as you can see in this screenshot.

Comment 4

[Daniel Holbert [:dholbert]]

(The toolbar and the background image are all content that we [or the theme author] controls, of course; and we know that the background image isn't complex enough to cause visual confusion when it shines through the partially-transparent toolbar-bgcolor, which is why a transparent color value is ~fine up there. But when the content underneath this background-color is a print-previewed document, as in the comment 0 screenshot, then we have no guarantee over how-complex this content will be and how good-vs-muddy it looks through this possibly-transparent toolbar-bgcolor color.)

Comment 5

[Sam Foster [:sfoster] (he/him)]

I do like the visual cue of the pagination toolbar being distinctly browser chrome, by virtue of using the --toolbar-bgcolor We should be able to put a opaque color behind it. I think that's at least worth a try first before looking at using a different variable. Let me poke at it a bit.

Comment 6

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1771313 - Add an opaque background behind the print preview paginator buttons. r?mstriemer!

Comment 7

[Sam Foster [:sfoster] (he/him)]

Attachment: bug-1771313-before-after.png

Before (above) and After (below) with the attachment 9280580 [details] patch applied, using a light system theme and the alpenglow browser theme.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54b137d5e9a3
Add an opaque background behind the print preview paginator buttons. r=mstriemer

Comment 9

[bszekely]

https://hg.mozilla.org/mozilla-central/rev/54b137d5e9a3

Comment 10

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

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

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

For more information, please visit auto_nag documentation.

Comment 11

[Sam Foster [:sfoster] (he/him)]

:dholbert, if that patch does what you expected I'll request uplift here.

Comment 12

[Daniel Holbert [:dholbert]]

(In reply to Sam Foster [:sfoster] (he/him) from comment #11)

:dholbert, if that patch does what you expected I'll request uplift here.

It does what I expected, yeah (visually). Just tested with current mozilla-central and confirmed that it seems to be fixed. Thanks!

(I think we're spinning up final betas now/soon, so it's possible this might not make it in time; but that'd be fine if it has to wait, since we've been living with this bug for nearly 20 releases now.)

Comment 13

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 9280580 [details]
Bug 1771313 - Add an opaque background behind the print preview paginator buttons. r?mstriemer!

Beta/Release Uplift Approval Request

• User impact if declined: The pagination toolbar in print preview is transparent, and in some themes it looks visually cluttered with the preview document being visible through it
• 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: Use print preview with the Alpenglow/light theme. The previewed document should not be visible through the toolbar used to navigate through the pages in the preview.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): CSS fix to just this one toolbar.
  We've shipped with this behavior for many releases so it can also wait to ride the trains.
• String changes made/needed: None
• Is Android affected?: No

Comment 14

[Sam Foster [:sfoster] (he/him)]

This isn't a plea to sneak this into a late beta. It can just as easily wait, but the bot asked so I answered :)

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9280580 [details]
Bug 1771313 - Add an opaque background behind the print preview paginator buttons. r?mstriemer!

We actually have no beta left, the last one shipped this morning.

Comment 16

[Anca Soncutean, Desktop QA]

Verifed fixed with Fx 103.0a1 (2022-06-16) on Windows 10, macOS 12.1 and Ubuntu 20.04.

Comment 17

[Daniel Holbert [:dholbert]]

(In reply to Pascal Chevrel:pascalc from comment #15)

We actually have no beta left, the last one shipped this morning.

That's fine; we can call this wontfix for the Firefox 102 release, per comment 12 and comment 14. --> Marking as such.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Hi Sam, did you want to nominate this for ESR102 uplift? Please do so if yes.

Comment 19

[Sam Foster [:sfoster] (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)

Hi Sam, did you want to nominate this for ESR102 uplift? Please do so if yes.

I don't think so. It didn't make 102 and is enough of a low-severity, low-frequency edge-case that I think it can wait.