1437564 - [Windows] Unreadable Landscape/Portrait buttons’ labels in Print preview when in dark theme

Status: VERIFIED FIXED

(Toolkit :: Themes, defect, P2)

Description

[Anca Soncutean, Desktop QA]

Attachment: screenshot showing the issue.png

[Affected versions]:
- 58.0.2 Release (20180206200532)
- 59.0b8 Beta (20180208193705)
- 60.0a1 Nightly (2018-02-11)

[Affected platforms]:
- Windows 10 x64

[Steps to reproduce]:
1. Open Firefox
2. Customize the browser’s theme to dark (Hamburger menu - Customize)
3. Go to Print preview (Hamburger menu - Print)
4. Observe the Portrait button
5. Click on the Landscape button

[Expected result]:
- The Portrait/ Landscape labels are visible all the time.

[Actual result]:
- The Portrait/ Landscape labels are visible only when not selected.

[Regression range]:
Last good revision: e5bd43668caaa8dbb97b5709b3af725299e93faf
First bad revision: 829a5b7de7cf408137e4ec270f1ccb9a153de411
 Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=e5bd43668caaa8dbb97b5709b3af725299e93faf&tochange=829a5b7de7cf408137e4ec270f1ccb9a153de411

[Additional notes]:
- The issue is not reproducible on Ubuntu 16.04 x64

Comment 1

[Anca Soncutean, Desktop QA]

It seams that your patch from bug 1368672 regressed this behavior. Can you please take a look?

Comment 2

[Mike Taylor [:miketaylr]]

Too late for 58.

Comment 3

[Dão Gottwald [:dao]]

Attachment: Bug 1437564 - Stop setting a text color for checked toolbarbuttons on Windows since the native background is mostly transparent and the color should be inherited.

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

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8955094 [details]
Bug 1437564 - Stop setting a text color for checked toolbarbuttons on Windows since the native background is mostly transparent and the color should be inherited.

https://reviewboard.mozilla.org/r/224254/#review230232

Looks good, thanks!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5736dea94b68
Stop setting a text color for checked toolbarbuttons on Windows since the native background is mostly transparent and the color should be inherited. r=johannh

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/5736dea94b68

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Given that it's a one-line CSS fix, this seems look a good uplift candidate for Monday's RC1 build. Please request release approval if you agree :)

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8955094 [details]
Bug 1437564 - Stop setting a text color for checked toolbarbuttons on Windows since the native background is mostly transparent and the color should be inherited.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1368672
[User impact if declined]: see  attachment 8950247 [details]
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: don't think so
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: trivial fix
[String changes made/needed]: /

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8955094 [details]
Bug 1437564 - Stop setting a text color for checked toolbarbuttons on Windows since the native background is mostly transparent and the color should be inherited.

Trivial CSS fix, approved for 59rc1.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/afe411646f89 (FIREFOX_59b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/2e428240c044

Comment 11

[Gabi Cheta [:Gabi] Desktop QA]

Verified as fixed on Windows 10x64 with 59.0 and 60.0a1.

Comment 12

[ovidiu boca[:Ovidiu]]

Based on previous comments this bug was verified.