[webkit-reviews] review granted: [Bug 176474] Add WKUIDelegatePrivate equivalent of WKPageUIClient's pinnedStateDidChange : [Attachment 320049] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 176474: Add WKUIDelegatePrivate equivalent of WKPageUIClient's
pinnedStateDidChange
https://bugs.webkit.org/show_bug.cgi?id=176474

Attachment 320049: Patch

https://bugs.webkit.org/attachment.cgi?id=320049&action=review




--- Comment #2 from Tim Horton <thorton at apple.com> ---
Comment on attachment 320049
  --> https://bugs.webkit.org/attachment.cgi?id=320049
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320049&action=review

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:5284
> +- (_WKRectEdge) _pinnedState

No space after the )

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:5289
> +- (_WKRectEdge) _rubberBandState

etc.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:5294
> +- (void) _setRubberBandState:(_WKRectEdge)state

This really isn't state, I wonder if we should take this opportunity to rename
it to rubberBandingEnabled or rubberBandableEdges (please no) or something :P

> Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h:83
> +typedef NS_OPTIONS(NSInteger, _WKRectEdge) {

It would be pretty nice if this were a NSRectEdge or UIRectEdge typedef,
wouldn't it? As it stands people are going to have to do silly things to use
this API where they probably have platform edges in their code.