WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
145454
Change method of signaling "should open external urls" to WebKit
https://bugs.webkit.org/show_bug.cgi?id=145454
Summary
Change method of signaling "should open external urls" to WebKit
Brady Eidson
Reported
2015-05-28 21:38:26 PDT
Change method of signaling "should open external urls" to WebKit
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
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2015-05-28 21:45:23 PDT
Created
attachment 253893
[details]
Patch v1
mitz
Comment 2
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.
Brady Eidson
Comment 3
2015-05-28 22:48:29 PDT
Created
attachment 253894
[details]
Patch for landing
WebKit Commit Bot
Comment 4
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
>
Alexey Proskuryakov
Comment 5
2015-06-01 19:19:24 PDT
Marking resolved.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug