1768376 - Main menubar active color at win10

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[alstjr7375]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:100.0) Gecko/20100101 Firefox/100.0

Steps to reproduce:

Hover or open main menubar's menu

• Win10
• Light theme (It works well in dark themes.)

Actual results:

Background color is -moz-menuhover

Expected results:

The following color should be applied.

 #main-menubar > menu[_moz-menuactive="true"],
.titlebar-button:hover {
  background-color: hsla(0,0%,0%,.12);
}

https://searchfox.org/mozilla-central/source/browser/themes/windows/browser-aero.css#186

Comment 1

[alstjr7375]

More context:

v99 is works well
https://user-images.githubusercontent.com/25581533/167306360-7da184af-882d-4ada-8412-462e95549d06.png
https://user-images.githubusercontent.com/25581533/167307100-6bf4a0db-aeae-4235-83c9-9f1fa9958e54.png

v101 is not works
https://user-images.githubusercontent.com/25581533/167306297-c302e89a-5dc8-48fa-a617-8e601f9de411.png
https://user-images.githubusercontent.com/25581533/167307017-64a8e036-c099-482d-b623-5d856922d914.png

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Theme' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Alin Ilea, Desktop Test Engineering]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:100.0) Gecko/20100101 Firefox/100.0

Hi,

I am able to reproduce the issue in release 100, beta 101 and the latest nightly 102.0a1 (2022-05-13) using Windows 10.
Here is the regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f4823585e985134965ed14c18af62b57346625de&tochange=60f515867f95f47b025f1bb280c6332a9f394321

Thanks for the report.

Comment 4

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

:sfoster, since you are the author of the regressor, bug 1753767, could you take a look?
For more information, please visit auto_nag documentation.

Comment 6

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

Possibly related to or a dupe of bug 1768380. I'm looking into this.

Comment 7

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

Ok, I see what happened here. Prior to this patch, the rules from browser-aero.css were included close to the end of browser/themes/windows/browser.css: https://hg.mozilla.org/mozilla-central/file/4cb7108079fd542e3cfb1c8a8992fe837b917b72/browser/themes/windows/browser.css, and the converted that %include to an @import, and moved it to the top of the file. This had the effect that rules in browser.css would now supercede those with the same specificity defined in browser-aero.css

Specifically, we have a rule for firefox on windows generally that gives the active menu its color in browser.css:

#main-menubar > menu[_moz-menuactive="true"] {
  background-color: -moz-menuhover;
  color: -moz-menuhovertext;
}

And we have a rule in browser-aero.css which should override this:

#main-menubar > menu[_moz-menuactive="true"],
.titlebar-button:hover {
  background-color: hsla(0,0%,0%,.12);
}

This order got reversed, so the browser-aero.css rule is no longer applied in this case.

Comment 8

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

Attachment: Bug 1768376 - Restore previous order of aero-related rules by loading browser-aero.css for XP_WIN only from browser.xhtml. r?dao!

Comment 9

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

The severity field is not set for this bug.
:dao, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 10

[Pascal Chevrel:pascalc]

Sam, could we get that reviewed and landed before the merge? Thanks

Comment 11

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

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

Sam, could we get that reviewed and landed before the merge? Thanks

I've just put a revised patch up for review, so I think this can happen - and my reviewer is also aware we want this landed before merge day.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/22b5dfe319e3
Restore previous order of aero-related rules by loading browser-aero.css for XP_WIN only from browser.xhtml. r=dao

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/22b5dfe319e3

Comment 14

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

The patch landed in nightly and beta is affected.
:sfoster, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 15

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

Comment on attachment 9277839 [details]
Bug 1768376 - Restore previous order of aero-related rules by loading browser-aero.css for XP_WIN only from browser.xhtml. r?dao!

Beta/Release Uplift Approval Request

• User impact if declined: Colors of some elements like menus don't match expected theme colors
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment #0, this only affects some window configurations
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Restores previous CSS rule order, no functional changes
• String changes made/needed: None
• Is Android affected?: No

Comment 16

[alstjr7375]

Thank you for fix bug!!

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9277839 [details]
Bug 1768376 - Restore previous order of aero-related rules by loading browser-aero.css for XP_WIN only from browser.xhtml. r?dao!

Thanks for sorting this out. Approved for 101.0rc2.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/8c9eeeff7a2a

Comment 19

[Alin Ilea, Desktop Test Engineering]

Verified - Fixed in latest Nightly 102.0a1 (build id: 20220525185636) and Fx101.0rc2 (build id: 20220525210143) using Windows 10.