Created attachment 256530 [details] [diff] [review] patch I noticed two minor problems with the session manager code that we might want to straighten up for the sake of correctness as well as preventing compiler warnings. Neither were affecting functionality or caused any major problem. > SessionManager.mm:133: warning: `BookmarkToolbar' may not respond to `-isVisible' > SessionManager.mm:180: warning: `BookmarkToolbar' may not respond to `-setVisible:' #import "BookmarkToolbar.h". > SessionManager.mm:141: warning: invalid conversion from `const NSString*' to `NSString*' > [... 14 more ...] The constant NSString keys used throughout SessionManager.mm are currently declared like this: const NSString* kBookmarkBarIsVisibleKey = @"BookmarkBarVisible"; Which mandates that the object itself (NSString/NSConstantString) must remain constant, while the pointer to that object (kBookmarkBarIsVisibleKey) is permitted to change and refer to another address. NSStrings/NSContstantStrings are already immutable by definition. Instead, marking the pointer variable itself as constant results in the correct behavior: static NSString* const kBookmarkBarIsVisibleKey = @"BookmarkBarVisible"; This will prevent those conversions from occurring. Also, the prior declaration did not impose any restriction on reassigning a key to another string, whereas this approach will cause the compiler to complain. Adding the static keyword keeps the keys local to this file only.
Attachment #256530 - Flags: review?
Comment on attachment 256530 [details] [diff] [review] patch Whoops, that was a braindead mistake. I thought I ran that code through a warning check, but apparently not. r/sr=smorgan (Although I really need to rework session saving to be more like serialization, so SM isn't so dependent on knowing about all these classes)
Attachment #256530 - Flags: review? → superreview+
Checked in on trunk and MOZILLA_1_8_BRANCH.
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.