Closed
Bug 1409341
Opened 7 years ago
Closed 7 years ago
[Beta] [Windows 7] Incomplete hover state on tabs in default theme
Categories
(Firefox :: Theme, defect, P1)
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)
(deleted),
text/x-review-board-request
|
dao
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details |
[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•7 years 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•7 years 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•7 years 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•7 years ago
|
status-firefox58:
--- → fixed
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
Assignee | ||
Comment 7•7 years 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.
Comment 9•7 years ago
|
||
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•7 years ago
|
||
bugherder uplift |
Comment 11•7 years ago
|
||
Hello,
Is this fix in 57.0b10? It doesn't seem fixed there.
Flags: needinfo?(ryanvm)
Comment 12•7 years ago
|
||
Yes, the commit in comment 10 was included in 57b10. Sounds like Johann needs to follow-up here.
Flags: needinfo?(ryanvm) → needinfo?(jhofmann)
Assignee | ||
Comment 13•7 years ago
|
||
Works for me in 57b10. The tab background gets a little lighter on hover, which is what we're looking for.
Flags: needinfo?(jhofmann)
Comment 14•7 years ago
|
||
Alexandru, can you please take another look?
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(alexandru.simonca)
Resolution: --- → FIXED
Comment 15•7 years ago
|
||
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.
Description
•