1762913 - Restore changing the color of toplevel menubar items in inactive windows on Windows

Status: RESOLVED FIXED

(Toolkit :: Themes, defect, P1)

Description

[Dão Gottwald [:dao]]

Instead of removing the condition to stop catering for OS/2, bug 1757642 ended up removing the rule entirely: https://hg.mozilla.org/mozilla-central/rev/dbbcfe333292#l4.12

This looks to have been a misunderstanding of bug 1757642 comment 0.

Sarah, would you please look into this?

Comment 1

[Sarah Clements [:sclements]]

Oops, I did indeed misread Gijs' comment. Will get a patch ASAP.

Comment 2

[Sarah Clements [:sclements]]

I took a look at bug 1759510 and my patch removing the windows-specific menubar rule in bug 1757642 isn't the cause of that regression (there is however, a specific rule in that same file that needs to be wrapped in a not (prefers-contrast) media query. I can push a patch for that, but was this the only reason you filed this bug Dão (or did you want this rule I removed restored regardless)?

Comment 3

[:Gijs (he/him)]

(In reply to Sarah Clements [:sclements] from comment #2)

I took a look at bug 1759510 and my patch removing the windows-specific menubar rule in bug 1757642 isn't the cause of that regression (there is however, a specific rule in that same file that needs to be wrapped in a not (prefers-contrast) media query. I can push a patch for that, but was this the only reason you filed this bug Dão (or did you want this rule I removed restored regardless)?

Sorry, I caused this confusion so let me jump in: I came across 1759510 randomly and it reminded me of this bug, so I linked the two, thinking that they were so related that perhaps we could easily address 1759510 at the same time. (and it does sound like you've figured out how we'd fix that bug!)

This bug needs fixing regardless (and ideally uplifted to Fx100 as it's a regression there), because without the rule that got removed in 1757642, the menubar items don't have the correct colour in inactive windows. Does that make sense?

Comment 4

[Sarah Clements [:sclements]]

(In reply to :Gijs (he/him) from comment #3)

(In reply to Sarah Clements [:sclements] from comment #2)

I took a look at bug 1759510 and my patch removing the windows-specific menubar rule in bug 1757642 isn't the cause of that regression (there is however, a specific rule in that same file that needs to be wrapped in a not (prefers-contrast) media query. I can push a patch for that, but was this the only reason you filed this bug Dão (or did you want this rule I removed restored regardless)?

Sorry, I caused this confusion so let me jump in: I came across 1759510 randomly and it reminded me of this bug, so I linked the two, thinking that they were so related that perhaps we could easily address 1759510 at the same time. (and it does sound like you've figured out how we'd fix that bug!)

This bug needs fixing regardless (and ideally uplifted to Fx100 as it's a regression there), because without the rule that got removed in 1757642, the menubar items don't have the correct colour in inactive windows. Does that make sense?

Yes, thanks!

Comment 5

[Sarah Clements [:sclements]]

Attachment: Bug 1762913 - Restore inactive menubar style for Windows r=Gijs

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ca2447c80f4
Restore inactive menubar style for Windows r=Gijs

Comment 7

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

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

Comment 8

[Cristian Tuns]

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

Comment 9

[Sarah Clements [:sclements]]

Comment on attachment 9270871 [details]
Bug 1762913 - Restore inactive menubar style for Windows r=Gijs

Beta/Release Uplift Approval Request

• User impact if declined: Windows users will be affected since the menubar items don't have the correct color in inactive windows.
• 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: In Windows, check that the menubar items have the expected/correct colour in inactive windows
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Its adding one style back that was removed by accident (the condition wrapping it was supposed to be removed, not the entire style) but we didn't catch until FX100 moved to Beta.
• String changes made/needed: No

Comment 10

[Dianna Smith [:diannaS]]

Comment on attachment 9270871 [details]
Bug 1762913 - Restore inactive menubar style for Windows r=Gijs

Approved for 100.0b3

Comment 11

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5c6e5632e832

Comment 12

[Anca Soncutean, Desktop QA]

I don't see any menu bar style differences between a build with the fix vs one without it on an inactive window. Am I missing something here? Thank you.

Comment 13

[Sarah Clements [:sclements]]

I took a look, and if I have a default windows theme, and default FX theme, I see another condition in browser.css that also supplies the rule this patch does. And in other themes (like Alpine and Dark), there are specific color overrides that negates the rule in this patch. Gijs or Dao, can one of you please outline the steps to reproduce because I'm also not seeing a difference?

Comment 14

[Dão Gottwald [:dao]]

It's quite possible that this can't be reproduced in Firefox, indeed it looks like there's no case where we don't override the toolkit colors for menubars. There's another one in the Library window doing its own thing as well (and I think this has a higher specificity than the restored toolkit rule): https://searchfox.org/mozilla-central/rev/0e93b94f4c2045c6a5f5260ee48bbf7a94a993bc/browser/themes/windows/places/organizer.css#117

The fix is still valid but I guess we don't need to verify it.