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

Optional compile-time decision for WKWebView over UIWebView #662

Closed
wants to merge 20 commits into from
Closed

Optional compile-time decision for WKWebView over UIWebView #662

wants to merge 20 commits into from

Conversation

fjaeger
Copy link

@fjaeger fjaeger commented Aug 30, 2019

As Apple seems to become super picky about the usage of UIWebView APIs, this PR adds a preference (WKWebViewOnly) to config.xml, that makes sure that all traces of UIWebView are left behind on compile-time.

This PR should resolve #661

@codecov-io
Copy link

Codecov Report

Merging #662 into master will decrease coverage by 0.05%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #662      +/-   ##
==========================================
- Coverage   74.24%   74.19%   -0.06%     
==========================================
  Files          11       11              
  Lines        1833     1837       +4     
==========================================
+ Hits         1361     1363       +2     
- Misses        472      474       +2
Impacted Files Coverage Δ
bin/templates/scripts/cordova/Api.js 70.56% <ø> (ø) ⬆️
bin/templates/scripts/cordova/lib/prepare.js 83.37% <60%> (-0.3%) ⬇️

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 be68c9f...c19b6fa. Read the comment docs.

@jcesarmobile
Copy link
Member

I've tested the PR and doesn't work, the problem is the WK_WEB_VIEW_ONLY is added to both the CordovaLib and the user projects, but when setting the preference to true, the WK_WEB_VIEW_ONLY is only set to 1 in the user project, but not in CordovaLib, so always build with 0 value and have no effect.
Manually changing the value to 1 works.

@fjaeger
Copy link
Author

fjaeger commented Sep 5, 2019

Please try to re-add the iOS platform. Then it should work. Forgot to mention that part ;)

@jcesarmobile
Copy link
Member

Yeah, removing and adding the platform back works if you have the preference, but I think the WK_WEB_VIEW_ONLY value of CordovaLib value should be updated on prepare command as the WK_WEB_VIEW_ONLY value of the user project does.

@lcaprini
Copy link

Hi @fjaeger,
I used your cordova-ios fork but the warning still remains. I added in the config.xml the following preference:
<preference name="WKWebViewOnly" value="true" />
and all WK_WEB_VIEW_ONLY in XCode project was set to 1.
By the way Apple continues to send me the warning via email...

Did you tried to upload to the iTunes Connect one app with your cordova-ios platform?

Thanks,
--Luca

@NiklasMerz
Copy link
Member

NiklasMerz commented Sep 19, 2019

Hi @fjaeger,
I used your cordova-ios fork but the warning still remains. I added in the config.xml the following preference:
<preference name="WKWebViewOnly" value="true" />
and all WK_WEB_VIEW_ONLY in XCode project was set to 1.
By the way Apple continues to send me the warning via email...

Did you tried to upload to the iTunes Connect one app with your cordova-ios platform?

Thanks,
--Luca

This is probably not related. Please check if you use any plugins that reference UIWebView.

@willnix86
Copy link

I also couldn't get this to work, sadly. If I search my Xcode codebase for the project, I still see a WHOLE bunch of UIWebView references (in Cordova itself, not just plugins) - as far as I'm aware, Apple just scans your code and sends the warning if it finds refs to UIWebView, right?

@jcesarmobile
Copy link
Member

This works if you have the preference before adding the platform, or if you remove and add it again. The compiler flags will prevent Apple from seeing the UIWebView references.

@brodycj
Copy link
Contributor

brodycj commented Nov 1, 2019

This works if you have the preference before adding the platform, or if you remove and add it again.

This looks like it could be a major pitfall for people not super familiar with Cordova.

Assuming we document this option, I think a big bold warning statement would be needed.

Any way we could have Cordova build show a warning if someone changes the preference without removing and adding back cordova-ios?

@jcesarmobile
Copy link
Member

I think the PR should be changed so it updates the project without removing and re-adding the platform if possible. If not possible, then document it.

@@ -659,6 +660,7 @@
PRODUCT_NAME = Cordova;
SDKROOT = iphoneos;
SKIP_INSTALL = YES;
WK_WEB_VIEW_ONLY = 0;
Copy link

@tom-hartz tom-hartz Nov 4, 2019

Choose a reason for hiding this comment

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

This should reflect the config.xml preference, instead of always being 0.

@tom-hartz
Copy link

I am seeing the same issue as jcesarmobile, setting the preference
<preference name="WKWebViewOnly" value="true" />
and then re-adding the platform.

It only sets the WK_WEB_VIEW_ONLY=1 flag in the user project. It then has to be manually changed in the CordovaLib project.

https://github.com/apache/cordova-ios/pull/662/files#r342301814

@fjaeger is there a way to read the config.xml preference instead of putting 0 here?

@jcesarmobile
Copy link
Member

I've modified your prepare script to make prepare edit the projects without the need to removing and adding the platform back.
#715

@erisu erisu closed this in #715 Nov 20, 2019
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.

Remove all UIWebView code?
9 participants