1455950 - "Learn more" links in arrow panels have very low contrast on dark themes

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P2)

Description

[Tim Nguyen :ntim]

Attachment: Screen Shot 2018-04-22 at 1.03.20 PM.png

Comment 1

[Marion Daly [:mdaly]]

Is this a result of the work the students?

Comment 2

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

Yes, I suspect so - Connor (MSU student) worked on bug 1408121, which I believe causes this behaviour.

Comment 3

[Harry Twyford]

Attachment: Bug 1455950 - Increased contrast on dark theme UI text links.

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

Comment 4

[Harry Twyford]

Pushed small fix that improves Learn More... and other UI text links on dark theme. I'll address related Bug 1451827 when addressing Bug 1453722.

Comment 5

[Tim Nguyen :ntim]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

https://reviewboard.mozilla.org/r/244802/#review251576

::: browser/themes/shared/compacttheme.inc.css:35
(Diff revision 1)
> +:root:-moz-lwtheme-brighttext .text-link {
> +  color: #45a1ff;
> +}

Other dark themes are also affected. This shouldn't be specific to compacttheme.css

Comment 7

[Harry Twyford]

Attachment: Bug 1455950 - Increased contrast on dark theme UI text links.

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

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8979687 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

https://reviewboard.mozilla.org/r/245830/#review252056

::: browser/themes/shared/customizableui/panelUI.inc.css:753
(Diff revision 1)
>    background-color: rgba(48,230,11,.2);
>  }
>  
> +:root[lwt-popup-brighttext] .text-link {
> +  color: #45a1ff;
> +}

Please fold both patches into one, and move this to browser.inc.css rather than panelUI.inc.css.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

https://reviewboard.mozilla.org/r/244802/#review252060

Comment 10

[Harry Twyford]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

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

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

https://reviewboard.mozilla.org/r/244802/#review252244

::: browser/themes/shared/browser.inc.css:26
(Diff revision 2)
>  :root[sessionrestored]:-moz-lwtheme {
>    transition: @themeTransition@;
>  }
>  
> +/* Increase contrast of UI links on dark themes */
> +:root[lwt-popup-brighttext] .text-link {

This is a bit fragile as it assumes that all text-links are inside popups, or that UI surfaces outside of popups use a similar background. Let's instead use this selector for now:

:root[lwt-popup-brighttext] panel .text-link

Comment 12

[Harry Twyford]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

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

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8579efe6135a
Increased contrast on dark theme UI text links. r=dao

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/8579efe6135a

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this patch when you get a chance.

Comment 16

[Harry Twyford]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

This is a small CSS fix to improve the contrast of UI text links on dark theme. 

[Feature/Bug causing the regression]: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1408121

[User impact if declined]:
Users may be unable to make out some UI links on the new dark theme. This should be in the Beta stream since dark theme is currently in Beta, and this should be a part of the initial release launch of dark theme.

[Is this code covered by automated tests?]:
N/A

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]:
No, although if you want to reproduce it, you can visit a site that requires DRM (open.spotify.com) and click the DRM icon to the left of the address bar to see the "Learn More..." link.

[List of other uplifts needed for the feature/fix]:
N/A

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
It's a small CSS fix and does not impact performance. There are also relatively few text UI links affected by this bug, in the event that there is some unforeseen side effect.

[String changes made/needed]:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8976704 [details]
Bug 1455950 - Increased contrast on dark theme UI text links.

Trivial fix to improve dark theme support. Approved for 61.0b9.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5ff34e06b8e6

Comment 19

[Oana Botisan, Desktop QA]

I managed to reproduce the issue using an older version of Nightly (2018-04-22) on Windows 10 x64.
I retested everything on latest Nightly 62.0a1 and beta 61.0b9 using Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64, but the bug is not reproducing anymore. The text can be seen clearly.