1118032 - In Bug 639134 the word "Automatic" does not convey any information on what the choice actually does.

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Philip Chee]

> https://hg.mozilla.org/mozilla-central/rev/460d573b8822#l3.17
> +<!ENTITY  allowPagesToUseColors.automatic.label "Automatic">
> +<!ENTITY  allowPagesToUseColors.always.label    "Always">
> +<!ENTITY  allowPagesToUseColors.never.label     "Never">
> 
> It's never explained anywhere what "Automatic" does. I expect end users
> to start hitting support.mozilla.org on this when this goes to release.
> This can be mitigated by replacing "Automatic" with something clearer.

I suggest slightly rearranged:
"Yes"
"Yes, except in high contrast themes" and
"No"

Comment 1

[:Gijs (he/him)]

(In reply to Philip Chee from comment #0)
> > https://hg.mozilla.org/mozilla-central/rev/460d573b8822#l3.17
> > +<!ENTITY  allowPagesToUseColors.automatic.label "Automatic">
> > +<!ENTITY  allowPagesToUseColors.always.label    "Always">
> > +<!ENTITY  allowPagesToUseColors.never.label     "Never">
> > 
> > It's never explained anywhere what "Automatic" does. I expect end users
> > to start hitting support.mozilla.org on this when this goes to release.
> > This can be mitigated by replacing "Automatic" with something clearer.
> 
> I suggest slightly rearranged:
> "Yes"
> "Yes, except in high contrast themes" and
> "No"

Thanks for filing. I think "Yes, except in high contrast themes" is still kind of long.

Maybe we could invert the text here and use:

"Override the colors specified by the page with my selections above:"

"Always"
"Only on High Contrast themes"
"Never"

Would that work as far as you are concerned, Jared & Philip?

Comment 2

[:Gijs (he/him)]

(In reply to Gijs Kruitbosch from comment #1)
> "Only on High Contrast themes"

s/on/in/, maybe? Not sure what the best way to say that is.

Comment 3

[Philip Chee]

(In reply to Gijs Kruitbosch from comment #2)
> (In reply to Gijs Kruitbosch from comment #1)
> > "Only on High Contrast themes"
> s/on/in/, maybe? Not sure what the best way to say that is.
I'd say Only _with_

Comment 4

[:Gijs (he/him)]

Attachment: adjust UI describing color behaviour,

I needed to add flex=1 because otherwise the label of the high contrast option got ellipsis-ed...

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8545211 [details] [diff] [review]
adjust UI describing color behaviour,

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

::: browser/components/preferences/colors.xul
@@ +83,5 @@
>      <vbox align="start">
> +      <label accesskey="&overridePageColors.accesskey;"
> +             control="useDocumentColors">&overridePageColors.label;</label>
> +      <menulist id="useDocumentColors" preference="browser.display.document_color_use"
> +             flex="1">

I don't see why flex=1 was needed here, and having it here makes the menuitem get truncated with an ellipsis if the selected value is Always and the menupopup is opened.

Removing the flex=1 has the menupopup showing the full size necessary to fit the "Only with High Contrast themes" in all cases.

::: browser/locales/en-US/chrome/browser/preferences/colors.dtd
@@ +11,4 @@
>  
> +<!ENTITY  overridePageColors.never.label  "Never">
> +<!ENTITY  overridePageColors.auto.label   "Only with High Contrast themes">
> +<!ENTITY  overridePageColors.always.label "Always">

Please put these in ordering that they appear in the dialog (always, only, never). Conveniently, that is also alphabetical ordering of the entity names too :)

Comment 6

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> Comment on attachment 8545211 [details] [diff] [review]
> adjust UI describing color behaviour,
> 
> Review of attachment 8545211 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/components/preferences/colors.xul
> @@ +83,5 @@
> >      <vbox align="start">
> > +      <label accesskey="&overridePageColors.accesskey;"
> > +             control="useDocumentColors">&overridePageColors.label;</label>
> > +      <menulist id="useDocumentColors" preference="browser.display.document_color_use"
> > +             flex="1">
> 
> I don't see why flex=1 was needed here, and having it here makes the
> menuitem get truncated with an ellipsis if the selected value is Always and
> the menupopup is opened.
> 
> Removing the flex=1 has the menupopup showing the full size necessary to fit
> the "Only with High Contrast themes" in all cases.

Errr, that's odd, because I saw exactly the opposite behaviour.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gijs Kruitbosch from comment #6)
> > Removing the flex=1 has the menupopup showing the full size necessary to fit
> > the "Only with High Contrast themes" in all cases.
> 
> Errr, that's odd, because I saw exactly the opposite behaviour.

Yeah, which caused me to double and triple check what I was seeing. Can you try it again?

Comment 8

[:Gijs (he/him)]

Attachment: adjust UI describing color behaviour,

Updated as per IRC discussions so the dropdown stretches the width of the dialog, which means text doesn't get ellipsis'd anywhere, and with the l10n bits sorted.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8546771 [details] [diff] [review]
adjust UI describing color behaviour,

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

LGTM, thanks! I wonder if there is a bug with OSX that is causing the dropdown arrow to not be given enough space so when the text is long enough it causes the ellipsis to appear. The menulist works perfectly fine on Windows without the align=start and no flex=1. The alternative may be to keep the align=start but wrap it in a #ifdef XP_WIN, at least until we can figure out why it doesn't work as intended on other platforms.

Comment 10

[:Gijs (he/him)]

w/ ifdef

remote:   https://hg.mozilla.org/integration/fx-team/rev/28624e12ad87

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28624e12ad87