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

Status: VERIFIED FIXED

(Toolkit :: Themes, defect, P1)

Description

[Mike de Boer [:mikedeboer]]

Attachment: Preferences_and_Enter_Bug__Firefox.png

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!

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Screen Shot 2018-09-13 at 10.15.36 AM.png

It looks pretty bad here on my macOS machine too. This is pretty rough.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

I'm searching for a regressor.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

 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?

Comment 4

[Mike Conley (:mconley) (:⚙️)]

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.

Comment 5

[:Gijs (he/him)]

[Tracking Requested - why for this release]:
Seems to have regressed in 63, makes UI harder to use.

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch

Comment 7

[Pascal Chevrel:pascalc]

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!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/934ae0ea69fc

Comment 10

[Dão Gottwald [:dao]]

Mike says he still sees the bug.

Comment 11

[Dão Gottwald [:dao]]

Attachment: followup fix

Comment 12

[Mike de Boer [:mikedeboer]]

Comment on attachment 9010572 [details] [diff] [review]
followup fix

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

Yes, this works for me! Thanks!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/f41f9765f63c

Comment 15

[Julien Cristau [:jcristau]]

Brindusa, can your team verify the fix in nightly?

Dão, is this ready for uplift to beta?

Comment 16

[Daniel Bodea [:danibodea]]

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.

Comment 17

[Dão Gottwald [:dao]]

Attachment: patch for uplift

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]: /

Comment 18

[Pascal Chevrel:pascalc]

Comment on attachment 9011801 [details] [diff] [review]
patch for uplift

Uplift approved for 63 beta 10, thanks.

Comment 19

[Dorel Luca [:dluca]]

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?

Comment 20

[Dão Gottwald [:dao]]

(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$

Comment 21

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b94d1d0eaf9e

Comment 22

[Daniel Bodea [:danibodea]]

This issue is not verified in Beta v63.0b11. Bug fully validated. Thank you.