apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.15k stars 987 forks source link

(ios) feat: navigationAction in shouldOverrideLoadWithRequest #1333

Open ollm opened 1 year ago

ollm commented 1 year ago

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

dpogue commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.