(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
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
