1385277 - Dark theme ev certificate doesn't have enough contrast

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Jonathan Kingston [:jkt] he/him]

Attachment: Selection_744.png

In the latest changes to make dark theme black instead of dark blue the EV box is pretty unreadable.

I have attached a screenshot.

:johannh suggested there should be a hover state which is missing: http://design.firefox.com/people/shorlander/photon/Mockups/linux.html

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: Bug 1385277 - Use lighter green for ev text in dark theme.

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

Comment 2

[Dale Harvey (:daleharvey)]

Shorlander confirmed this color change in IRC

Comment 3

[Dale Harvey (:daleharvey)]

Comment on attachment 8892834 [details]
Bug 1385277 - Use lighter green for ev text in dark theme.

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

Comment 4

[Tim Nguyen :ntim]

Comment on attachment 8892834 [details]
Bug 1385277 - Use lighter green for ev text in dark theme.

https://reviewboard.mozilla.org/r/163838/#review169288

::: browser/themes/shared/compacttheme.inc.css:281
(Diff revision 2)
>    box-shadow: none !important;
>  }
>  
> +%ifdef MOZ_PHOTON_THEME
> +#urlbar[pageproxystate="valid"] > #identity-box.verifiedIdentity > #identity-icon-labels:-moz-lwtheme-brighttext {
> +    color: #30e60b;

nit: 2 space indentation

Comment 5

[Dale Harvey (:daleharvey)]

Comment on attachment 8892834 [details]
Bug 1385277 - Use lighter green for ev text in dark theme.

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

Comment 6

[Dale Harvey (:daleharvey)]

Ah editor wasnt setup on my new env, cheers

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8892834 [details]
Bug 1385277 - Use lighter green for ev text in dark theme.

https://reviewboard.mozilla.org/r/163838/#review170280

::: browser/themes/shared/compacttheme.inc.css:279
(Diff revision 3)
>    color: inherit !important;
>    border: 1px solid var(--chrome-nav-bar-controls-border-color) !important;
>    box-shadow: none !important;
>  }
>  
> +%ifdef MOZ_PHOTON_THEME

no need to use the MOZ_PHOTON_THEME ifdef any longer

Comment 9

[Dale Harvey (:daleharvey)]

Comment on attachment 8892834 [details]
Bug 1385277 - Use lighter green for ev text in dark theme.

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b76ef688a02
Use lighter green for ev text in dark theme. r=dao

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8b76ef688a02

Comment 12

[Deac Alin]

Tested on Windows 10 x 64, Mac OSX 10.12.5 and Ubuntu 16.04 x64 on Latest Nightly 57.0a1 Build ID 20170821100350

The (green) EV bar text has now the same color as the padlock in Dark theme. Also the identity block background gets brighter when hovered. I will mark this as Verified Fixed. Thank you