1196878 - Security error unreadable with dark theme

Status: VERIFIED FIXED

Description

[J. Ryan Stinnett [:jryans] (use ni?)]

Created attachment 8650627 [details]
Security panel with error text

The error text in the security panel is basically unreadable in the dark theme.

See attached screenshot.

Comment 1

[Giovanny Gongora [:gioyik]]

Hi Ryan,

I want to work on this bug. Could you tell me where to make the change? I am checking the code and didn't found a css rule where to add the change for the color, so I think it's necessary create a new one. Added to it, could you tell me what is the color (variable with the color) you want for the text?

Thanks

Comment 2

[Louie Lu]

Created attachment 8651511 [details] [diff] [review]
Bug 1196878.patch

fixed Bug 1196878, add .theme-dark #security-error-message for font color.

Comment 3

[J. Ryan Stinnett [:jryans] (use ni?)]

(In reply to Giovanny Gongora [:gioyik] from comment #1)
> Hi Ryan,
> 
> I want to work on this bug. Could you tell me where to make the change? I am
> checking the code and didn't found a css rule where to add the change for
> the color, so I think it's necessary create a new one. Added to it, could
> you tell me what is the color (variable with the color) you want for the
> text?

It looks like grapherd jumped ahead with a patch for this one, so it's probably best to look for another bug to try.  In general though, you can use the Browser Toolbox[1] to inspect the tools and tweak style like regular web content.  The various DevTools theme variables are listed on MDN[2].

As for which one...  I wouldn't really know myself until I've looked at other similar text types.  I'll be doing that momentarily as part of review.

[1]: https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox
[2]: https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors

Comment 4

[J. Ryan Stinnett [:jryans] (use ni?)]

Comment on attachment 8651511 [details] [diff] [review]
Bug 1196878.patch

This change looks good to me!  However, can you correct the commit message to say "r=jryans" at the end?

Also, I'll run a try build to ensure it's safe to land:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e8354fa7104

Comment 5

[Louie Lu]

Created attachment 8652870 [details] [diff] [review]
1196878.patch

fixed reviewer in patch commit

Comment 6

[J. Ryan Stinnett [:jryans] (use ni?)]

Comment on attachment 8652870 [details] [diff] [review]
1196878.patch

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

Great, this looks good to me.

The try run looks good too, so let's mark it for landing.

Thank you for working on this!

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/dc2cd8274358

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Khalid Syfullah Zaman [:khalid32]]

I have reproduced this bug on Nightly 43.0a1 (2015-08-20) on ubuntu 14.04 LTS, 32 bit!

The bug's fix is now verified on Latest Beta 43.0b1!

Build ID: 20151103023037
User Agent: Mozilla/5.0 (X11; Linux i686; rv:43.0) Gecko/20100101 Firefox/43.0

[bugday-20151104]

Comment 10

[Sauradeep Dutta]

I have reproduced this bug on Nightly 43.0a1 (2015-08-20) on windows 8.1 , 64 bit!

The bug's fix is now verified on Latest Beta 48.0b9!

Build ID: 20160718142219
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0

[bugday-20160727]