353221 - "Don't warn again" checkboxes can become polluted

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

[froodian (Ian Leue)]

bug 340412 comment 18:
> >-
> >-  [self setPref:"camino.warn_when_closing" toBoolean:([checkboxWarnWhenClosing state] == NSOnState)];
> 
> Why is this being removed (and if there's a good reason, why is the previous
> one not being removed)?

We manually set some prefs on closing the General prefpane, "just to be sure."  In actuality, this can easily compromise the "Don't warn again" checkboxes.  For instance:

1. Open Preferences.  Make sure "warn me when closing..." is on.  Leave the window open.
2. Open a bunch of tabs, cmd-shift-W, check the "don't warn me again" checkbox, hit cancel.
3. Close the preferences window.

What happens: Closing the prefs window sets the preference back to true.
What should happen: We should just trust that we're doing the right thing, and let the pref stay as it is (as we do in all other prefpanes)

There are easier ways this can come up too, since |mainViewDidLoad| doesn't necessarily get called every time you open the prefs window, but that may clouding the issue with what very well may be a different bug.  For instance:

1. Open Preferences.  Make sure "warn me when closing..." is on.  Close preferences.
2. Open a bunch of tabs, cmd-shift-W, check the "don't warn me again" checkbox, hit cancel.
3. Open Preferences (checkbox is on, since it doesn't call |mainViewDidLoad|, but pref is off internally)
4. Close Preferences (since checkbox is on, we set the pref to on)

Comment 1

[froodian (Ian Leue)]

Attachment: Patch

Gets rid of the manual pref setters at prefpane de-selection.

Comment 2

[Håkan Waara]

Comment on attachment 239090 [details] [diff] [review]
Patch

Makes sense. Checkboxes' and radiobuttons' effect in prefs should be immediate (while the textboxes require closing the prefs window).

Comment 3

[Chris Lawson (gone)]

(In reply to comment #2)
> (From update of attachment 239090 [details] [diff] [review] [edit])
> Makes sense. Checkboxes' and radiobuttons' effect in prefs should be immediate

Yes.

> (while the textboxes require closing the prefs window).

No. Text boxes should take effect as soon as focus leaves the text input. (And in fact, I believe we've largely addressed that issue.)

cl

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

> > (while the textboxes require closing the prefs window).
> 
> No. Text boxes should take effect as soon as focus leaves the text input. (And
> in fact, I believe we've largely addressed that issue.)

History days still doesn't auto-apply, iirc (I wonder if that's because you can't really "leave" text fields unless there's a second text field or you have FKA on?).  But yeah, they should auto-apply.

Comment 5

[Håkan Waara]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 239090 [details] [diff] [review] [edit] [edit])
> > Makes sense. Checkboxes' and radiobuttons' effect in prefs should be immediate
> 
> Yes.
> 
> > (while the textboxes require closing the prefs window).
> 
> No. Text boxes should take effect as soon as focus leaves the text input. (And
> in fact, I believe we've largely addressed that issue.)

If that's the case, then I'm wondering what the lines above froodian's changes do. Unfortunately, the diff doesn't let me see the method name, but I suspect it is when we're closing the window?

Textfields have a didEndEditing delegate method that can be used to apply the pref when we "leave" it somehow, I think.

Comment 6

[Chris Lawson (gone)]

(In reply to comment #5)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 239090 [details] [diff] [review] [edit] [edit] [edit])
> > > Makes sense. Checkboxes' and radiobuttons' effect in prefs should be immediate
> > 
> > Yes.
> > 
> > > (while the textboxes require closing the prefs window).
> > 
> > No. Text boxes should take effect as soon as focus leaves the text input. (And
> > in fact, I believe we've largely addressed that issue.)
> 
> If that's the case, then I'm wondering what the lines above froodian's changes
> do. Unfortunately, the diff doesn't let me see the method name, but I suspect
> it is when we're closing the window?

They ensure that the edit gets committed if focus is still in the field when the prefs are closed (or when a different pref pane is selected).

cl

Comment 7

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 239090 [details] [diff] [review]
Patch

sr=pink

Comment 8

[froodian (Ian Leue)]

Checked in on 1.8branch and trunk.