(ios) feat: navigationAction in shouldOverrideLoadWithRequest

[ollm]

Platforms affected

iOS

Motivation and Context

Currently shouldOverrideLoadWithRequest only returns the request and navigationType, the plugin I'm working with needs to check if navigationAction.sourceFrame is null or not to decide whether to open the url in the browser, currently I can't do that (far as I know).

Description

Added a new shouldOverrideLoadWithRequest:navigationAction selector to have full access to the navigationAction object.

Testing

I have kept the old implementation so that plugins that already use it are not affected, with the change both the old and the new one work correctly.

I don't know if there is a better way to implement this, since my knowledge of Objective-C is basic, or if it would be better to replace completely with the new one, to avoid redundancy.

Checklist

• I've run the tests to see all new and existing tests pass
• I added automated test coverage as appropriate for this change
• Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
• If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
• I've updated the documentation if necessary

[dpogue]

The problem with this approach is that it exposes WebKit-specific API (by exposing WKNavigationAction) as part of Cordova's API, and we try not to do that so that we don't end up with breaking API changes when Apple decides to change something. We had a bunch of issues in the past due to the deprecation of UIWebView impacting our plugin API and breaking a bunch of 3rd party plugins.

What information about the frame do you need specifically? I'm guessing the security origin?

[ollm]

Currently I only check if sourceFrame is null, in the plugin I'm working on, when clicked on links inside iframes they have to open in the browser, and some website iframes in particular (It happens to me in Youtube Video Iframe and AdSense Ad), when clicked the navigationType it's in .other and not in .linkActivated, I can't tell it apart from other requests like loading external scripts, the only difference I've noticed is that those have the sourceFrame set to null and the others don't.

For my part, just passing the sourceFrame or indicating if it is null would be enough for me.

[ollm]

As an example, this is the code that I am currently testing in the plugin.

@objc func shouldOverrideLoadWithRequest(_ request: URLRequest, navigationAction: WKNavigationAction) -> Bool {

    var allowNavigationsPass = true

    if let url = request.url, url.scheme == "http" || url.scheme == "https" {

        if navigationAction.sourceFrame == nil {
            allowNavigationsPass = false
        }

        switch navigationAction.navigationType {
            case .linkActivated: // This is not triggered in some iframes
                allowNavigationsPass = false
            case .other:
                let range = url.absoluteString.range(of: "utm_content")
                if range != nil {
                    allowNavigationsPass = false
                }
            default:
                break
        }

        if !allowNavigationsPass {
            UIApplication.shared.open(url)
        }
    }

    return allowNavigationsPass
}

[ollm]

What do you think about something similar to this? This should avoid the problem of exposing the WebKit API and should allow you to extend it if a plugin needs other information in the future.

@interface CDVWebViewOverrideLoadWithRequest : NSObject

@property(nonatomic, readwrite) NSURLRequest * request;
@property(nonatomic, readwrite) int navigationType;
@property(nonatomic, readwrite) Boolean sourceFrameIsNil;
//@property(nonatomic, readwrite) WKFrameInfo * sourceFrame;
// ...

@end

@implementation CDVWebViewOverrideLoadWithRequest

@end

and

SEL selector2 = NSSelectorFromString(@"shouldOverrideLoadWithRequest:overrideLoadWithRequest:");
if ([plugin respondsToSelector:selector2]) {
    anyPluginsResponded = YES;
    CDVWebViewOverrideLoadWithRequest *overrideLoadWithRequest = [[CDVWebViewOverrideLoadWithRequest alloc] init];
    overrideLoadWithRequest.request = navigationAction.request;
    overrideLoadWithRequest.navigationType = (int)navigationAction.navigationType;
    overrideLoadWithRequest.sourceFrameIsNil = (navigationAction.sourceFrame == nil) ? YES : NO;
    //...
    shouldAllowRequest = (((BOOL (*)(id, SEL, id, id))objc_msgSend)(plugin, selector2, navigationAction.request, overrideLoadWithRequest));
    if (!shouldAllowRequest) {
        break;
    }
}

Sorry if the code is not quite correct, I'm still learning how Objective-C works.

[dpogue]

My current thinking is that we could pull the properties of the WKNavigationAction into an NSDictionary and pass that along as an "extra data" parameter to the plugin handler method. It's not as strongly typed as what you've proposed, but is more future compatible for other types of WebViews with their own navigation properties (since all the dictionary keys would be optional)

[ollm]

It looks good to me.