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

WKWebViewOnly setting for removing UIWebView #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidgovea
Copy link

I know that PRs won't be merged in this repo, but adding this here for visibility.
See Issue #46

The most straightforward way to fix the UIWebView deprecation issue is to remove all the related code. Several forks have done this; see:

This PR takes the same approach as cordova-ios and cordova-plugin-inappbrowser, implementing conditional compilation based on the WKWebViewOnly config.xml setting. See Cordova article here for more information.

Adding <preference name="WKWebViewOnly" value="true" /> to the ios platform in config.xml will cause UIWebView code to be removed.

However.. maybe the "remove-all" approach is better? Why do we ever need UIWebView code around now? This change does represent fewer lines changed, so maybe that's something.


This change can also be pulled in using patch-package:

patches/cordova-plugin-remote-injection+0.5.2.patch

diff --git a/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjection.m b/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjection.m
index 9c11eb9..7a72cb2 100644
--- a/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjection.m
+++ b/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjection.m
@@ -81,6 +81,10 @@ - (void) pluginInitialize
     }
 
     id webView = [self findWebView];
+    #if WK_WEB_VIEW_ONLY
+        webViewDelegate = [[CDVRemoteInjectionWKWebViewDelegate alloc] init];
+        [webViewDelegate initializeDelegate:self];
+    #else
     if ([webView isKindOfClass:[UIWebView class]]) {
         NSLog(@"Found UIWebView");
         webViewDelegate = [[CDVRemoteInjectionUIWebViewDelegate alloc] init];
@@ -96,6 +100,7 @@ - (void) pluginInitialize
     } else {
         NSLog(@"Not a supported web view implementation");
     }
+    #endif
 }
 
 /*
diff --git a/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.h b/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.h
index b451824..3967ca9 100644
--- a/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.h
+++ b/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.h
@@ -1,3 +1,5 @@
+#if !WK_WEB_VIEW_ONLY
+
 #import "CDVRemoteInjection.h"
 #import "CDVRemoteInjectionWebViewBaseDelegate.h"
 
@@ -11,3 +13,5 @@
 @interface CDVRemoteInjectionUIWebViewNotificationDelegate : WrappedDelegateProxy <UIWebViewDelegate>
 @property (readwrite, weak) CDVRemoteInjectionUIWebViewDelegate *webViewDelegate;
 @end
+
+#endif
diff --git a/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.m b/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.m
index bb0251e..d1c0dd4 100644
--- a/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.m
+++ b/node_modules/cordova-plugin-remote-injection/src/ios/CDVRemoteInjectionUIWebViewDelegate.m
@@ -2,6 +2,8 @@
 //  CDVRemoteInjection.m
 //
 
+#if !WK_WEB_VIEW_ONLY
+
 #import <Foundation/Foundation.h>
 
 #import "CDVRemoteInjectionUIWebViewDelegate.h"
@@ -98,3 +100,5 @@ - (void) retryCurrentRequest
 }
 
 @end
+
+#endif

@davidgovea davidgovea mentioned this pull request May 20, 2020
@fnicollet
Copy link

As a side note on @davidgovea PR, the forks can be used in Cordova, by replacing the plugin name in package.json with the URL of a fork:

github:RobertY9/cordova-plugin-remote-injection

I was able to pass certification with this setting

Thanks @davidgovea !
Fabien

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants