We should expose a few of the really standard settings (layer borders, repaint counters, debug indicator, etc.) on WKPreferencesPrivate.
Created attachment 234433 [details] patch
Comment on attachment 234433 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review > Source/WebKit2/UIProcess/API/Cocoa/WKPreferencesPrivate.h:42 > +// FIXME: This should be setter=_setTelephoneNumberDetectionEnabled:, getter=_isTelephoneNumberDetectionEnabled, property=telephoneNumberDetectionEnabled Why? Getter names shouldn’t be question phrases. They should fit naturally into conditions like this: if ([object isHot] && [object isLarge] || [object cornersAreRound] && [object hasSharpEdges] || [object nightVisionIsEnabled]) > Source/WebKit2/UIProcess/API/Cocoa/WKPreferencesPrivate.h:48 > +@property (nonatomic, setter=_setCompositingBordersVisible:, getter=_areCompositingBordersVisible) BOOL _compositingBordersVisible; > +@property (nonatomic, setter=_setCompositingRepaintCountersVisible:, getter=_areCompositingRepaintCountersVisible) BOOL _compositingRepaintCountersVisible; > +@property (nonatomic, setter=_setTiledScrollingIndicatorVisible:, getter=_isTiledScrollingIndicatorVisible) BOOL _tiledScrollingIndicatorVisible; Same comments about naming.
(In reply to comment #2) > (From update of attachment 234433 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review > > > Source/WebKit2/UIProcess/API/Cocoa/WKPreferencesPrivate.h:42 > > +// FIXME: This should be setter=_setTelephoneNumberDetectionEnabled:, getter=_isTelephoneNumberDetectionEnabled, property=telephoneNumberDetectionEnabled > > Why? Getter names shouldn’t be question phrases. They should fit naturally into conditions like this: > if ([object isHot] && [object isLarge] || [object cornersAreRound] && [object hasSharpEdges] || [object nightVisionIsEnabled]) > > > Source/WebKit2/UIProcess/API/Cocoa/WKPreferencesPrivate.h:48 > > +@property (nonatomic, setter=_setCompositingBordersVisible:, getter=_areCompositingBordersVisible) BOOL _compositingBordersVisible; > > +@property (nonatomic, setter=_setCompositingRepaintCountersVisible:, getter=_areCompositingRepaintCountersVisible) BOOL _compositingRepaintCountersVisible; > > +@property (nonatomic, setter=_setTiledScrollingIndicatorVisible:, getter=_isTiledScrollingIndicatorVisible) BOOL _tiledScrollingIndicatorVisible; > > Same comments about naming. I guess I got very confused by WKPreferences.h, which has getters like isJavaScriptEnabled and arePlugInsEnabled?
Comment on attachment 234433 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review >> Source/WebKit2/UIProcess/API/Cocoa/WKPreferencesPrivate.h:48 >> +@property (nonatomic, setter=_setTiledScrollingIndicatorVisible:, getter=_isTiledScrollingIndicatorVisible) BOOL _tiledScrollingIndicatorVisible; > > Same comments about naming. Dan, I think the “preferences” context is a bit more difficult. Here’s what we obviously don’t want: if ([preferences areConfiguredForThoseWhoPreferVisibleCompositingBorders]) What exactly is the good alternative? Is there a good example of this to model this class on?
(In reply to comment #4) > (From update of attachment 234433 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review > > >> Source/WebKit2/UIProcess/API/Cocoa/WKPreferencesPrivate.h:48 > >> +@property (nonatomic, setter=_setTiledScrollingIndicatorVisible:, getter=_isTiledScrollingIndicatorVisible) BOOL _tiledScrollingIndicatorVisible; > > > > Same comments about naming. > > Dan, I think the “preferences” context is a bit more difficult. Here’s what we obviously don’t want: > > if ([preferences areConfiguredForThoseWhoPreferVisibleCompositingBorders]) > > What exactly is the good alternative? Approximately if ([preferences compositingBordersVisible]) or if ([preferences compositingBordersEnabled]) or with the verb “are” after “borders” > Is there a good example of this to model this class on? I don’t know of any similar class that consistently avoid the question-phrase style. In the legacy WebPreferences class, for example, there is both a (good) example like -javaScriptCanOpenWindowsAutomatically and a (bad) example like -isJavaScriptEnabled.
Thanks, Dan. Sounds good.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 234433 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review > > Is there a good example of this to model this class on? > > I don’t know of any similar class that consistently avoid the question-phrase style. In the legacy WebPreferences class, for example, there is both a (good) example like -javaScriptCanOpenWindowsAutomatically and a (bad) example like -isJavaScriptEnabled. Sooooo, will we fix the 3 out of 5 actually-API prefs that are poorly-named? Or do they get a pass because they match the legacy WebKit names? Or some other reason?
(In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #4) > > > (From update of attachment 234433 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review > > > Is there a good example of this to model this class on? > > > > I don’t know of any similar class that consistently avoid the question-phrase style. In the legacy WebPreferences class, for example, there is both a (good) example like -javaScriptCanOpenWindowsAutomatically and a (bad) example like -isJavaScriptEnabled. > > Sooooo, will we fix the 3 out of 5 actually-API prefs that are poorly-named? Or do they get a pass because they match the legacy WebKit names? Or some other reason? Yes, I think these should be changed: @property (nonatomic, getter=isJavaScriptEnabled) BOOL javaScriptEnabled; @property (nonatomic, getter=isJavaEnabled) BOOL javaEnabled; @property (nonatomic, getter=arePlugInsEnabled) BOOL plugInsEnabled;
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > (From update of attachment 234433 [details] [details] [details] [details]) > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=234433&action=review > > > > Is there a good example of this to model this class on? > > > > > > I don’t know of any similar class that consistently avoid the question-phrase style. In the legacy WebPreferences class, for example, there is both a (good) example like -javaScriptCanOpenWindowsAutomatically and a (bad) example like -isJavaScriptEnabled. > > > > Sooooo, will we fix the 3 out of 5 actually-API prefs that are poorly-named? Or do they get a pass because they match the legacy WebKit names? Or some other reason? > > Yes, I think these should be changed: > > @property (nonatomic, getter=isJavaScriptEnabled) BOOL javaScriptEnabled; > @property (nonatomic, getter=isJavaEnabled) BOOL javaEnabled; > @property (nonatomic, getter=arePlugInsEnabled) BOOL plugInsEnabled; I filed https://bugs.webkit.org/show_bug.cgi?id=134678 about that. I will post a new version of this patch with the names adjusted in a bit.
Created attachment 234481 [details] patch with new names
We decided on no verbs in https://bugs.webkit.org/show_bug.cgi?id=134678, so I'll post another-other-other patch to match.
Created attachment 234592 [details] patch with no verbs
Comment on attachment 234592 [details] patch with no verbs View in context: https://bugs.webkit.org/attachment.cgi?id=234592&action=review > Tools/MiniBrowser/mac/WK1BrowserWindowController.m:140 > + [menuItem setState:[self layerBordersAreVisible] ? NSOnState : NSOffState]; remove the "Are" > Tools/MiniBrowser/mac/WK2BrowserWindowController.m:150 > + [menuItem setState:[self layerBordersAreVisible] ? NSOnState : NSOffState]; remove the Ares and Iss
http://trac.webkit.org/changeset/170896