Blair McBride [:Unfocused] (UNAVAILABLE) Reporter
	Description • 10 years ago

The checkboxes and radio buttons in about:preferences lack any indication they're checked/selected when using High Contrast mode, making the two states indistinguishable. See attached screenshot.

	Jefferson
	Comment 1 • 10 years ago

Possibly the same issue as bug 582951 and bug 575974 (both for non-chrome content pages).

	Tim Nguyen :ntim
	Comment 2 • 10 years ago

I'm planning on using the SVG here : https://bug1014208.bugzilla.mozilla.org/attachment.cgi?id=8427995

And I'm planning to use Highlight as high contrast color. This looks similar to original color on Windows, which is great, but it doesn't look similar at all on OSX or Linux.

So, Is there a solution to this ? or should I simply use the hardcoded color (the checkmark would be visible in that case, but not respecting the system settings).

	Jared Wein [:jaws] (please needinfo? me)
	Comment 3 • 10 years ago

We may want to try to get bug 425598 fixed, as that will allow us to use the desired colors at all times except for high contrast themes. I don't know of a special system color that we can use that will keep the shade of blue that is desired.

	Dão Gottwald [:dao]
	Comment 4 • 10 years ago

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #3)
> We may want to try to get bug 425598 fixed, as that will allow us to use the
> desired colors at all times except for high contrast themes. I don't know of
> a special system color that we can use that will keep the shade of blue that
> is desired.

That sounds like a perfect use case for the default theme metric. I.e. use the custom color for the default theme, use a system color otherwise.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 5 • 10 years ago

(In reply to Dão Gottwald [:dao] from comment #4)
> That sounds like a perfect use case for the default theme metric. I.e. use
> the custom color for the default theme, use a system color otherwise.

Huh, for some reason I was under the belief that the -moz-windows-default-theme metric matched to true even with high contrast themes, but I just tested it out and my memory was wrong.

Looks like this bug can move forward then. Tim, can you add some rules to the styling that do the following:

#ifdef XP_WIN
@media not all and (-moz-windows-default-theme) {
  .checkbox { list-style-image: url(checkbox.svg); }
}
#endif

and then the checkbox.svg can just use -moz-DialogText normally and GrayText when disabled. Just like we did for the dropdown.

	Richard Marti (:Paenglab) Assignee
	Comment 6 • 10 years ago

Tim, sorry to chime in to your bug and you don't mind, but it's a rainy day and I had some time.

Jared what do you think about this? 

Classic is also not a -moz-windows-default-theme and the used rule makes the radio and checkmark also under Classic black instead of the blue. So this would make bug 425598 still preferable to be fixed.

	Dão Gottwald [:dao]
	Comment 7 • 10 years ago

Comment on attachment 8523417 [details] [diff] [review]
HC-Radio-Check.patch

> xul|*.checkbox-check[checked] {
>-  background-image: url("chrome://global/skin/in-content/check.png"),
>-                    /* !important needed to override toolkit !important rule */
>-                    linear-gradient(#fff, rgba(255,255,255,0.8)) !important;
>+  list-style-image: url("chrome://global/skin/in-content/check.svg#check");
>+  /* !important needed to override toolkit !important rule */
>+  background-image: linear-gradient(#fff, rgba(255,255,255,0.8)) !important;
> }
> 
>+%ifdef XP_WIN
>+@media not all and (-moz-windows-default-theme) {
>+  xul|*.checkbox-check[checked] {
>+    list-style-image: url("chrome://global/skin/in-content/check.svg#check-hc");
>+  }
>+}
>+%endif

-moz-dialogText can be white, so using that on a hardcoded white background isn't sane.

Note that this bug also affects Linux with browser.display.use_document_colors=false.

	Tim Nguyen :ntim
	Comment 8 • 10 years ago

Comment on attachment 8523417 [details] [diff] [review]
HC-Radio-Check.patch

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

::: toolkit/themes/shared/in-content/radio.svg
@@ +8,5 @@
> +    use {
> +      fill: #2292d0;
> +    }
> +    use[id$="-hc"] {
> +      fill: -moz-DialogText;

Highlight would match better for third-party themes, and would still work with High contrast themes.

	Richard Marti (:Paenglab) Assignee
	Comment 9 • 10 years ago

(In reply to Tim Nguyen [:ntim] from comment #8)
> 
> Highlight would match better for third-party themes, and would still work
> with High contrast themes.

Yeah, this would be on Classic a dark blue and on the dark HC themes a different color to the border/text like we also have on normal themes.

Jared, what do you think?

	Tim Nguyen :ntim
	Comment 10 • 10 years ago

Comment on attachment 8523417 [details] [diff] [review]
HC-Radio-Check.patch

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

::: toolkit/themes/shared/in-content/check.svg
@@ +12,5 @@
> +      fill: -moz-DialogText;
> +    }
> +  </style>
> +  <defs style="display: none;">
> +    <path id="check-shape" d="M 9.39,16.5 16.28,6 14.77,4.5 9.37,12.7 6.28,9.2 4.7,10.7 z"/>

You can use a <circle> tag here.

	Richard Marti (:Paenglab) Assignee
	Comment 11 • 10 years ago

I'm turning this to a review now.

I'm using now Highlight as color on non-default themes to address Dão's comment 7. With this it imitates more the original Chameleon's style with the different colors of the borders and inner elements.

	Dão Gottwald [:dao]
	Comment 12 • 10 years ago

Comment on attachment 8527162 [details] [diff] [review]
HC-radio-check.patch

> xul|*.checkbox-check[checked] {
>-  background-image: url("chrome://global/skin/in-content/check.png"),
>-                    /* !important needed to override toolkit !important rule */
>-                    linear-gradient(#fff, rgba(255,255,255,0.8)) !important;
>+  list-style-image: url("chrome://global/skin/in-content/check.svg#check");
>+  /* !important needed to override toolkit !important rule */
>+  background-image: linear-gradient(#fff, rgba(255,255,255,0.8)) !important;

How is this different from the background-image for the unchecked state?

> }
> 
>+%ifdef XP_WIN
>+@media not all and (-moz-windows-default-theme) {
>+  xul|*.checkbox-check[checked] {
>+    list-style-image: url("chrome://global/skin/in-content/check.svg#check-hc");
>+  }
>+}
>+%endif

What ensures that this is actually visible on the background? Highlight can be any color just like -moz-dialogText, so this doesn't really address comment 7.

What about Linux?

	Richard Marti (:Paenglab) Assignee
	Comment 13 • 10 years ago

(In reply to Dão Gottwald [:dao] from comment #12)
> Comment on attachment 8527162 [details] [diff] [review]
> HC-radio-check.patch
> 
> > xul|*.checkbox-check[checked] {
> >-  background-image: url("chrome://global/skin/in-content/check.png"),
> >-                    /* !important needed to override toolkit !important rule */
> >-                    linear-gradient(#fff, rgba(255,255,255,0.8)) !important;
> >+  list-style-image: url("chrome://global/skin/in-content/check.svg#check");
> >+  /* !important needed to override toolkit !important rule */
> >+  background-image: linear-gradient(#fff, rgba(255,255,255,0.8)) !important;
> 
> How is this different from the background-image for the unchecked state?

Good catch, I remove it.

> >+%ifdef XP_WIN
> >+@media not all and (-moz-windows-default-theme) {
> >+  xul|*.checkbox-check[checked] {
> >+    list-style-image: url("chrome://global/skin/in-content/check.svg#check-hc");
> >+  }
> >+}
> >+%endif
> 
> What ensures that this is actually visible on the background? Highlight can
> be any color just like -moz-dialogText, so this doesn't really address
> comment 7.

With Windows HC-themes the background-image isn't shown. This is also why the check and radio aren't shown and need the list-style-image. Then the Highlight, and also the -moz-dialogText, are drawn over the -moz-dialog of http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/global.css#32. I could also go back to -moz-dialogText which better correlates to -moz-dialog than Highlight.

> What about Linux?

Linux still uses #2292d0.

	Dão Gottwald [:dao]
	Comment 14 • 10 years ago

(In reply to Richard Marti (:Paenglab) from comment #13)
> > What ensures that this is actually visible on the background? Highlight can
> > be any color just like -moz-dialogText, so this doesn't really address
> > comment 7.
> 
> With Windows HC-themes the background-image isn't shown. This is also why
> the check and radio aren't shown and need the list-style-image. Then the
> Highlight, and also the -moz-dialogText, are drawn over the -moz-dialog of
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/
> global.css#32. I could also go back to -moz-dialogText which better
> correlates to -moz-dialog than Highlight.

My question wasn't limited to high-contrast themes. As for high-contrast, is the background-*color* not shown either? (This would mean that we effectively flip browser.display.use_document_colors for these themes.)

> > What about Linux?
> 
> Linux still uses #2292d0.

And is that sufficient? See comment 7.

	Richard Marti (:Paenglab) Assignee
	Comment 15 • 10 years ago

(In reply to Dão Gottwald [:dao] from comment #14)
> (In reply to Richard Marti (:Paenglab) from comment #13)
> > > What ensures that this is actually visible on the background? Highlight can
> > > be any color just like -moz-dialogText, so this doesn't really address
> > > comment 7.
> > 
> > With Windows HC-themes the background-image isn't shown. This is also why
> > the check and radio aren't shown and need the list-style-image. Then the
> > Highlight, and also the -moz-dialogText, are drawn over the -moz-dialog of
> > http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/
> > global.css#32. I could also go back to -moz-dialogText which better
> > correlates to -moz-dialog than Highlight.
> 
> My question wasn't limited to high-contrast themes. As for high-contrast, is
> the background-*color* not shown either? (This would mean that we
> effectively flip browser.display.use_document_colors for these themes.)

I've added background: HighlightText to give a contrasting color for the background. This would then apply third party themes.

> > > What about Linux?
> > 
> > Linux still uses #2292d0.
> 
> And is that sufficient? See comment 7.

What do you propose to do for Linux?

	Richard Marti (:Paenglab) Assignee
	Comment 16 • 10 years ago

Updated to tip.

	Dão Gottwald [:dao]
	Comment 17 • 10 years ago

Comment on attachment 8529874 [details] [diff] [review]
HC-radio-check.patch

>+%ifdef XP_WIN
>+@media not all and (-moz-windows-default-theme) {
>+  xul|*.checkbox-check[checked] {
>+    list-style-image: url("chrome://global/skin/in-content/check.svg#check-hc");
>+    background: HighlightText;
>+  }
>+}
>+%endif

So this doesn't work well with browser.display.use_document_colors=false, because we'll use the image but not the background color. As I understand it, using -moz-fieldText or -moz-dialogText rather than Highlight for the icon (and -moz-field or -moz-dialog for the background) is going to be the safer choice, then. And we'll need to do this on linux too.

nit: please rename -hc to -native

	Richard Marti (:Paenglab) Assignee
	Comment 18 • 10 years ago

Changed the native checkmark- and radio-button icons to -moz-dialogText and -moz-dialog.

Linux is using now always the native colors. Is this okay like this?

	Tim Nguyen :ntim
	Comment 19 • 10 years ago

(In reply to Richard Marti (:Paenglab) from comment #18)
> Created attachment 8532026 [details] [diff] [review]
> HC-radio-check.patch
> 
> Changed the native checkmark- and radio-button icons to -moz-dialogText and
> -moz-dialog.
> 
> Linux is using now always the native colors. Is this okay like this?

I'm afraid -moz-dialog will make the checkbox look disabled on windows 3rd party themes, since -moz-dialog is usually gray. I think -moz-field and -moz-fieldText would be better here.

	Dão Gottwald [:dao]
	Comment 20 • 10 years ago

Comment on attachment 8532026 [details] [diff] [review]
HC-radio-check.patch

>--- a/toolkit/themes/shared/in-content/common.inc.css
>+++ b/toolkit/themes/shared/in-content/common.inc.css
>@@ -393,49 +393,53 @@
> xul|*.checkbox-check {
...
>+  background-image: linear-gradient(#fff, rgba(255,255,255,0.8)) !important;
...
> }

>+  xul|*.checkbox-check[checked] {
...
>+    background: -moz-dialog;
>+  }

The background image is close to opaque and since you use !important, you're not removing it with "background", so the background color won't have much effect. This seems relevant for dark non-high-contrast themes.

Also, with growing divergence across platforms it might be a good idea to move the some of this stuff out of the shared file. I'm not a fan of ifdefs in the shared file when we already have specific theme files.

	Richard Marti (:Paenglab) Assignee
	Comment 21 • 10 years ago

Moved the styles to the platform files.

	Tim Nguyen :ntim
	Comment 22 • 10 years ago

Comment on attachment 8532216 [details] [diff] [review]
HC-radio-check.patch

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

::: toolkit/themes/linux/global/in-content/common.css
@@ +51,5 @@
> +}
> +
> +xul|*.checkbox-check[checked] {
> +  list-style-image: url("chrome://global/skin/in-content/check.svg#check-native");
> +  background-color: -moz-dialog;

Please use -moz-field, otherwise the checkboxes will look gray (and disabled) on Linux default theme and Windows third-party themes.

Also, I don't see why the background-color is only applied for the checked state.

@@ +66,5 @@
> +}
> +
> +xul|*.radio-check[selected] {
> +  list-style-image: url("chrome://global/skin/in-content/radio.svg#radio-native");
> +  background: -moz-dialog;

Same here.

Also, you used background-color earlier for checkbox-check, and now you're using background.

::: toolkit/themes/shared/in-content/check.svg
@@ +8,5 @@
> +    use {
> +      fill: #2292d0;
> +    }
> +    use[id$="-native"] {
> +      fill: -moz-dialogText;

Please use -moz-fieldText instead of -moz-dialogText.

::: toolkit/themes/shared/in-content/radio.svg
@@ +8,5 @@
> +    use {
> +      fill: #2292d0;
> +    }
> +    use[id$="-native"] {
> +      fill: -moz-dialogText;

Same comment about the color.

::: toolkit/themes/windows/global/in-content/common.css
@@ +32,5 @@
> +    list-style-image: url("chrome://global/skin/in-content/check.svg#check-native");
> +    background-color: -moz-dialog;
> +  }
> +}
> +

Same comments apply here.

@@ +49,5 @@
> +    list-style-image: url("chrome://global/skin/in-content/radio.svg#radio-native");
> +    background-color: -moz-dialog;
> +  }
> +}
> +

Same here.

	Richard Marti (:Paenglab) Assignee
	Comment 23 • 10 years ago

(In reply to Tim Nguyen [:ntim] from comment #22)
> Comment on attachment 8532216 [details] [diff] [review]
> HC-radio-check.patch
> 
> Review of attachment 8532216 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/themes/linux/global/in-content/common.css
> @@ +51,5 @@
> > +}
> > +
> > +xul|*.checkbox-check[checked] {
> > +  list-style-image: url("chrome://global/skin/in-content/check.svg#check-native");
> > +  background-color: -moz-dialog;
> 
> Please use -moz-field, otherwise the checkboxes will look gray (and
> disabled) on Linux default theme and Windows third-party themes.

I checked on my Linux Mint and -moz-dialogText is a dark grey and this doesn't look disabled. I'll let it land like it is now. If it doesn't work on other themes this can be changed in a new bug.

> Also, I don't see why the background-color is only applied for the checked
> state.

In non-checked state the original background-color is used to better match the original page background-color.

> @@ +66,5 @@
> > +}
> > +
> > +xul|*.radio-check[selected] {
> > +  list-style-image: url("chrome://global/skin/in-content/radio.svg#radio-native");
> > +  background: -moz-dialog;
> 
> Same here.
> 
> Also, you used background-color earlier for checkbox-check, and now you're
> using background.

Fixed, use background-color now.

	Richard Marti (:Paenglab) Assignee
	Comment 24 • 10 years ago

Try build: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0d624aae7d13

	Carsten Book [:Tomcat]
	Comment 25 • 10 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 26 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/56ba684600e1

	Camelia Badau [:cbadau], Desktop QA
	Comment 27 • 10 years ago

Verified fixed on Windows 7 64bit, Windows XP 32bit and Windows 8 32bit using latest Nightly 37.0a1 (buildID: 20141214030205).