apache / cordova-ios

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

Fix #1232: Reload issue resulting in white screen #1235

Open JH7 opened 2 years ago

JH7 commented 2 years ago

Fixes #1232 Fixes meteor/meteor#11811

Platforms affected

iOS

Motivation and Context

If the iOS system kills the WKWebView processes due to e.g. memory issues, the Cordova app tries to reload the current URL on becoming foreground here:

https://github.com/apache/cordova-ios/blob/67b0bb2cfceb3f04fcd25a09222e86404805c594/CordovaLib/Classes/Private/Plugins/CDVWebViewEngine/CDVWebViewEngine.m#L530-L533

Due to client side routing, the current page may not be the correct initial URL (which includes an access token e.g.). Hence, some apps show a blank screen due to failure of reloading.

See https://github.com/apache/cordova-ios/issues/1232 and https://github.com/meteor/meteor/issues/11811 for an in-depth analysis of the problem and a feasible replication.

https://github.com/apache/cordova-ios/issues/1232 also indicates that the logic behind shouldReloadWebView could be an additional problem. @gouteru describes a further fix in https://github.com/apache/cordova-ios/issues/1232#issuecomment-1112812092 where they change this logic. As I found that the tests in

https://github.com/apache/cordova-ios/blob/67b0bb2cfceb3f04fcd25a09222e86404805c594/tests/CordovaLibTests/CDVWebViewEngineTest.m#L191-L193

explicitly state that an empty title should not result in a reload, I decided to not include they fix. But it could also be possible that the fix is required, as a result, the tests would have to be changed accordingly. If checking the title for 0 lengthiness is strictly required for your use case, please correct me @gouteru.

Description

Since a reload is not sufficient, CDVWebViewEngine.m gets the initial app URL via CDVViewController (by making appUrl a public method) and initiates a new request with the initial app URL. The splashscreen is shown before the new request to hide a temporary white screen from the user.

Testing

I used the changes with my Meteor/Cordova app on different iOS devices and I could not detect any white screen.

Checklist

addonflare commented 2 years ago

Thanks for this patch, applied it but unfortunately still getting the blank screen after the app has been running in the background for some time. Any way we can chat? I can pay for your help

delki8 commented 2 years ago

@JH7 Thank you for this. We've been facing a similar issue with our app and I'd like to test your solution. I'm already running meteor from checkout and from what I understood it will use your fix through cordova-plugin-meteor-webapp.

But since cordova-plugin-meteor-webapp appears to be mainly composed of native code, I'm not sure how to build it pointing to my local version of cordova-ios. Would you mind providing some instructions on how to test your fix on a meteor checkout?

JH7 commented 2 years ago

@delki8 cordova-plugin-meteor-webapp does not use cordova-ios directly. cordova-ios is rather installed by meteor add-platform ios (along with other Cordova packages).

Currently, I'm applying these fixes by hand, meaning: I build my Meteor project with meteor build, then I open the built Xcode project. Fromt there, I manually edit the source files (CDVWebViewEngine.m and CDVViewController.h) by hand before I build the project in Xcode. So I think that you need some sort of Meteor project set up, I cannot say anything regarding testing directly on a Meteor checkout since I haven't tried that.

I'm pretty sure there is a smarter way to do this - I did not try anything here since Meteor uses its bundled npm for the Cordova package installations (at least I guess so), so I don't know how to force Meteor to resolve the npm pacakge cordova-ios to my fork on GitHub for example.