Closed Bug 1409341 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Theme, defect, P1)

57 Branch
All
Windows 7
defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox56 --- unaffected
firefox57 + verified
firefox58 --- verified

People

(Reporter: johannh, Assigned: johannh)

References

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(3 files)

[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).
Flags: qe-verify+
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 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
Attachment #8919253 - Flags: review?(dao+bmo) → review+
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
Attachment #8919253 - Flags: approval-mozilla-beta?
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.
Tracking 57+, visual bug on popular platform
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.
Attachment #8919253 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Hello,
Is this fix in 57.0b10? It doesn't seem fixed there.
Flags: needinfo?(ryanvm)
Yes, the commit in comment 10 was included in 57b10. Sounds like Johann needs to follow-up here.
Flags: needinfo?(ryanvm) → needinfo?(jhofmann)
Works for me in 57b10. The tab background gets a little lighter on hover, which is what we're looking for.
Flags: needinfo?(jhofmann)
Alexandru, can you please take another look?
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: needinfo?(alexandru.simonca)
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(alexandru.simonca)
You need to log in before you can comment on or make changes to this bug.