1468645 - 62 Nightly, Focused item in Menu Bar is wrong highlight color in Windows 7 with Classic Theme

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[MarkRH]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180613100146

Steps to reproduce:

Just click on menu item in Nightly build 20180613100146


Actual results:

It's highlighted in dark blue with black text.  Normally an indented greyish color.  Looks like this now: https://i.imgur.com/XB6SI8a.png


Expected results:

It should look like this:

https://i.imgur.com/lflOpak.png

This is a followup to this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1437052

I ran a moz regression and got:

2018-06-13T17:18:56: INFO : Narrowed inbound regression window from [bbc9b2b5, 55ac4685] (3 builds) to [bbc9b2b5, b015b36a] (2 builds) (~1 steps left)
2018-06-13T17:18:56: DEBUG : Starting merge handling...
2018-06-13T17:18:56: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=b015b36a001c72400eec5734dc9fcabc4ec74bff&full=1
2018-06-13T17:18:56: DEBUG : Found commit message:
Bug 1437052 - Consolidate #main-menubar > menu appearance and color rules. r=Felipe

MozReview-Commit-ID: 7RicXHVrbEQ

2018-06-13T17:18:56: INFO : The bisection is done.
2018-06-13T17:18:56: INFO : Stopped

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Dão, Felipe, could one of you take a look? The text seems unreadable with these colors.

Comment 2

[Jim Mathies [:jimm]]

Attachment: XB6SI8a.png

Comment 3

[MarkRH]

Attachment: Windows 7 Pro Classic Theme menu issue

The latest Nightly Build ID 20180626100238 and 62beta3 have the same problem, even with clean profiles.

Comment 4

[MarkRH]

Can any userChrome.css or userContent.css override these colors?  Tried the following with no effect:

#main-menubar > menu {
  -moz-appearance: none !important;
  background-color: transparent !important;
}
#main-menubar > menu:-moz-any(:hover,[open],[_moz-menuactive]) {
  background: Highlight !important;
  color: HighlightText !important;
}

Comment 5

[MarkRH]

I did get the userChrome.css override to work after removing some other conflicting code.  At least the text is white now.

Comment 6

[Mike Taylor [:miketaylr]]

Hey Dao, do we think we can get this into 62 still?

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Hey Mike, is this a P1 and do we need it for 62?

Comment 8

[Mike de Boer [:mikedeboer]]

Yes, it is. (I'm following Dão's assessment here.)

Dão, could you take a look at this? There are two options here:
1. Backout the patch from bug 1437052 from the 62 (beta) branch. This seems low effort to me and will give you more time to worry about and work on a proper fix here.
2. Prepare a fix in here after all and uplift to beta - also feasible I think, because the patch will be small and low-risk.

Your call!

Comment 9

[Dão Gottwald [:dao]]

Attachment: Bug 1468645 - Set active menu bar item color to -moz-menuhovertext instead of -moz-menubarhovertext since we've not using -moz-appearance:menuitem anymore.

Review commit: https://reviewboard.mozilla.org/r/259910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/259910/

Comment 10

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8995480 [details]
Bug 1468645 - Set active menu bar item color to -moz-menuhovertext instead of -moz-menubarhovertext since we've not using -moz-appearance:menuitem anymore.

https://reviewboard.mozilla.org/r/259910/#review267076

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/653f9be185f8
Set active menu bar item color to -moz-menuhovertext instead of -moz-menubarhovertext since we've not using -moz-appearance:menuitem anymore. r=Felipe

Comment 12

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/653f9be185f8

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 8995480 [details]
Bug 1468645 - Set active menu bar item color to -moz-menuhovertext instead of -moz-menubarhovertext since we've not using -moz-appearance:menuitem anymore.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1437052
[User impact if declined]: see comment 0
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: see comment 0
[List of other uplifts needed for the feature/fix]: bug 1479874 but I just included that followup in this patch
[Is the change risky?]: not much
[Why is the change risky/not risky?]: it's possible there other edge cases I missed, but overall this should be moving in the right direction
[String changes made/needed]: /

Comment 14

[laszlo.bialis]

Hi!

I've checked the issue on the latest Nightly v63.0a1 Build ID: 20180805231147 on Win7 Professional x64 and the results were the following:

1. The text color is changed to white as mentioned in comment 5, but doesn't look like the screenshot from the bug description at Expected results. (see attached: classic theme menu bar.png)

2. The issue is still reproducible (see attached: classic theme menu bar repro.png)  with the following steps:
    - Open latest Nightly on Win7 with Classic Theme
    - Change Win 7 Theme to Basic while the browser is open
    - Change Win 7 Theme back to Classic while the browser is open
    - Inspect the menu bar
If one after the above steps closes the browser and reopens it again the menu bar highlighting is done like in the "classic theme menu bar.png"

Dao, could you please tell me if the situation from number 1 above is the expected behavior? What about situation number 2?

Comment 15

[laszlo.bialis]

Attachment: classic theme menu bar.png

Comment 16

[laszlo.bialis]

Attachment: classic theme menu bar repro.png

Comment 17

[Dão Gottwald [:dao]]

Changing the OS theme while Firefox is running causes various quirks that may look like this bug but are different issues. For this bug we're only interested in the case where Firefox is restarted after the theme change.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8995480 [details]
Bug 1468645 - Set active menu bar item color to -moz-menuhovertext instead of -moz-menubarhovertext since we've not using -moz-appearance:menuitem anymore.

Fix for readability of menus, let's uplift for beta 16.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/34af42d33444

Comment 20

[laszlo.bialis]

I've checked again on the latest Nightly v63.0a1 Build ID: 20180807220134 on Win7 Professional x64 and the result was the same as in attachment "classic theme menu bar.png". Marking as verified on Nightly. 
I will check back on the issue after uplift to Beta is done.

Comment 21

[Daniel Bodea [:danibodea]]

Hello,

I have verified this issue on Beta v62.0b18. The text is now white when the button/menu is highlighted. 
Marking as verified in 62 and removing qe-verify+ flag.