79603 - Disable color controls when 'Use system color' is checked

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect, P4)

Description

[Ben Bucksch (:BenB)]

Reproduce:
1. Prefs|Appearance|Colors
2. Disable "System colors"
3. Select white as background color.
4. Activate 'Use my colors, ignoring the websites' ones'
5. Visit any page

Expected result:
White background

Actaul result:
Grey background

Additional Comments:
regression

Comment 1

[Travis Ogdon]

Unchecking "Use System Colors" on Linux makes the background color work again
for me. 

Comment 2

[karnaze (gone)]

Reassigning to pierre and moving to m0.9.3.

Comment 3

[Pierre Saslawsky]

I can't see it on Mac and Window.  Does it still happen on Linux?

Comment 4

[Ben Bucksch (:BenB)]

> Unchecking "Use System Colors" on Linux makes the background color work again
> for me.

That's what I see. It is still a bug. I would expect either:
- System colors checked disables the Text and Background Color widgets above it
(they should better be moved down, then)
- System colors sets all colors but background and text. (Assuming there are
other system colors than background and text.)
I prefer the latter.

Comment 5

[Pierre Saslawsky]

To reproduce the bug:
- Go to http://www.yahoo.com/
- Open the "Appearance | Color" preferences
- Set the text color to something like green
- Set the background color to something like yellow
- Select "Use my chosen colors, ignoring the colors specified"
- Uncheck "Use system colors"
- Click OK 
==> The page is indeed green on yellow
- Go back to the preferences
- Check "Use system colors" and click OK
==> The colors are ignored, the text is black on grey

This is a problem of UI feedback.  When "Use system colors" is checked, the two 
controls for Text color and Background color should be disabled.

Changed Platform to All, and summary to "Disable color controls when 'Use system 
color' is checked". Reassigned to Preferences.

Comment 6

[Pierre Saslawsky]

Really reassigning to Preferences...

Comment 7

[Samir Gehani]

Hewitt,
Could we make <colorpicker/> support a 'disabled' attribute (ignores events and
visual change to be disabled)?  

Comment 8

[Samir Gehani]

Attachment: Half-baked patch that requires <colorpicker/> to support the 'disabled' attribute.

Comment 9

[Samir Gehani]

Moving to mozilla0.9.8.

Comment 10

[Samir Gehani]

Moving to milestone after mozilla0.9.9 (mozilla1.0 for now).

Comment 11

[Samir Gehani]

*** This bug has been marked as a duplicate of 84212 ***

Comment 12

[Ben Bucksch (:BenB)]

Samir Gehani, the other bug is about the color picker *in general*, this one
about a certain instance.
Will this bug magically be fixed, if the other bug is fixed? Are you sure? If
not, please reopen.

Comment 13

[sairuh (rarely reading bugmail)]

marking verified as a duplicate.

if you decide to reopen this bug, please clarify why.

search string for bugspam removal: SalviaGuaranitica

Comment 14

[Ben Bucksch (:BenB)]

No answer. REOPEN. Reasons, see comment 12.

Comment 15

[Thomas]

I checked this with the first page I had open www.jaggle.nl
If I do the steps in the description I get the expected result: a white background.

Works for me.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b) Gecko/20050217

Comment 16

[Ben Bucksch (:BenB)]

(In reply to comment #15)
> I checked this with the first page I had open www.jaggle.nl

You misunderstood this bug. It's about the UI only, not page rendering.

Comment 17

[Tony Mechelynck [:tonymec]]

No comments in three years.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008033101 SeaMonkey/2.0a1pre

When "Use system colors" is checked, the color picker behaves weirdly: some of its squares become transparent (and show the underlying prefpane), apparently at random, as the mouse is moved over it. If you can catch a "visible" square and click on it, the corresponding color is set in the UI but not in the pages (not even if "use my chosen colors" is selected), which is logical I suppose, but then why let a color be selected?

I suppose it would be better to have the colors whose use is disabled by "Use system colours" be greyed out when the checkbox is selected.

Changing "Target" to "---" since "Mozilla 1.0" is long past. KaiRo, Standard8, Mnyromyr, if you think a different target is more appropriate, go ahead and change it of course. Don't know what to do about the Priority.

Resetting assignee & QAC in consideration of comment #13 and of the fact that the prefpane in question has recently been migrated to a different backend.

Comment 18

[Robert Kaiser]

IanN ported this panel to the new prefwindow in bug 411215, CCing him as he knows the code and it might be easy for him to add that disabling.

Comment 19

[Ian Neal]

Attachment: Disable and lock checking patch v1.0

This patch:
* Disables the color pickers when system colors box is checked as long as picker pref is not locked.
* Re-enables the color pickers when system colors box is unchecked as long as picker pref is not locked.

Comment 20

[Ian Neal]

Attachment: Disable and lock checking patch v1.0a

Correct patch that doesn't try adding a second pref-appearance.js entry to jar.mn

Comment 21

[[e-mail]]

Comment on attachment 312934 [details] [diff] [review]
Disable and lock checking patch v1.0a

Unfortunately disabling colourpickers doesn't work correctly - the binding simply makes a bold claim that it's a XUL control, when it should in fact extend the base control binding which additionally provides the code that handles the disabled and tabindex properties needed for correct disabling...

>+                    oncommand="ToggleCustomColorPickers(this.checked);"/>
This needs to run from an onchange event on the preference element.

>+function Startup()
>+{
>+  ToggleCustomColorPickers(document.getElementById("browserUseSystemColors").checked);
>+}
This needs to read the value from the preference element.

>+  if (aDisable)
>+    aElement.setAttribute("disabled", "true");
>+  else
>+    aElement.removeAttribute("disabled");
... and you'd therefore want to use the property instead of the attribute.

Comment 22

[Karsten Düsterloh]

> When "Use system colors" is checked, the color picker behaves weirdly: some of
> its squares become transparent (and show the underlying prefpane), apparently
> at random, as the mouse is moved over it. If you can catch a "visible" square
> and click on it, the corresponding color is set in the UI but not in the pages
> (not even if "use my chosen colors" is selected), which is logical I suppose,
> but then why let a color be selected?

This does not seem to belong to this bug here. I saw that, too, with other colour pickers when porting the tags panel, but this seems is WFM in my today's custom build [Mozilla/5.0 (X11; U; Linux x86_64; rv:1.9pre) Gecko/2008040222 Mnenhy/0.7.5.20005 SeaMonkey/2.0a1pre].

Comment 23

[Tony Mechelynck [:tonymec]]

(In reply to comment #22)
> > When "Use system colors" is checked, the color picker behaves weirdly: some of
> > its squares become transparent (and show the underlying prefpane), apparently
> > at random, as the mouse is moved over it. If you can catch a "visible" square
> > and click on it, the corresponding color is set in the UI but not in the pages
> > (not even if "use my chosen colors" is selected), which is logical I suppose,
> > but then why let a color be selected?
> 
> This does not seem to belong to this bug here. I saw that, too, with other
> colour pickers when porting the tags panel, but this seems is WFM in my today's
> custom build [Mozilla/5.0 (X11; U; Linux x86_64; rv:1.9pre) Gecko/2008040222
> Mnenhy/0.7.5.20005 SeaMonkey/2.0a1pre].
> 

If the colour picker was disabled in this case (as per this bug), it couldn't behave in any fashion, weird or otherwise.

Comment 24

[[e-mail]]

(In reply to comment #21)
> Unfortunately disabling colourpickers doesn't work correctly
See bug 425564.

Comment 25

[Ian Neal]

Attachment: Onchange patch v1.1

Changes since v1.0a - addressed review comments:
* use onchange on preference.
* use preference value instead of checkbox setting.
* set disabled property on elements rather than attribute.

Comment 26

[Karsten Düsterloh]

Comment on attachment 313728 [details] [diff] [review]
Onchange patch v1.1

Oops, playing around with some style rules for disabled colorpickers (using opacity) I noticed that you forgot to disable their labels when disabling the colorpickers. :-/

Comment 27

[Ian Neal]

Attachment: Label disabling patch v1.1a

Changes since v1.1:
* Labels now have the same disabled status as their associated colorpicker.

Comment 28

[Karsten Düsterloh]

Comment on attachment 313971 [details] [diff] [review]
Label disabling patch v1.1a

Yeah, that's much better.
You're gonna file a bug on the missing colourpicker[disabled] rules if there's none yet?

Comment 29

[Ian Neal]

Comment on attachment 313971 [details] [diff] [review]
Label disabling patch v1.1a

Checking in
jar.mn;
new revision: 1.45; previous revision: 1.44
pref/pref-colors.xul;
new revision: 1.60; previous revision: 1.59
pref/pref-colors.js;
/cvsroot/mozilla/suite/common/pref/pref-colors.js,v  <--  pref-colors.js
initial revision: 1.1
done