Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: navigation delegation loops #1272

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ - (nullable instancetype)initWithFrame:(CGRect)frame configuration:(nullable WKW
{
self = [super init];
if (self) {
if (NSClassFromString(@"WKWebView") == nil) {
return nil;
}

self.configuration = configuration;
self.engineWebView = configuration ? [[WKWebView alloc] initWithFrame:frame configuration:configuration] : [[WKWebView alloc] initWithFrame:frame];
}
Expand Down Expand Up @@ -497,7 +493,7 @@ - (void)userContentController:(WKUserContentController*)userContentController di
}
}

#pragma mark WKNavigationDelegate implementation
#pragma mark - WKNavigationDelegate implementation

- (void)webView:(WKWebView*)webView didStartProvisionalNavigation:(WKNavigation*)navigation
{
Expand Down Expand Up @@ -547,44 +543,57 @@ - (BOOL)defaultResourcePolicyForURL:(NSURL*)url

- (void) webView: (WKWebView *) webView decidePolicyForNavigationAction: (WKNavigationAction*) navigationAction decisionHandler: (void (^)(WKNavigationActionPolicy)) decisionHandler
{
NSURL* url = [navigationAction.request URL];
CDVViewController* vc = (CDVViewController*)self.viewController;

/*
* Give plugins the chance to handle the url
*/
BOOL anyPluginsResponded = NO;
BOOL shouldAllowRequest = NO;

for (NSString* pluginName in vc.pluginObjects) {
CDVPlugin* plugin = [vc.pluginObjects objectForKey:pluginName];
SEL selector = NSSelectorFromString(@"shouldOverrideLoadWithRequest:navigationType:");
if ([plugin respondsToSelector:selector]) {
anyPluginsResponded = YES;
// https://issues.apache.org/jira/browse/CB-12497
int navType = (int)navigationAction.navigationType;
shouldAllowRequest = (((BOOL (*)(id, SEL, id, int))objc_msgSend)(plugin, selector, navigationAction.request, navType));
if (!shouldAllowRequest) {
break;
// Give plugins the chance to handle the url, as long as this WebViewEngine is still the WKNavigationDelegate.
// This allows custom delegates to choose to call this method for `default` cordova behavior without querying all plugins.
if (webView.navigationDelegate == self) {
__block BOOL anyPluginsResponded = NO;
__block BOOL shouldAllowRequest = NO;
CDVViewController* vc = (CDVViewController*)self.viewController;
NSDictionary *pluginObjects = [NSDictionary dictionaryWithDictionary:vc.pluginObjects];
[pluginObjects enumerateKeysAndObjectsUsingBlock:^(NSString* _Nonnull pluginName, CDVPlugin* _Nonnull plugin, BOOL * _Nonnull stop) {
if ([plugin respondsToSelector:[self navSelector]]) {
anyPluginsResponded = YES;
shouldAllowRequest = [self allows:plugin navigationAction:navigationAction];
if (!shouldAllowRequest) {
*stop = YES;
}
}
}];
if (anyPluginsResponded) {
return decisionHandler(shouldAllowRequest);
}
} else {
// The default behavior is for just the CDVIntentAndNavigationFilter to handle this
CDVPlugin *intentAndNavFilter = [((CDVViewController*)self.viewController).pluginObjects objectForKey:@"CDVIntentAndNavigationFilter"];
if ([intentAndNavFilter respondsToSelector:[self navSelector]]) {
return decisionHandler([self allows:intentAndNavFilter navigationAction:navigationAction]);
}
}

if (anyPluginsResponded) {
return decisionHandler(shouldAllowRequest);
}

/*
* Handle all other types of urls (tel:, sms:), and requests to load a url in the main webview.
*/
// Handle all other types of urls (tel:, sms:), and requests to load a url in the main webview.
NSURL* url = [navigationAction.request URL];
BOOL shouldAllowNavigation = [self defaultResourcePolicyForURL:url];
if (shouldAllowNavigation) {
return decisionHandler(YES);
} else {
if (!shouldAllowNavigation) {
[[NSNotificationCenter defaultCenter] postNotification:[NSNotification notificationWithName:CDVPluginHandleOpenURLNotification object:url]];
}

return decisionHandler(NO);
return decisionHandler(shouldAllowNavigation);
}

#pragma mark WKNavigationDelegate private helpers

/// Creates the selector corresponding to `shouldOverrideLoadWithRequest:navigationType:`
- (SEL)navSelector {
return NSSelectorFromString(@"shouldOverrideLoadWithRequest:navigationType:");
}

/// Determines if a request should be allowed
/// @param plugin that supports `shouldOverrideLoadWithRequest:navigationType:`
/// @param navigationAction from the WKNavigationDelegate containing a request
- (BOOL)allows:(CDVPlugin *)plugin navigationAction:(WKNavigationAction *)navigationAction {
// https://issues.apache.org/jira/browse/CB-12497
int navType = (int)navigationAction.navigationType;
return (((BOOL (*)(id, SEL, id, int))objc_msgSend)(plugin, [self navSelector], navigationAction.request, navType));
}

#pragma mark - Plugin interface
Expand Down
6 changes: 5 additions & 1 deletion CordovaLib/include/Cordova/CDVWebViewEngineProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@
#define kCDVWebViewEngineWKUIDelegate @"kCDVWebViewEngineWKUIDelegate"
#define kCDVWebViewEngineWebViewPreferences @"kCDVWebViewEngineWebViewPreferences"

@protocol CDVWebViewEngineProtocol <NSObject>
/// Worth noting: by default, the CDVWebViewEngine is the WKNavigationDelegate, but only if you haven't either:
/// [a] extended CDVViewController to be a WKNavigationDelegate
/// [b] called `updateWithInfo` and specified a kCDVWebViewEngineWKNavigationDelegate
/// It would be more explicit to set the navigation delegate in the convenience initializer below, but I'll avoid doing that because these two other ways already exist.
@protocol CDVWebViewEngineProtocol <WKNavigationDelegate>

NS_ASSUME_NONNULL_BEGIN

Expand Down