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

UIWebView Strategy for Cordova iOS #110

Open
wants to merge 1 commit into
base: master
from

Conversation

@dpogue
Copy link
Member

commented Sep 3, 2019

Background: As of August 2019, Apple is now showing a deprecation warning when uploading apps to the App Store that include UIWebView-related code. As a result, all Cordova apps built for iOS receive this deprecation warning on upload.

We need to determine how Cordova as a project wants to proceed to solve this problem. There are pull requests already being opened that I'm quite concerned with end up breaking all existing Cordova apps, and I think we want to be more careful about that.

I've put together this document outlining some options for discussion.

@NiklasMerz

This comment has been minimized.

Copy link

commented Sep 4, 2019

I personally would vote for option 2 in case of cordova-ios and inappbrowser. It looks like this is the best compromise and backwards compatible way. App developers should have moved to WKWebView with the plugin for some time and this way the upgrade should be the easiest way.

We are still relying on IAB for some legacy sites embedded in the "new" app in Cordova. There are plans to get rid of them in the future but that may take a long time and could cause compatibility issues with old versions of our product.

That IAB is a bad practice should be made clear with advice how to use it, but I would argue that this is not a strong enough reason to drop it completely. But I must admit that SafariViewControler looks like the way to go in the far future.

@NiklasMerz
Copy link

left a comment

Add this file to README?

* Moving all apps over to WKWebView will result in data being lost (localStorage, indexedDB, etc.)
* WKWebView does not support all the same features as UIWebView and has stricter security requirements, and **many existing Cordova iOS apps will not work if run in WKWebView** without changes
* The current WKWebView plugin needs updates to use Scheme Handlers to avoid issues that require a local web server.
* The InAppBrowser plugin also uses UIWebView. Moving to WKWebView would potentially lose some functionality.

This comment has been minimized.

Copy link
@janpio

janpio Sep 4, 2019

Member

InAppBrowser has supported WKWebview for quite some time via usewkwebview.

This comment has been minimized.

Copy link
@NiklasMerz

NiklasMerz Sep 4, 2019

IAB with WKWebView basically works fine now except this issue apache/cordova-plugin-inappbrowser#492

IAB also needs to hide UIWebView behind a build flag, like some other plugins.

* WKWebView does not support all the same features as UIWebView and has stricter security requirements, and **many existing Cordova iOS apps will not work if run in WKWebView** without changes
* The current WKWebView plugin needs updates to use Scheme Handlers to avoid issues that require a local web server.
* The InAppBrowser plugin also uses UIWebView. Moving to WKWebView would potentially lose some functionality.
* Ideally, the InAppBrowser should be deprecated in favour of SFSafariViewController which severely restricts functionality in the name of security. Currently InAppBrowser use could largely be considered bad practice.

This comment has been minimized.

Copy link
@janpio

janpio Sep 4, 2019

Member

Very much different use case and functionality, also not related to UIWebView<->WKWebView. This should not be part of this discussion.

This comment has been minimized.

Copy link
@dpogue

dpogue Sep 4, 2019

Author Member

Very much different use case and functionality, also not related to UIWebView<->WKWebView. This should not be part of this discussion.

Agreed, but I also feel that deprecating IAB is in line with "the goal of [Cordova] is to cease to exist" now that there are platform-supported better options.
I'll drop this point from the discussion.

This comment has been minimized.

Copy link
@jcesarmobile

jcesarmobile Sep 4, 2019

Member

It's definitely a better option for what IAB was originally created, safe web browsing.
But as security is its strong suit, it's not as feature rich as IAB, all code/css injection, events, etc. that people relies on nowadays is not possible, so we can't deprecate it.

This comment has been minimized.

Copy link
@NiklasMerz

NiklasMerz Sep 4, 2019

I agree. I have to maintain some pages which rely on that for some time. If you want your pages in IAB tightly integrated to your app you need this script injection stuff. Deprecating it should be the last resort, since it works fine now if you know what you are doing and use it in a responsible way. So security is not really a point.


The steps are roughly:

1. Move the existing WKWebView plugin into cordova-ios.

This comment has been minimized.

Copy link
@janpio

janpio Sep 4, 2019

Member

Interesting to note: There are multiple, different WKWebView plugins out in the ecosystem. Just integrating the Cordova one would probably break all those.

@janpio

This comment has been minimized.

@NiklasMerz

This comment has been minimized.

Copy link

commented Sep 4, 2019

I think https://github.com/ionic-team/cordova-plugin-ionic-webview is the elephant in the room - how does this very popular plugin fit in here?

https://npm-stat.com/charts.html?package=cordova-plugin-ionic-webview&package=cordova-plugin-wkwebview-engine&from=2018-09-03&to=2019-09-03

Like said in the document the apache wkwebview engine needs to implement Scheme handlers. Scheme handlers in the ionic plugin make CORS and URL navigation easier, thats probably why it is the more popular choice apart being the default in new Ionic apps.

@janpio

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Possibly true, but not my point.

Should/Could this much more popular plugin be integrated? How can this plugin keep existing after Cordova iOS switched over to WKWebView (all those current users of it need a migration path)? What other side effects are there because of this situation of multiple plugins being there?

Ionic should be an active part of this discussion.

@NiklasMerz

This comment has been minimized.

Copy link

commented Sep 4, 2019

Good point. @jcesarmobile is probably the right person to ask.

@jcesarmobile

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

Bringing a previous discussion link where we agreed to rename the cordova-plugin-wkwebview-engine classes on cordova-ios integration so it doesn't break existing WKWebView plugins that started as a fork (probably all of them), and keep the plugable webviews so people can still use different plugins from the default.
https://markmail.org/message/fxe6q6nnernvr6h6.

The problem with Scheme handlers is they are iOS 11+, so we should deprecate iOS 10. We can probably fallback to file urls on iOS 10, but I think having different behaviours is worse for the users.

We have 2 PRs at the moment:
first one add a preference that enables the compiler definition to disable UIWebView if desired. I think this is a third option, similar to the option 2, but as long as we don't disable it by default, we can release this as a minor release as it's a new feature and not a breaking change. With this in place it should be easier to transition to option 2 in a major version where we change the default from UIWebView to WKWebView. This also "unlock" people that are not using UIWebView but still get the warning.

The second PR claims that the previous one didn't work for him and still got the warning. This one is for 1, removing UIWebView entirely, but doesn't rename the classes, so other plugins will collide and fail to install. Also doesn't have url scheme handlers as it's a direct port of current plugin.

So I personally like more the first one, but we should check if we still get the warning and try to figure out why to properly fix it.

About integrating the Ionic one, it has a little Ionic Appflow code that I don't think fits into cordova-ios, so better keep them separate. But I can give a hand with the scheme handlers as I implemented it on the ionic one. But as I said, that would mean deprecating iOS 10.

@timbru31

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

I think ionic-team/cordova-plugin-ionic-webview is the elephant in the room - how does this very popular plugin fit in here?

Ionic should be an active part of this discussion.

I totally agree with you Jan!
How about the folks over at @ionic-team give something back to the Cordova community by contributing their cordova-plugin-ionic-webview fork back to the (orignal) cordova-plugin-wkwebview-engine since the fork already has been migrated to handle, e.g., the scheme handler for iOS 11+? IMHO this would help the Cordova project to tackle this big hurdle much faster.


Regarding the proposed options from Darryl, I would favor the "hard removal" options. We are very limited in regards of the development capacity and removing old, legacy, and deprecated code instead of maintaining both of them makes things easier.

@NiklasMerz

This comment has been minimized.

Copy link

commented Sep 4, 2019

Just another thought. I don't know if it makes sense or is technically possible.

Move UIWebView to a plugin as well and make cordova-ios use this or some of the WKWebView plugins. cordova-ios just contains the code to load a webview. By default the next version should load the Apache WKWebViewEngine (Plugin installed by default on new apps, Migration instruction for old apps). Users who need UIWebView, the Ionic one, a fork or others can specify their own one like it is now.

That way no UIWebView would be in cordova-ios and it would still be flexible enough like today.

@jcesarmobile

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

That could work too, but it's a breaking change as they will need to install a webview plugin, and probably more work to move the UIWebView code into a separate plugin that will be short lived.

@dpogue

This comment has been minimized.

Copy link
Member Author

commented Sep 4, 2019

The problem with Scheme handlers is they are iOS 11+, so we should deprecate iOS 10.

This seems fine to me, but should probably be brought to the list for discussion. iOS 13 is rolling out soon, and Apple traditionally only supports the two most recent iOS versions.

we can release this as a minor release as it's a new feature and not a breaking change.

I feel pretty strongly that we should not be making any of these changes in a minor version bump.

keep the plugable webviews so people can still use different plugins from the default.

We will definitely be keeping the pluggable webview support, so it should be possible for people to keep using other webview plugins.

we agreed to rename the cordova-plugin-wkwebview-engine classes on cordova-ios integration so it doesn't break existing WKWebView plugins that started as a fork

Thank you for reminding me of this. I'll update the document to make it clear that we need to rename classes when we integrate.

@jcesarmobile

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

This seems fine to me, but should probably be brought to the list for discussion. iOS 13 is rolling out soon, and Apple traditionally only supports the two most recent iOS versions.

We unofficially also only support the last 2 versions, but we have been slow at removing them and if we do it fast people complain.

I feel pretty strongly that we should not be making any of these changes in a minor version bump.

If you see the first PR is pretty "simple". If you don't have the preference it will behave the same way it behaves at the moment, if you have the preference it will switch a few lines in compile time. I think that can safely go into a minor release and give us more time for a complete integration in next major release.

We will definitely be keeping the pluggable webview support, so it should be possible for people to keep using other webview plugins.
Thank you for reminding me of this. I'll update the document to make it clear that we need to rename classes when we integrate.

Related to the first point too. As we are integrating the WKWebView plugin renaming the classes, adding the scheme handlers and keeping the plugable webviews, in worst case, people can manually change deployment target to iOS 10 and still use the current cordova-plugin-wkwebview-engine that uses file instead of the scheme handlers.

@bpresles

This comment has been minimized.

Copy link

commented Sep 4, 2019

After reading this discussion, I found a good idea to rename classes of embedded WKWebView engine so that they don't conflict with existing engine plugins.

So the PR 663 (apache/cordova-ios#663) includes that change. I've tested it with stock v4.1.1 of cordova-plugin-ionic-webview and it's working just fine without any modification.

Also I created a UIWebView plugin based on current default UIWebView engine in cordova-ios (https://github.com/bpresles/cordova-plugin-uiwebview-engine) and published a first release : https://www.npmjs.com/package/cordova-plugin-uiwebview-engine
The funny thing is that this UIWebView engine can be used even on current and previous releases of cordova-ios (even though it's plain useless ;) ) as I took care to rename the classes ;)

@bryyyanribo

This comment has been minimized.

Copy link

commented Sep 5, 2019

@bpresles, What do you think is the best approach to this problem?

Should I Install this Cordova plugin for me to temporarily fix the issue regarding UIWebView?

4. Add warnings to cordova-ios for apps that are using the UIWebView preference.
5. Wrap all the UIWebView code behind a compiler definition that can be controlled by a preference, to allow disabling all the UIWebView code.
6. [Next Major] Disable the UIWebView code by default, but allow it to be enabled by the preference for apps that require it.

This comment has been minimized.

Copy link
@NiklasMerz

NiklasMerz Sep 5, 2019

I would add "Move UIWebView to plugin" as an option to this document, too. Here a first draft based on comments on this PR.

Suggested change
### 3 . Move UIWebView to a plugin and remove it from Cordova-ios
Move UIWebView to a plugin as well and make cordova-ios use this or some of the WKWebView plugins. cordova-ios just contains the code to load a webview. By default the next version of Cordova-ios should load the Apache WKWebViewEngine (Plugin installed by default on new apps, Migration instruction for old apps). Users who need UIWebView, the Ionic one, a fork or others can specify their own one like it is now.
There is already a proof-of-concept version of this done: https://github.com/bpresles/cordova-plugin-uiwebview-engine with https://github.com/apache/cordova-discuss/pull/110#issuecomment-528088573
This is a breaking change since existing apps need to install that plugin and creates work for a plugin that is probably short lived. InAppBrowser needs some work to get this working.
@bpresles

This comment has been minimized.

Copy link

commented Sep 6, 2019

@bpresles, What do you think is the best approach to this problem?

Should I Install this Cordova plugin for me to temporarily fix the issue regarding UIWebView?

Adding a WKWebView engine plugin won't remove the warning with current cordova-ios version as it still embed the UIWebView based engine even if you install a WKWebView engine plugin` that replaces it at execution time.

But yes, that's a good idea that you install a WKWebView engine plugin in your project to prepare it for the future, because you may (will?) have to adapt some part of your code to make it compatible with WKWebView, and so doing it the sooner possible makes sure that your app code is already prepared for when Apple will reject apps with UIWebView and so for future version of cordova-ios that will stop using UIWebView.

@timbru31 timbru31 referenced this pull request Sep 9, 2019
3 of 5 tasks complete
@manish2535

This comment has been minimized.

Copy link

commented Sep 12, 2019

Any updates on this guys?

@realappie

This comment has been minimized.

Copy link

commented Sep 12, 2019

I am personally for option one, sooner or later people will have to get rid of UIWebView We will be either not able to upload apps to the app store, or transition to WKWebView incase we want to upgrade cordova-ios. The latter seems more logical to me.

@rajashekaranugu

This comment has been minimized.

Copy link

commented Sep 12, 2019

Hi guys, I have also received same warning email from apple. currently am using ionic 4.5.0 and cordova 8.1.2 to build my iOS APP where we are using both inAppBrowser (3.1.0) and ionicWebView(4.1.1).

Really appreciate your time if some one can help me with below questions {{currently we are holding on a release }}

    1. do we have any specific time that iOS will stop accepting the apps with UIWebView.?
    1. instead of using in-app-browser can we use safari-view-controller ?
    1. if I release my app with above specifications (with out any changes) how long my app will be supported ?
    1. any solution for this so far?
@breautek

This comment has been minimized.

Copy link

commented Sep 12, 2019

@rajashekaranugu I'd prefer to keep comments on this ticket relating to discussing the solution of this issue instead of providing support but I'll quickly answer what I can below. If you have further questions, please read through apache/cordova-ios#661 and comment there, or create a new issue under cordova-ios repo.

do we have any specific time that iOS will stop accepting the apps with UIWebView.?

Apple has not provided any dates, so we don't know.

if I release my app with above specifications (with out any changes) how long my app will be supported ?

Due to the first question, again we don't know.

any solution for this so far?

There is no official solution. That is what this ticket is for, discussing how Cordova should proceed further. There are some PR proposals (apache/cordova-ios#663 & apache/cordova-ios#662) but they are likely introducing breaking changes and may not actually follow what Cordova thinks is the best way moving forward so no guarantee that they will be merged in or be a long term solution.

Again, if you have other questions, please use apache/cordova-ios#661 or create a new issue in the cordova-ios repo.

And of course, you are welcome to read through this discussion and provide your thoughts on how Cordova should move forward.

@EdwinRozario

This comment has been minimized.

Copy link

commented Sep 12, 2019

Ionic devs please also take a look at blog post. Ionic team might be able to provide some support while the cordova team is working on the fix.

@derman10

This comment has been minimized.

Copy link

commented Sep 12, 2019

Ionic devs please also take a look at blog post. Ionic team might be able to provide some support while the cordova team is working on the fix.

thanks for sharing!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.