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

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

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

Conversation

JH7
Copy link

@JH7 JH7 commented May 17, 2022

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:

- (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView
{
[webView reload];
}

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 #1232 and meteor/meteor#11811 for an in-depth analysis of the problem and a feasible replication.

#1232 also indicates that the logic behind shouldReloadWebView could be an additional problem. @gouteru describes a further fix in #1232 (comment) where they change this logic. As I found that the tests in

// document with empty title should *not* reload
// baseURL:nil results in about:blank, so we use a dummy here
[wkWebView loadHTMLString:empty_title_document baseURL:[NSURL URLWithString:@"about:"]];

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

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@addonflare
Copy link

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
Copy link

delki8 commented Jun 15, 2022

@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
Copy link
Author

JH7 commented Jun 27, 2022

@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.

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