Bug 145454

Summary: Change method of signaling "should open external urls" to WebKit
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, mitz
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1
mitz: review+
Patch for landing none

Description Brady Eidson 2015-05-28 21:38:26 PDT
Change method of signaling "should open external urls" to WebKit
Comment 1 Brady Eidson 2015-05-28 21:45:23 PDT
Created attachment 253893 [details]
Patch v1
Comment 2 mitz 2015-05-28 21:55:36 PDT
Comment on attachment 253893 [details]
Patch v1

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1777
> +    bool shouldOpenExternalURLs = [[loadOptions valueForKey:_WKShouldOpenExternalURLsKey] boolValue];

-valueForKey is a KVC method. For a dictionary, you should use -objectForKey:, but better yet, we can use dictionary subscripting: loadOptions[_WKShouldOpenExternalURLsKey]

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:30
> +#import <WebKit/WKDeclarationSpecifiers.h>

No need for this…

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:43
> +WK_EXPORT extern NSString * const _WKShouldOpenExternalURLsKey;

…we use WK_EXTERN in the modern API (and it’s coming from WKFoundation which is already imported above).

Even though this is SPI, you should add an availability annotation, WK_AVAILABLE(WK_MAC_TBA, WK_IOS_TBA).

> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:206
> +- (WKNavigation *)loadRequest:(NSURLRequest *)request withOptions:(NSDictionary *)loadOptions;

Like all SPI methods, this one needs to be prefixed with an underscore.

Even though this is SPI, you should add an availability annotation.
Comment 3 Brady Eidson 2015-05-28 22:48:29 PDT
Created attachment 253894 [details]
Patch for landing
Comment 4 WebKit Commit Bot 2015-05-28 23:38:42 PDT
Comment on attachment 253894 [details]
Patch for landing

Clearing flags on attachment: 253894

Committed r184982: <http://trac.webkit.org/changeset/184982>
Comment 5 Alexey Proskuryakov 2015-06-01 19:19:24 PDT
Marking resolved.