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

RESOLVED FIXED

Status

Camino Graveyard
Preferences
--
enhancement
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: froodian (Ian Leue), Assigned: froodian (Ian Leue))

Tracking

({fixed1.8.1})

Trunk
PowerPC
Mac OS X
fixed1.8.1

Details

Attachments

(5 attachments, 6 obsolete attachments)

(Assignee)

Description

11 years ago
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

11 years ago
I'm fairly certain we decided this was a good idea on IRC, so confirming.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

11 years ago
Assignee: nobody → stridey
(Assignee)

Comment 2

11 years ago
Created attachment 228746 [details] [diff] [review]
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.
Attachment #228746 - Flags: review?(hwaara)

Comment 3

11 years ago
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?
(Assignee)

Comment 4

11 years ago
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. :(
Depends on: 277706
(Assignee)

Comment 5

11 years ago
Created attachment 229108 [details] [diff] [review]
Relies on nibs to give initial allowsMixedState value
Attachment #228746 - Attachment is obsolete: true
Attachment #229108 - Flags: review?(hwaara)
Attachment #228746 - Flags: review?(hwaara)
(Assignee)

Comment 6

11 years ago
Created attachment 229109 [details]
New Privacy.nib

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

Comment 7

11 years ago
Created attachment 229110 [details]
New Tabs.nib
(Assignee)

Comment 8

11 years ago
Created attachment 229111 [details]
New WebFeatures.nib
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED

Comment 9

11 years ago
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

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

See last comment.
Attachment #229108 - Flags: review?(hwaara) → review+

Comment 11

11 years ago
(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
(Assignee)

Comment 12

11 years ago
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.
Attachment #229108 - Flags: review?(bugzilla)
(Assignee)

Updated

11 years ago
Attachment #229108 - Flags: review?(bugzilla) → superreview?(mikepinkerton)
+  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.
(Assignee)

Comment 14

11 years ago
Created attachment 230761 [details] [diff] [review]
Moves the work to methods

I'm not quit sure I understood you right.  Is this what you wanted?
Attachment #229108 - Attachment is obsolete: true
Attachment #230761 - Flags: superreview?(mikepinkerton)
Attachment #229108 - Flags: superreview?(mikepinkerton)
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.
Attachment #230761 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 16

11 years ago
Created attachment 231771 [details] [diff] [review]
sr=pink patch
Attachment #230761 - Attachment is obsolete: true
(Assignee)

Updated

11 years ago
Whiteboard: [needs checkin]
(Assignee)

Updated

11 years ago
Blocks: 174070

Comment 17

11 years ago
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

11 years ago
Created attachment 233476 [details] [diff] [review]
Diff problems.
(Assignee)

Comment 19

11 years ago
Created attachment 233679 [details]
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
Attachment #229111 - Attachment is obsolete: true
(Assignee)

Comment 20

11 years ago
Created attachment 233680 [details]
New Tabs.nib
Attachment #229110 - Attachment is obsolete: true
(Assignee)

Comment 21

11 years ago
Created attachment 233681 [details]
New Privacy.nib
Attachment #229109 - Attachment is obsolete: true

Comment 22

11 years ago
Checked in on the trunk and MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1
Whiteboard: [needs checkin]
(Assignee)

Updated

11 years ago
No longer blocks: 174070
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.