386459 - Unchecking SWM needs to set the pref to 2, not 0, and 2 should not have "mixed state"

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

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

Right now, when someone unchecks the SWM pref, we set the value of the pref to 0 (OPEN_DEFAULTWINDOW), which is just "do whatever the default behavior is".  Bug 368839 sets the default to 2 (OPEN_NEWWINDOW) explicitly rather than implicitly.

This creates two problems:

1) Unchecking SWM (or clearing a mixed state from a user-set "reuse") sets to the 'random' 0 value
2) Set to 2, the checkbox displays in a mixed state instead of unchecked.

s/OPEN_DEFAULTWINDOW/OPEN_NEWWINDOW/ in that code fixes 1 and *part* of 2, but with that change and the patch from bug 368839, the checkbox (now defaulting to 2 on a fresh profile) still shows up in a mixed state on launch.

The rest of the fix is probably exceedingly simple, but it's beyond my look-and-copy skills ;)

Comment 1

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

Attachment: fix

Apparently it was only beyond my ability to read up a few lines in the code. :p  This fixes everything.

Comment 2

[Chris Lawson (gone)]

Comment on attachment 270448 [details] [diff] [review]
fix

second-r=me. This looks good.

Comment 3

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

Sean or Wevah, would one of you have a chance to look at this little patch since Ian's sans-internet?  You'll also need the patch from bug 368839 for testing.

Comment 4

[Sean Murphy]

Comment on attachment 270448 [details] [diff] [review]
fix

r=me.

The second line of this patch triggers a compiler warning: "enumeral mismatch in conditional expression".  This is not caused by Smokey's modification though, and was around before the change.  Apparently it's a compiler issue and nothing we're doing wrong: <http://www.cocoabuilder.com/archive/message/xcode/2006/5/12/6332>

Comment 5

[Stuart Morgan]

Why is it that 2 (force new window) is a better default than 0 (do the default thing)?

Comment 6

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

(In reply to comment #5)
> Why is it that 2 (force new window) is a better default than 0 (do the default
> thing)?

Force new window is the actual default behavior that will occur when 0 is set (at least in our case). 0 is never defined in any of the user-facing docs (I had to dig around mxr to even find it existed as a "real" value), so it makes more sense to actually set our default to what we want (and to a value that anyone interested in figuring out can find out about), IMO.

Comment 7

[Stuart Morgan]

Landed on trunk and branch.