1854486 - Menu Bar activation in fullscreen results in undesirable space under inactive tabs on dark theme

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Anca Soncutean, Desktop QA]

Affected versions

• 119.0a1 (2023-09-21)

Affected platforms

• macOS 13

Steps to reproduce
Preconditions:

• Have the dark theme set at the os level

1. Open several pages
2. Enter fullscreen
3. Place the cursor at the top of the screen to trigger the Menu bar appearance

Expected result

• All tabs shift their position without any visual issue.

Actual result

• The active Tab keeps its height, while the rest of the unfocused ones have an unnecessary space displayed under (see notes).

Regression range

• Last good revision: 751ce10ffa95f4706e709666d45db519e48786c7
• First bad revision: 85ed967e285f304f7709461aec0d33ca72805791
• Pushlog:
  https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=751ce10ffa95f4706e709666d45db519e48786c7&tochange=85ed967e285f304f7709461aec0d33ca72805791
• Potential regressor: 1843044

Notes

• See the screencast issue
• Not reproducible if any about page is in focus

Comment 1

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

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

:emilio, since you are the author of the regressor, bug 1843044, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1854486 - Unify how we set the toolbox background across platforms. r=dao

Turns out macOS relies on the background being set in the toolbox rather
than :root because we slide the toolbox down entirely when the menubar
shows up in full-screen.

Since this is also the set up we have for lightweight themes and Linux,
and we can change Windows to do the same now that Aero is not a thing
anymore, clean-up the code and move all the toolbox background code to a
shared place.

In the future if we want to do something like Mica / Aero on Windows
again, we could do this trivially by setting --toolbox-non-lwt-bgcolor:
transparent or so on the relevant platform's file.

I can write a simpler patch for beta if needed.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45493b54f70e
Unify how we set the toolbox background across platforms. r=dao,desktop-theme-reviewers

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/45493b54f70e

Comment 5

[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. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
• If no, please set status-firefox119 to wontfix.

For more information, please visit BugBot documentation.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9354752 [details]
Bug 1854486 - Unify how we set the toolbox background across platforms. r=dao

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: bug 1855151
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): It's a non-trivial styling change, but it is a simplification and we're early in the beta cycle, so probably worth trying to get in rather than crafting a hacky patch for beta.
• String changes made/needed: none
• Is Android affected?: No

Comment 7

[Dianna Smith [:diannaS]]

Comment on attachment 9354752 [details]
Bug 1854486 - Unify how we set the toolbox background across platforms. r=dao

Approved for 119.0b3

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/eaf70e9733a5

Comment 9

[Anca Soncutean, Desktop QA]

Verifed fixed with Fx 120.0a1 (2023-09-28) and treeherder build Fx 119.0b3 on macOS 13.