1449774 - Improve contrast of security text color on dark themes

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Tim Nguyen :ntim]

Attachment: Screen Shot 2018-03-29 at 12.56.47 AM.png

Comment 1

[Tim Nguyen :ntim]

Attachment: Bug 1449774 - Improve contrast of security text color on dark themes.

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

Comment 2

[Tim Nguyen :ntim]

Comment on attachment 8963416 [details]
Bug 1449774 - Improve contrast of security text color on dark themes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232316/diff/1-2/

Comment 3

[Johann Hofmann [:johannh]]

Comment on attachment 8963416 [details]
Bug 1449774 - Improve contrast of security text color on dark themes.

https://reviewboard.mozilla.org/r/232316/#review237880

Hm the red doesn't fare perfectly on https://contrastchecker.com/ (with the black that is on OSX popups right now) but I don't think we can make it any brighter.

I'd say it's definitely an improvement, thanks!

Comment 4

[Tim Nguyen :ntim]

Comment on attachment 8963416 [details]
Bug 1449774 - Improve contrast of security text color on dark themes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232316/diff/2-3/

Comment 5

[Tim Nguyen :ntim]

(In reply to Johann Hofmann [:johannh] from comment #3)
> Comment on attachment 8963416 [details]
> Bug 1449774 - Improve contrast of security text color on dark themes.
> 
> https://reviewboard.mozilla.org/r/232316/#review237880
> 
> Hm the red doesn't fare perfectly on https://contrastchecker.com/ (with the
> black that is on OSX popups right now) but I don't think we can make it any
> brighter.
> 
> I'd say it's definitely an improvement, thanks!

Thanks for the review! I left out the non-secure color, since contrast checker says it regressed :)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a69be6eee5a3
Improve contrast of security text color on dark themes. r=johannh

Comment 7

[Tim Nguyen :ntim]

Comment on attachment 8963416 [details]
Bug 1449774 - Improve contrast of security text color on dark themes.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1417880
[User impact if declined]: Bad visibility of security text when using a dark theme that styles the application menu like https://addons.mozilla.org/en-GB/firefox/addon/arc-dark-theme-webextension/ (See bug screenshot)

[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[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, low risk change
[Why is the change risky/not risky?]: CSS only
[String changes made/needed]: no

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8963416 [details]
Bug 1449774 - Improve contrast of security text color on dark themes.

I just realized we can't really uplift this to beta, since there's no lwt-popup-brighttext attribute there.

Comment 9

[Andrei Ciure[:aciure]]

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