1409341 - [Beta] [Windows 7] Incomplete hover state on tabs in default theme

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Johann Hofmann [:johannh]]

[Tracking Requested - why for this release]:
Visible deficiency in primary UI on our most popular platform.

On Beta 57 only, tabs do not change their background when they're hovered in Windows 7 (and I believe Windows 8 as well). This leaves the small strip at the top as the only hover indicator. Not good.

This issue was fixed in bug 1391539 under the assumption that we'd uplift it to Beta, but we decided not to.

We should uplift the non-risky parts that fix this bug to Beta. I'll prepare a patch.

Note that this is a separate issue from light theme not getting proper hover state on Windows 7, which is bug 1409340 and takes a different approach to solve (and is lower priority, IMO).

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1409341 - Apply background background to tab instead of tab-background on Windows 7.

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

Comment 2

[Johann Hofmann [:johannh]]

dao, this is code that we already landed as part of bug 1391539, so this is an uplift-only patch for Beta. I guess I wouldn't have to get review on this technically, but another pair of eyes on it would be useful.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8919253 [details]
Bug 1409341 - Apply background background to tab instead of tab-background on Windows 7.

https://reviewboard.mozilla.org/r/190154/#review195410

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8919253 [details]
Bug 1409341 - Apply background background to tab instead of tab-background on Windows 7.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1399498
[User impact if declined]: It's hard to perceive hover state on tabs in Windows 7 and 8.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes, it's been in Nightly for over a week as part of bug 1391539.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No
[Why is the change risky/not risky?]: This is a relatively straightforward CSS change that has been baking on Nightly since bug 1391539 landed. Bug 1391539 caused a bunch of other regressions (which is why we didn't uplift it), but this part of the patch was not responsible for any of them.
[String changes made/needed]: None

Comment 5

[Johann Hofmann [:johannh]]

Attachment: Win 7 Beta - Hover state

Comment 6

[Johann Hofmann [:johannh]]

Attachment: Win 7 Nightly - Hover state

Comment 7

[Johann Hofmann [:johannh]]

I attached two screenshots from Beta and Nightly to emphasize that this is not a polish issue, but an actual bug that we should fix in 57, IMO.

Comment 8

[Marcia Knous [:marcia]]

Tracking 57+, visual bug on popular platform

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8919253 [details]
Bug 1409341 - Apply background background to tab instead of tab-background on Windows 7.

New regression & seems safe, taking it.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Alexandru Simonca [:asimonca], Release Desktop QA]

Hello,
Is this fix in 57.0b10? It doesn't seem fixed there.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Yes, the commit in comment 10 was included in 57b10. Sounds like Johann needs to follow-up here.

Comment 13

[Johann Hofmann [:johannh]]

Works for me in 57b10. The tab background gets a little lighter on hover, which is what we're looking for.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Alexandru, can you please take another look?

Comment 15

[Alexandru Simonca [:asimonca], Release Desktop QA]

Yeah, checked again and it looks good. Sorry about that. 
Verified fixed in latest Nightly 58.0a1 (id: 20171020100426) and Beta 57.0b10 on Windows 7 x64.