31376 - textfields for deselected checkboxes shouldn't allow editing

Status: RESOLVED WORKSFORME

(SeaMonkey :: Preferences, defect, P5)

Description

[sairuh (rarely reading bugmail)]

if this is a dup, let me know --i noticed this in the Preferences dialog on all
three platforms, using opt comm bits from 2000.03.09.xx.

this also occurs in Communicator 4.7x, and imho it's kinda confusing UI design
--if a checkbox or radio button with an associated textfield is not selected,
the user shouldn't be able to edit the contents of that textfield.

to observe/repro:
1. open Prefs.
2. go to Mail & News > Disk Space.
3. go to a textfield that's associated with a checkbox or radio button that is
*not* selected --especially if it already has contents, eg,
"Do not store messages locally that are larger than 50 KB" or
"Automatically compact folders when it will save over 100 KB" or
"Keep the newest 30 messages"
4. remove/change the value in the textfield. eg, i replaced the three values in
step (3) with 42.
5. click OK to dismiss Prefs.
6. reopen Prefs, and read the prefs.js file...

result: you'll be able to change those unselected textfields! in fact, they're
even written to the prefs.js if you exited Prefs w/OK:

user_pref("mail.max_size", 42);
user_pref("mail.purge_threshhold", 42);
user_pref("news.keep.count", 42);

i would think that these values would be neither written to prefs.js *or*
displayed upon returning to Preferences --when they weren't even selected in the
first place.

Comment 1

[sairuh (rarely reading bugmail)]

this also occurs in 4.7x --sure sounds like it would be a long-running bug then.
however, lisa, could you tell me whether/why this would be considered expected
behavior? (if it is, okay, i might invalidate this.)

note that changing the values (in both 4.7x and seamonkey) back to their
defaults (still *without* selecting them), removes them from
preferences.js/prefs.js. odd.

Comment 2

[sairuh (rarely reading bugmail)]

if this is a valid bug, am nominating for beta1.

would send email about this issue, but the mail server is sucking big time at
the moment. :-(

Comment 3

[lchiang]

On some platforms, if you have an edit field associated with a radio button or a 
checkbox that is not selected/checked, when you edit the field, the radio button 
or checkbox should become automatically enabled.

I don't think in 4.x we had this working properly.

Comment 4

[leger]

Putting on PDT+ radar for beta1. 

Comment 5

[don]

Ben sez he'll take care of this ...

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

clearing PDT+ as this is a 4.x issue as well and I think it requires some more 
thought.

yes the text fields should be disabled, but how the information that is in them 
is disabled is saved or not saved as the case may be is the important thing. We 
have two options:

1) destroy the data in prefs for prefs set by disabled fields
   cons: data loss. e.g. on windows, set up networking to use a specific IP, DNS 
server etc. save, then change to auto suppplied IP, and disable DNS. Save. 
Return and enable DNS and go to specify IP again - all previously entered data 
is lost. 

2) Maintain data last entered in field 
   better, but what if there are conflicts between these prefs remaining set and 
the disabling of the meta (owning) pref?

Comment 7

[leger]

Putting on PDT- radar for beta1.

Comment 8

[don]

If this is also 4.x behavior then I don't think we need to bother with this for
beta 2 either.  Why is the severity of this bug "critical"?  Can this be reduced
to "normal" or even "enhancement"?

Comment 9

[german]

I think this bug falls under 'good UI design pratice/polish', but is not a
'critical' thing for beta 2. user error is unlikely as a result of this
unpolished behavior.
with regards to Ben's ponderings for the longer run I think it is acceptable in
disabled state to keep the entries around only long enough while the prefs
dialog is open, and not save them, if this would cut down time to implement.
This behavior - while not as convenient as saving disabled entries - is at least
predictable.

Comment 10

[sairuh (rarely reading bugmail)]

okay, i'll bump this down to normal. :-) but i agree w/german --this problem is
poor UI behavior...

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

not a priority, pushing out as far as possible.

Comment 12

[sairuh (rarely reading bugmail)]

pardon the spam: beta1 is long gone...removing this keyword. will soon replace
w/nsbeta2...

Comment 13

[don]

Move to M21 target milestone.

Comment 14

[sairuh (rarely reading bugmail)]

so, this is no longer a problem with textfields associated with radiobuttons 
(eg, the Proxies panel). but this is still a problem with textfields associated 
with checkboxes (eg, the Composer panel).

ben/jrgm, possible to fix [soon]?

beppe, who handles the composer prefs (thought it's brade, but she's on 
sabbatical now)...?

Comment 15

[rubydoo123]

is this only in the editor pref section or is it across all prefs? If it is 
across all prefs, then it belongs to Ben

Comment 16

[John Morrison]

Enabling/disabling certain widgets depending on the state of some "master" 
widget needs to done on a case-by-case basis. There is no simple way to 
develop a general purpose facility for managing this dependent behaviour
(e.g., the dependent set of widgets could be some arbitrary collection, and 
the logical relation is not apparent from the structure of the XUL).

There is a good example of how to implement JS to control enabling/disabling
of dependent widgets in chrome://communicator/content/pref/pref-proxies.

Perhaps, it would be best to just close this bug out, and sairuh find the 
places in the prefs UI where something similar needs to done, on a separate
bug-per-panel basis.

(As far as what to do with pref values written into disabled widgets, that 
would best be handled by the back-end code that acts on these pref values.
Pref values that only have meaning in a particular mode, should never be used
unless the pref that sets that mode is also turned on (although I suspect that
most back-end code already does the right thing)).

Comment 17

[johng]

nav triage team:
nsbeta3-, M Future

Comment 18

[Blake Ross]

Eddy, can you take this?  Talk to Ben for more info.

Comment 19

[eddyk]

There are two issues in this bug:

1) disabling elements based on a master element

This has been mostly handled on an individual basis (the Mail/News disk space 
panel isn't even there anymore) and the same issues in other panels should be 
filed as new bugs.

2) Writing prefs to the prefs file when the xul element was disabled.

In my opinion, this is a front end issue, much like 1).  The nsPrefWindow code 
should not make an assumption that a disabled xul element should not be written 
out.

I thought that this might interfere wih the eClient pref lockdown feature, but 
on second thought, as long as the front end display is not affected, we should 
be ok.  But if we're still showing the value in an element, would the user 
assume that it is saved and can be seen later?  Does this fix the problem of 
conflicting preferences?

The front end should clear the contents of an element if necessary.  While it 
seems fairly simple to modify nsPrefWindow to not write out prefs for disabled 
elements, I'm not sure if this is the right thing to do.  Please convice me if 
otherwise.

Comment 20

[Peter Trudelle]

->ben

Comment 21

[rubydoo123]

removing myself from the cc list

Comment 22

[Serge Gautherie (:sgautherie)]

(Filter "spam" on 'prefs-nobody-20080612'.)

Comment 23

[Philip Chee]

I've checked all the way back to SeaMonkey 1.0 and as far as I can tell all the specific items mentioned here have been fixed. And any not specifically mentioned should have been fixed in the toolkit preferences migration. If there are any remaining items, please file a new bug.