Closed Bug 1118032 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Preferences, defect)

defect
Not set
Points:
1

Tracking

()

RESOLVED FIXED
Firefox 37
Iteration:
37.3 - 12 Jan

People

(Reporter: philip.chee, Assigned: Gijs)

References

Details

Attachments

(1 file, 1 obsolete file)

> 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"
(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?
Flags: needinfo?(philip.chee)
Flags: needinfo?(jaws)
(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.
(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_
Flags: needinfo?(philip.chee)
Status: NEW → ASSIGNED
Component: Layout → Preferences
Flags: qe-verify-
Flags: in-testsuite-
Flags: firefox-backlog+
OS: Windows 7 → All
Product: Core → Firefox
Hardware: x86_64 → All
Assignee: nobody → gijskruitbosch+bugs
I needed to add flex=1 because otherwise the label of the high contrast option got ellipsis-ed...
Attachment #8545211 - Flags: review?(jaws)
Iteration: 37.1 → 37.3 - 12 Jan
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 :)
Attachment #8545211 - Flags: review?(jaws)
(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.
(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?
Flags: needinfo?(jaws)
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.
Attachment #8546771 - Flags: review?(jaws)
Attachment #8545211 - Attachment is obsolete: true
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.
Attachment #8546771 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/28624e12ad87
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
You need to log in before you can comment on or make changes to this bug.