Closed Bug 1490937 Opened Last year Closed Last year

Tab titles for selected tabs in modal dialogs have too little contrast

Categories

(Toolkit :: Themes, defect, P1)

Unspecified
macOS
defect

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 + verified
firefox64 --- verified

People

(Reporter: mikedeboer, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(5 files)

This could be a nmore general theme bug for in-content styles, but this seemed like the most appropriate place to file it.
The attached screenshot should make things very clear.

STR:
1. Go to Preferences, Privacy & Security.
2. Scroll to the bottom of that page and click the 'View Certificates...' button.
3. In the modal dialog that pops up, switch between tabs and notice how badly readable the selected tab label (caption) is right now.

Bonus points for fixing the horizontal alignment of said labels!
It looks pretty bad here on my macOS machine too. This is pretty rough.
I'm searching for a regressor.
Keywords: regression
 8:28.14 INFO: Last good revision: 3ef0a331591001bf9bcf5b31ad7fa9b89749e690
 8:28.14 INFO: First bad revision: 2f9c90bd0f2f12f580c59602893cb430acda68a0
 8:28.14 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3ef0a331591001bf9bcf5b31ad7fa9b89749e690&tochange=2f9c90bd0f2f12f580c59602893cb430acda68a0

Hey dao, got a few minutes to look at this?
Blocks: 1446168
Flags: needinfo?(dao+bmo)
I don't consider our certificate stuff primary UI, but the fact that the contrast is so bad here might render the tabs useless for some users. I'm inclined to mark this a P1.
Priority: -- → P1
[Tracking Requested - why for this release]:
Seems to have regressed in 63, makes UI harder to use.
Assignee: nobody → dao+bmo
Component: Preferences → Themes
Flags: needinfo?(dao+bmo)
OS: Unspecified → Mac OS X
Product: Firefox → Toolkit
Attached patch patchSplinter Review
Attachment #9009033 - Flags: review?(mdeboer)
Mike, could you review dao's patch? We will probably want to uplift it and I'd prefer not too late in the cycle. Thanks!
Flags: needinfo?(mdeboer)
Attachment #9009033 - Flags: review?(mdeboer) → review+
Keywords: checkin-needed
Flags: needinfo?(mdeboer)
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/934ae0ea69fc
Fix in-content tab styling on Mac since tabbox.css is now loaded as a document stylesheet. r=mikedeboer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/934ae0ea69fc
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Flags: qe-verify+
Mike says he still sees the bug.
Status: RESOLVED → REOPENED
Flags: qe-verify+
Resolution: FIXED → ---
Attached patch followup fixSplinter Review
Attachment #9010572 - Flags: review?(mdeboer)
Comment on attachment 9010572 [details] [diff] [review]
followup fix

Review of attachment 9010572 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this works for me! Thanks!
Attachment #9010572 - Flags: review?(mdeboer) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f41f9765f63c
Followup fix for in-content tab styling on Mac since tabbox.css is now loaded as a document stylesheet. r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/f41f9765f63c
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → FIXED
Flags: qe-verify+
Brindusa, can your team verify the fix in nightly?

Dão, is this ready for uplift to beta?
Flags: needinfo?(dao+bmo)
Flags: needinfo?(brindusa.tot)
This issue was reproduced on Nightly v64.0a1 from 2018-09-01 and the fix was validated in Nightly v64.0a1 from 2018-09-25 on Mac OS X 10.13.6.
Attached patch patch for upliftSplinter Review
Approval Request Comment
[Feature/Bug causing the regression]: bug 1446168
[User impact if declined]: see comment 0
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: /
[Is the change risky?]: no
[Why is the change risky/not risky?]: straightforward CSS-only fix
[String changes made/needed]: /
Flags: needinfo?(dao+bmo)
Flags: needinfo?(brindusa.tot)
Attachment #9011801 - Flags: approval-mozilla-beta?
Status: RESOLVED → VERIFIED
Comment on attachment 9011801 [details] [diff] [review]
patch for uplift

Uplift approved for 63 beta 10, thanks.
Attachment #9011801 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
When trying to graft this bug for uplift I ran into conflicts on this file toolkit/themes/osx/global/in-content/common.css.

Can you please take a look?
Flags: needinfo?(pascalc)
Flags: needinfo?(dao+bmo)
(In reply to Dorel Luca [:dluca] from comment #19)
> When trying to graft this bug for uplift I ran into conflicts on this file
> toolkit/themes/osx/global/in-content/common.css.
> 
> Can you please take a look?

Seems to work just fine over here:

dao@x1:~/mozilla/beta$ hg import --no-commit https://bug1490937.bmoattachments.org/attachment.cgi?id=9011801
applying https://bug1490937.bmoattachments.org/attachment.cgi?id=9011801
dao@x1:~/mozilla/beta$ hg stat
M toolkit/themes/osx/global/in-content/common.css
M toolkit/themes/shared/in-content/common.inc.css
dao@x1:~/mozilla/beta$
Flags: needinfo?(dao+bmo)
Flags: needinfo?(pascalc)
This issue is not verified in Beta v63.0b11. Bug fully validated. Thank you.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.