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

feat: add optional compile-time decision for disabling UIWebView #715

Merged
merged 21 commits into from
Nov 20, 2019

Conversation

jcesarmobile
Copy link
Member

@jcesarmobile jcesarmobile commented Nov 9, 2019

Based on #662, updated the prepare script to also modify the CordovaLib project so no removing and re adding platform is needed when <preference name="WKWebViewOnly" value="true" /> was added to the config.xml after the platform was added.
Also makes it possible to unset the value, the implementation on #662 didn't allow to change the value back to 0 without removing the platform.

I added the modified platform and submitted to App store and got the warning, then added <preference name="WKWebViewOnly" value="true" />, ran cordova prepare ios, submitted again and didn't get the warning.

closes: #662

@codecov-io
Copy link

codecov-io commented Nov 9, 2019

Codecov Report

Merging #715 into master will increase coverage by 0.15%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #715      +/-   ##
==========================================
+ Coverage   74.08%   74.24%   +0.15%     
==========================================
  Files          11       11              
  Lines        1829     1844      +15     
==========================================
+ Hits         1355     1369      +14     
- Misses        474      475       +1
Impacted Files Coverage Δ
bin/templates/scripts/cordova/Api.js 70.21% <ø> (ø) ⬆️
bin/templates/scripts/cordova/lib/prepare.js 83.98% <93.75%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9964c74...f7a8948. Read the comment docs.

Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks OK to me.

I would like one more review if possible.

As I see it, the goal of this PR is implement the minimum required changes to disable UIWebView during compile-time when the WKWebView only flag is set. The PR is targeted for a minor release so that these changes should not change the output of a default build. (UIWebView should remain default.)

@erisu erisu requested a review from dpogue November 10, 2019 07:21
@erisu erisu added this to the 5.1.0 milestone Nov 10, 2019
@rajashekaranugu
Copy link

rajashekaranugu commented Nov 19, 2019

@jcesarmobile Can you please explain us clearly the steps you followed.

we also used to get the warning email from apple when we used ionic3, cordova 8.1.2, but currently we have updated our app to IONIC4 and Cordova 9.0.0 (did't deploy app to store yet with updated framework)

Is just adding: < preference name="WKWebViewOnly" value="true" / >, in our config.xml and run cordova prepare ios; enough. Am not clear how to clone this repo. Please explain any other steps involved, so that I will also give a try

Below is the IONIC INFO:

Ionic:

ionic (Ionic CLI) : 4.5.0
Ionic Framework : ionic-angular 3.9.2
@ionic/app-scripts : 3.2.4

Cordova:

cordova (Cordova CLI) : 8.1.2 (cordova-lib@8.1.1)
Cordova Platforms : android 7.1.4, ios 4.5.5
Cordova Plugins : cordova-plugin-ionic-webview 1.2.1, (and 21 other plugins)

System:

ios-deploy : 1.9.4
NodeJS : v10.16.3 (/usr/local/bin/node)
npm : 6.10.1
OS : macOS Mojave
Xcode : Xcode 11.2.1 Build version 11B500.

Thanks for your time

@jcesarmobile
Copy link
Member Author

jcesarmobile commented Nov 19, 2019

you have to clone the repository
git clone https://github.com/apache/cordova-ios
then go to the cloned folder
cd cordova-ios
then check the path to the folder where you cloned cordova-ios
pwd
use the result of that command to install cordova-ios into your project
(from your app project)
cordova platform add (result from pwd here)

Now if you add <preference name="WKWebViewOnly" value="true" /> to the config.xml and run cordova prepare ios the UIWebView conde will be hidden by at build time, so you need a wkwebview plugin installed or your app won't work.

Updated comment since it's merged and I deleted my branch

@rajashekaranugu
Copy link

you have to clone the repository
git clone https://github.com/jcesarmobile/cordova-ios
then go to the cloned folder
cd cordova-ios
then check the branch where I committed the changes
git checkout conditional-webview
then check the path to the folder where you cloned cordova-ios
pwd
use the result of that command to install cordova-ios into your project
(from your app project)
cordova platform add (result from pwd here)

Now if you add <preference name="WKWebViewOnly" value="true" /> to the config.xml and run cordova prepare ios the UIWebView conde will be hidden by at build time, so you need a wkwebview plugin installed or your app won't work.

Thanks for the response will try it and will let you know

@erisu erisu merged commit e8a041e into apache:master Nov 20, 2019
@jcesarmobile jcesarmobile deleted the conditional-webview branch November 20, 2019 13:00
@l3ender
Copy link

l3ender commented Nov 24, 2019

This is a great change--thanks for it. Should it be added to readme for more visibility?

@l3ender
Copy link

l3ender commented Nov 24, 2019

Also, I see the 5.1.0 release hasn't made it to npm registry yet. Is there anything I can do to help with this?

@jcesarmobile
Copy link
Member Author

It’s in the Apache release process at the moment, will take a few days to be released if everything goes fine.

@jcesarmobile
Copy link
Member Author

it's released now https://cordova.apache.org/announcements/2019/11/25/cordova-ios-release-5.1.0.html

@jcesarmobile
Copy link
Member Author

Don't really understand the question

@webb24h
Copy link

webb24h commented Jun 3, 2020

Based on #662, updated the prepare script to also modify the CordovaLib project so no removing and re adding platform is needed when <preference name="WKWebViewOnly" value="true" /> was added to the config.xml after the platform was added.
Also makes it possible to unset the value, the implementation on #662 didn't allow to change the value back to 0 without removing the platform.

I added the modified platform and submitted to App store and got the warning, then added <preference name="WKWebViewOnly" value="true" />, ran cordova prepare ios, submitted again and didn't get the warning.

closes: #662

Oh wow you sir did surely save my life! This works and no more warning in the email!

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

Successfully merging this pull request may close these issues.

None yet

7 participants