344036 - Give certain pref checkboxes 'mixed state' look when neither on nor off

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, enhancement)

Description

[froodian (Ian Leue)]

Certain prefs (SWM, for instance) can be set to neither on nor off via about:config.  When a user has done this, it'd be nice to give the checkbox the 'mixed state' look instead of just turning it off.

Comment 1

[Chris Lawson (gone)]

I'm fairly certain we decided this was a good idea on IRC, so confirming.

Comment 2

[froodian (Ian Leue)]

Attachment: Enables mixed state

Enables a mixed state look for all nonboolean prefs that we map to boolean in the UI, except for 

browser.startup.page
browser.tabs.startPage

because setting them to values other than those exposed in the UI didn't seem to do anything.

The code in WebFeatures.mm for |mEnableAnnoyanceBlocker| is kinda ugly - I'm almost certain there's a more elegant way to do that.

Comment 3

[Håkan Waara]

Can't you do the equivalent of setAllowsMixedState: in the nib on those checkboxes where it makes sense? Or is there some special reason to set it on every click in the action method?

Comment 4

[froodian (Ian Leue)]

We'll still need to set it to NO every click (because otherwise the checkboxes can be clicked into the mixed state, which doesn't mean anything), but yeah, we can set the mixed state to YES in the nib, and avoid having to set it to YES in the |mainViewDidLoad| methods.

Marking dependent on a bug currently holding up the webfeatures nib. :(

Comment 5

[froodian (Ian Leue)]

Attachment: Relies on nibs to give initial allowsMixedState value

Comment 6

[froodian (Ian Leue)]

Attachment: New Privacy.nib

This (and the following two nibs) just check the mixed state checkbox for the appropriate checkboxes.

Comment 7

[froodian (Ian Leue)]

Attachment: New Tabs.nib

Comment 8

[froodian (Ian Leue)]

Attachment: New WebFeatures.nib

Comment 9

[Håkan Waara]

One thing I find sort of confusing is that you can only get into the mixed states by editing your about:config. So it's sort of a special case.

But I guess it's ok then, as long as users don't run into this and can't get back into the mixed state.

Comment 10

[Håkan Waara]

Comment on attachment 229108 [details] [diff] [review]
Relies on nibs to give initial allowsMixedState value

See last comment.

Comment 11

[Chris Lawson (gone)]

(In reply to comment #9)
> One thing I find sort of confusing is that you can only get into the mixed
> states by editing your about:config. So it's sort of a special case.

Wasn't that the whole point of this bug in the first place?

cl

Comment 12

[froodian (Ian Leue)]

Comment on attachment 229108 [details] [diff] [review]
Relies on nibs to give initial allowsMixedState value

Yeah, this is sort of a "by the way, you messed with your about:config" bug.  Normal users will never see its products.

Comment 13

[Mike Pinkerton (not reading bugmail)]

+  if([self getBooleanPref:"dom.disable_window_status_change" withSuccess:&gotPref] &&
+     [self getBooleanPref:"dom.disable_window_move_resize" withSuccess:&gotPref] &&
+     [self getBooleanPref:"dom.disable_window_flip" withSuccess:&gotPref])

rather than duplicating this code and inverting it, why not break it out into separate routines? Will make the init code easier to parse.

+  if ([preventAnimation isEqualToString:@"once"])
+    [mPreventAnimation setState:NSOnState];
+  else if ([preventAnimation isEqualToString:@"normal"])
+    [mPreventAnimation setState:NSOffState];
+  else
+    [mPreventAnimation setState:NSMixedState];

move this into a function that returns the button state? happens two or three times.

Comment 14

[froodian (Ian Leue)]

Attachment: Moves the work to methods

I'm not quit sure I understood you right.  Is this what you wanted?

Comment 15

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 230761 [details] [diff] [review]
Moves the work to methods

+  if([self getBooleanPref:"dom.disable_window_status_change" withSuccess:NULL] &&
+     [self getBooleanPref:"dom.disable_window_move_resize" withSuccess:NULL] &&
+     [self getBooleanPref:"dom.disable_window_flip" withSuccess:NULL])
+    return kAnnoyancePrefAll;
+  if(![self getBooleanPref:"dom.disable_window_status_change" withSuccess:NULL] &&
+     ![self getBooleanPref:"dom.disable_window_move_resize" withSuccess:NULL] &&
+     ![self getBooleanPref:"dom.disable_window_flip" withSuccess:NULL])
+    return kAnnoyancePrefNone;

can you rework this slightly such that you only fetch each pref once rather than mutliple times?

sr=pink with that.

Comment 16

[froodian (Ian Leue)]

Attachment: sr=pink patch

Comment 17

[Nick Kreeger]

All three of the attached nib's are old and have conflicts on the trunk! The patch has been checked in so we need these new nibs asap.

(I don't know what changes have been made to the nibs or I would do it myself)

cvs commit: file `camino/PreferencePanes/Privacy/English.lproj/Privacy.nib/info.nib' had a conflict and has not been modified
cvs commit: file `camino/PreferencePanes/Tabs/English.lproj/Tabs.nib/keyedobjects.nib' had a conflict and has not been modified
cvs commit: file `camino/PreferencePanes/WebFeatures/English.lproj/WebFeatures.nib/info.nib' had a conflict and has not been modified
cvs commit: file `camino/PreferencePanes/WebFeatures/English.lproj/WebFeatures.nib/keyedobjects.nib' had a conflict and has not been modified

Comment 18

[Nick Kreeger]

Attachment: Diff problems.

Comment 19

[froodian (Ian Leue)]

Attachment: New Webfeatures.nib

These should be better.  In case they're not, all I did was make the following checked boxes have mixed state:

- Prevent sites from changing, moving or resizing windows
- Play animated images only once
- Ask before accepting each cookie
- SWM

Comment 20

[froodian (Ian Leue)]

Attachment: New Tabs.nib

Comment 21

[froodian (Ian Leue)]

Attachment: New Privacy.nib

Comment 22

[Mark Mentovai]

Checked in on the trunk and MOZILLA_1_8_BRANCH.