Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Bug 639134 changed boolean "browser.display.use_document_colors" to tristate "browser.display.document_color_use"

237 views
Skip to first unread message

Philip Chee

unread,
Dec 6, 2014, 2:14:06 AM12/6/14
to
I think the changes for this bug are sub-optimal.

First:

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 more
informative, understandable, clearer, and less beware of leopard.

My second problem is that the new preference is a tri-state (int)
preference. The use of multi-state preferences has not always been a
happy story.

See For example:
> Bug 1042135 - Change three-state DNT back to two state and update text.
> Bug 530209 - Improve search suggestions ui for locationbar prefs

I suggest that unless there is a clear win for a multi-state preference
in this particular instance, we should stick to something simpler. For
example two boolean preferences.
The original preference:
browser.display.use_document_colors
plus a new one:
browser.display.use_document_colors.even_for_high_contrast_os_themes.do_what_I_say_dammit

Also the use case for Bug 639134 was for Windows. Does this setting have
the proper effect on Linux and OSX? Is this even needed on non-windows?

Phil

--
Philip Chee <phi...@aleytys.pc.my>, <phili...@gmail.com>
http://flashblock.mozdev.org/ http://xsidebar.mozdev.org
Guard us from the she-wolf and the wolf, and guard us from the thief,
oh Night, and so be good for us to pass.

Gijs Kruitbosch

unread,
Dec 6, 2014, 10:42:45 AM12/6/14
to Philip Chee, Jared Wein, L. David Baron, David Bolter
(replying to m.d.platform, but also pulling some folks into the CC list)

Hi Philip,

Thanks for raising this.

On 05/12/2014 23:13, Philip Chee wrote:
> I think the changes for this bug are sub-optimal.
>
> First:
>
> 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 more
> informative, understandable, clearer, and less beware of leopard.

I'm happy to agree "Automatic" isn't ideal, but nobody came up with a
better suggestion. I don't think making it a full sentence will be any
better, and it's hard to succinctly summarize this otherwise. Do you
have a suggestion?

(fwiw, *I* have no idea what "beware of leopard" means)

As for SUMO, I expect end users are already hitting support.m.o when
they toggle the current checkbox and suddenly find pages not working
particularly well, just like I know for sure that they currently hit
sumo/bugzilla/irc when using high contrast themes and preferring either
the one or the other behaviour, and being confused about why Firefox
doesn't do what they want.

Short of removing the pref from the UI altogether, I'm not sure how we
can fix that. In the meantime, I'm happy to help the SUMO team document
the change - but I don't think that the prefs UI here is the cause for
confusion.

> My second problem is that the new preference is a tri-state (int)
> preference. The use of multi-state preferences has not always been a
> happy story.
>
> See For example:
>> Bug 1042135 - Change three-state DNT back to two state and update text.
>> Bug 530209 - Improve search suggestions ui for locationbar prefs
>
> I suggest that unless there is a clear win for a multi-state preference
> in this particular instance, we should stick to something simpler.

This is putting the cart before the horse. In both of your examples, we
changed the prefs used *because of explicit behavioral requirements* (ie
getting rid of one of the options for bug 1042135, and adding more
options as checkboxes instead of combinatorial explosions in 530209),
not because int prefs used for options are inherently evil.


> For
> example two boolean preferences.
> The original preference:
> browser.display.use_document_colors
> plus a new one:
> browser.display.use_document_colors.even_for_high_contrast_os_themes.do_what_I_say_dammit

This means the second pref overrides the first (ie makes it useless).
That makes for more complicated UI (probably want to hide/disable the
checkbox, which then makes instructions on how to toggle that pref in
the UI more complicated, etc.). Programmatically, you'd also be using 4
possible states (2^2) to encode 3 different options, which seemed
suboptimal to me (ie you could change just the backend and use the
current UI to implicitly set the two bools, but it'd be messy, too).

> Also the use case for Bug 639134 was for Windows. Does this setting have
> the proper effect on Linux and OSX?

We don't currently honour high contrast themes on either of those OSes.
OS X doesn't really have a high contrast theme like Windows/GTK does,
that I'm aware of (they have a high contrast mode that messes with the
display at the OS level, AIUI). Linux's API is basically "read the
current GTK theme name and if it says 'high contrast', it probably is",
which is presumably part of the reason support was never implemented.
That is actually on my list of things to get to, in which case this
option should Just Work.

So for Linux/OSX the option "Automatic" devolves to "Always" while we
don't recognize high contrast themes. I don't think that that's bad
enough to justify doing a bunch of work to default to the "Always"
setting (and maybe hide the "Automatic" option, unless people have
manually set it to that by about:config or by copying a profile from
another machine, etc.) on those OSes, especially if we do still have
implementing high contrast support for Linux on the agenda, but we could
do that if people are convinced that's worth it.

The Linux implementation is bug 239914 / bug 1064164.

~ Gijs

Philip Chee

unread,
Dec 7, 2014, 12:11:19 AM12/7/14
to
On 06/12/2014 23:42, Gijs Kruitbosch wrote:
> (replying to m.d.platform, but also pulling some folks into the CC list)
>
> Hi Philip,
>
> Thanks for raising this.
>
> On 05/12/2014 23:13, Philip Chee wrote:
>> I think the changes for this bug are sub-optimal.
>>
>> First:
>>
>> 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 more
>> informative, understandable, clearer, and less beware of leopard.
>
> I'm happy to agree "Automatic" isn't ideal, but nobody came up with a
> better suggestion. I don't think making it a full sentence will be any
> better, and it's hard to succinctly summarize this otherwise. Do you
> have a suggestion?

People using high contrast themes in windows would know what high
contrast themes are since they have to select those themes from the
"High Contrast Themes" section.

> (fwiw, *I* have no idea what "beware of leopard" means)

That illustrates my point exactly! As in replacing "Automatic" with
"Beware of leopard" makes no difference to the (lack of) clarity.

Also did you remember to pack a toothbrush and a towel before heading
out to Portland?

>> Also the use case for Bug 639134 was for Windows. Does this setting have
>> the proper effect on Linux and OSX?
>
> We don't currently honour high contrast themes on either of those OSes.
> OS X doesn't really have a high contrast theme like Windows/GTK does,
> that I'm aware of (they have a high contrast mode that messes with the
> display at the OS level, AIUI). Linux's API is basically "read the
> current GTK theme name and if it says 'high contrast', it probably is",
> which is presumably part of the reason support was never implemented.
> That is actually on my list of things to get to, in which case this
> option should Just Work.
>
> So for Linux/OSX the option "Automatic" devolves to "Always" while we
> don't recognize high contrast themes. I don't think that that's bad
> enough to justify doing a bunch of work to default to the "Always"
> setting (and maybe hide the "Automatic" option, unless people have
> manually set it to that by about:config or by copying a profile from
> another machine, etc.) on those OSes, especially if we do still have
> implementing high contrast support for Linux on the agenda, but we could
> do that if people are convinced that's worth it.
>
> The Linux implementation is bug 239914 / bug 1064164.

AFAICS nobody is working on GTK and OSX. In the mean time I propose that
"Automatic" not be exposed to users on those platforms who might wonder
what it does (answer: nothing). File under useless-UI

Gijs Kruitbosch

unread,
Dec 8, 2014, 4:35:25 PM12/8/14
to
On 07/12/2014 05:11, Philip Chee wrote:
> People using high contrast themes in windows would know what high
> contrast themes are since they have to select those themes from the
> "High Contrast Themes" section.

So you're suggesting... ?

>> (fwiw, *I* have no idea what "beware of leopard" means)
>
> That illustrates my point exactly! As in replacing "Automatic" with
> "Beware of leopard" makes no difference to the (lack of) clarity.

I disagree. We use "automatic" to describe, for instance, our default
cache size decisions, rather than saying "based on the available disk
space and potentially some other stuff" (I don't actually know what
other things come into this, but I guess there might be other things?).
The Privacy settings do not indicate after what amount of time we clear
history, and the Certificates settings under Advanced have an option to
"Select one automatically", rather than describing exactly how we make
the decision.

"Beware of leopard", on the other hand, is just nonsense.


> Also did you remember to pack a toothbrush and a towel before heading
> out to Portland?

... I also don't know what you're getting at here.


> AFAICS nobody is working on GTK and OSX. In the mean time I propose that
> "Automatic" not be exposed to users on those platforms who might wonder
> what it does (answer: nothing). File under useless-UI

Seems fair, did you file this? Feel free to assign to me and I can look
at doing this.

~ Gijs

Steve Fink

unread,
Dec 8, 2014, 5:38:55 PM12/8/14
to Gijs Kruitbosch, dev-pl...@lists.mozilla.org
On 12/08/2014 01:35 PM, Gijs Kruitbosch wrote:
> On 07/12/2014 05:11, Philip Chee wrote:
>
>>> (fwiw, *I* have no idea what "beware of leopard" means)
>>
>> That illustrates my point exactly! As in replacing "Automatic" with
>> "Beware of leopard" makes no difference to the (lack of) clarity.
>
> I disagree. We use "automatic" to describe, for instance, our default
> cache size decisions, rather than saying "based on the available disk
> space and potentially some other stuff" (I don't actually know what
> other things come into this, but I guess there might be other
> things?). The Privacy settings do not indicate after what amount of
> time we clear history, and the Certificates settings under Advanced
> have an option to "Select one automatically", rather than describing
> exactly how we make the decision.

"platform-default"?

0 new messages