Bug 145454 - Change method of signaling "should open external urls" to WebKit
Summary: Change method of signaling "should open external urls" to WebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-28 21:38 PDT by Brady Eidson
Modified: 2022-03-01 02:47 PST (History)
3 users (show)

See Also:


Attachments
Patch v1 (14.42 KB, patch)
2015-05-28 21:45 PDT, Brady Eidson
mitz: review+
Details | Formatted Diff | Diff
Patch for landing (14.28 KB, patch)
2015-05-28 22:48 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.