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

VERIFIED FIXED

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
a month ago
a month ago

People

(Reporter: johannh, Assigned: johannh)

Tracking

57 Branch
All
Windows 7
Points:
---

Firefox Tracking Flags

(firefox56 unaffected, firefox57+ verified, firefox58 verified)

Details

(Whiteboard: [reserve-photon-visual])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Assignee)

Description

a month ago
[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+
Comment hidden (mozreview-request)
(Assignee)

Comment 2

a month ago
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

a month ago
mozreview-review
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+
(Assignee)

Comment 4

a month ago
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?
(Assignee)

Updated

a month ago
status-firefox58: --- → fixed
(Assignee)

Comment 5

a month ago
Created attachment 8919398 [details]
Win 7 Beta - Hover state
(Assignee)

Comment 6

a month ago
Created attachment 8919399 [details]
Win 7 Nightly - Hover state
(Assignee)

Comment 7

a month ago
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
tracking-firefox57: ? → +
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+

Comment 10

a month ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/f3271256298d
status-firefox57: affected → fixed
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.
status-firefox57: fixed → affected
Flags: needinfo?(ryanvm) → needinfo?(jhofmann)
(Assignee)

Comment 13

a month ago
Works for me in 57b10. The tab background gets a little lighter on hover, which is what we're looking for.
status-firefox57: affected → fixed
Flags: needinfo?(jhofmann)
Alexandru, can you please take another look?
Status: ASSIGNED → RESOLVED
Last Resolved: a month 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
status-firefox57: fixed → verified
status-firefox58: fixed → verified
Flags: qe-verify+
Flags: needinfo?(alexandru.simonca)
You need to log in before you can comment on or make changes to this bug.