Now that everything in the Privacy pane is all Cocoa-y, there's a lot of opportunity to share code among the three very similar sheets.
Created attachment 295876 [details] [diff] [review] sharing and cleanup This shares more of the initialization, filter, and sort logic. The actual filter and sort implementations wouldn't have really benefited from merging, so I left them alone. Since I was already moving a fair amount of stuff around, it seemed like a good opportunity to do a much-need organization/grouping of methods, so I did that as well--and some style cleanup for good measure, since by then I was touching pretty much everything anyway.
Attachment #295876 - Flags: superreview?(mark)
Created attachment 295877 [details] new nib Corresponding nib, since I needed to change the filter field actions. I'm not addicted to revving this nib. I can quit anytime I want!
Comment on attachment 295876 [details] [diff] [review] sharing and cleanup Cleanup diffs are so gnarly, but this looks good. >+- (IBAction)launchKeychainAccess:(id)sender >+ FSRef fsRef; >+ CFURLRef urlRef; >+ OSErr err = ::LSGetApplicationForInfo('APPL', 'kcmr', NULL, kLSRolesAll, &fsRef, &urlRef); We don't need fsRef, you can pass NULL there. Maybe wrap this line while you're changing it. >+ CFRelease(fileSystemURL); Should probably follow the ::pattern used for other calls in this function. This function leaks, but it's not obvious. urlRef is actually owned by the callee and needs to be released. >+ CookieDateFormatter* cookieDateFormatter = >+ [[CookieDateFormatter alloc] initWithDateFormat:@"%b %d, %Y" >+ allowNaturalLanguage:NO]; One of us is going to step on the other of us (411516). I'll let you go first, mine's easier to merge in after your change than the other way around. >+ // This is an artifact of something weird in the nib; if the table is >+ // rebuilt (or the nib is otherwise fixed) this can be removed). Specifically what? >++ (int)indexForPolicy:(int)policy policyForIndex uses switch, why doesn't this?
Attachment #295876 - Flags: superreview?(mark) → superreview+
Landed on trunk and MOZILLA_1_8_BRANCH, with all the changes suggested (except for the switch, which the compiler won't let me do). I also added a note that the weird table issue may have been 10.2-only, and that we should re-test sometime.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Camino1.6
You need to log in before you can comment on or make changes to this bug.