-
Notifications
You must be signed in to change notification settings - Fork 396
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 wksuspendinbackground for iOS12.2 #343
fix wksuspendinbackground for iOS12.2 #343
Conversation
Includes backward compatibility for earlier versions
This has never been a bug on the webview plugin, the apps crashing were using katzer background plugin. Our approach was still working on iOS 12.2. We removed it from webview plugin because it's a private API and because it was causing white screens when coming to foreground after a few (~3) minutes. Looks like the white screen problem was solved before 12.2 final, or at least I can't reproduce now. Not sure if we will merge this, despite the method name it's hidden and Apple won't notice, it's still private, we are discussing it, will let you know what we decide. Can't this be implemented in the katzer plugin (or in a different background plugin) instead of putting it inside the webview plugin? BTW, the PR is not working, you are not setting the value for the method anywhere. |
@jcesarmobile yeh I understand that it's a private API but being able to have javascript run in the background for valid use cases is kind of essential and this was the only way to work around it. This could be implemented in the background-mode plugin, however, that plugin also fakes it's background capability and plays silent audio to force infinite background run time even when not appropriate. This may be a solution for some specific use cases but I think having a simple solution in place that just makes background processing work IF you have a valid use and capability set should be part of the webview plugin as this is basic APP functionality that people would expect. I do think this setting should default to disabled so that only if people have a valid need do they turn it on and the base case remains for most. I never tested with the 12.2 beta's and haven't ever experienced the "white screen" issue, so hopefully that was resolved by the GA release, as many things often are with Apple betas. Also thanks for pointing out the missing setting! I was working in xcode to test rapidly and then ported the code back into the plugin and missed a couple of lines, will add a further commit to resolve this. |
removed obfuscation now applies config correctly
Now that you removed the obfuscation, wouldn’t Apple complain again on submissions? |
@jcesarmobile Apple has never complained on submission in my experience across many apps using this. I see some people did report issues but I would be interested to see their background run time use case and other plugins as I suspect there might be something more going on (like background mode plugin installed and claiming background audio capability) However, I have been doing some testing and found that the version check isn't really necessary in this plugin. As we expose the property with a setter and the setter is still the same and will set correctly to the updated property key without any change in this implementation. In the background-mode plugin they are not using the setter method but manually injecting the property value themselves, so when the property value key changed the background-mode plugin broke and need needed to be adjusted to set the new property key. So while that actually makes this PR pointless it does leave us with 2 options:
I think 2 would be a good option as then people who need it can add it, keeps everything nice and modular and removes those bits from the background-mode plugin that make it not app store compliance, battery hungry etc. Those who need "infinite, forced" background run time can use the original plugin. Would Ionic team be interested in making an ionic-background-mode fork? I am happy to do the work adjusting the plugin, ionic-native wrapper and docs appropriately and actively maintain the fork for you. |
@ghenry22 Patch does not compile in XCode because |
@ghenry22 I'm not able to build with the patch but really need that background option back to allow me to send a quick request when app goes into the background |
@jcesarmobile @ghenry22 We would love this to be merged in. because we still have WKWebkit restarting the app, from background usage. Any chance this can be fixed? |
added config setting in wrong location, should be fixed now.
@bramk @geshub @gregavola sorry that was a stupid mistake on my part putting something in the wrong place. It should be fixed now. |
looks like @jcesarmobile has merged the wksuspendinbackground setting back in to master with this change: So this PR should be unnecessary now. |
Yeah, sorry, I forgot to mention. I went with changes based on your first commit because we previously had reports from some other users receiving warning messages about the private API (see #286) |
This plugin causes problems with the way my app behaves in the background. I have a legitimate background use case too. I'm using the latest version of this plugin by running 'ionic cordova plugin add ionic-cordova-ionic-webview@latest'. I'm only getting version 4.0.1 (latest post suggest 4.1.0) of the plugin and when I look through the CDVWebViewEngine.m file in the plugin I cannot see the code changes that this thread talks about. Please merge in these changes or what else can I try? Thanks. |
4.1.0 is not released, you can test the pre release with |
Okay that's great! Thank you. |
Will this @next version solve the background booting issues? |
It should. Test it and let us know |
I will do. Thanks for your help. I have high hopes! I've been sweating over this issue for days. Testing on a few devices over night, will come back to you tomorrow. |
Hello @jcesarmobile! So I've done some testing. I've been running an Ionic app that does background geolocation on 5 iPhones over the last 24 hours. The background tracking failed to boot in the background in 3/5 devices. Scenario 2: Scenario 3: Implying that devices were unable to boot up the application in the background |
Tested the same scenarios again over night last night. This time: Yesterday afternoon (devices hadn't reloaded the app in around 3 hours): 2/5 devices tracked. |
@jcesarmobile This issue of iOS apps white-screening when booted in the background is so notorious with my users that I've created a special issue label WebView plugin My recommendation for users of my plugins is to "remove any webview plugin" or migrate to React Native or Flutter. A terminated iOS app can be launched in the background automatically by iOS for many reasons, including:
When one uses a Cordova App implementing any of the above APIs, their app is 100% Guaranteed to fail with a white-screen when launched in the background within a day or two, as @obkdev has noted above. My cordova-background-geolocation-lt implements all of these APIs. Let me know if you want me to create for you a simple Hello World app that reproduces this issue for you. |
Sure, a sample app might help. |
@jcesarmobile I appreciate the quick response. I haven't tested this myself in a while. I'll setup a simple hello world and run it on my iPhone X @ @obkdev is one of my users of cordova-background-geolocation-lt: what iOS version(s) are you testing upon? |
@jcesarmobile here's a simple hello-world with background-geolocation. I've just installed this on my iPhone X @ 12.3.1. |
You didn't add the |
Thanks, will do |
@jcesarmobile and I’m going to first continue my initial test without that preference before doing a 2nd test with it. I have not yet seen a white-screen so far. |
@jcesarmobile I guess for me - I'm not using background processes, but the app terminates after 5 minutes not being used. If you also close the app while browsing something in an In-App Browser or SFViewController, and you close that view, it goes back to a white screen. What version am I supposed to be using that enabled this feature to not white-screen? |
@gregavola that's a different issue. Would be good if you could create a new issue with more details, like plugin version, iOS versions where you see that, if using |
@jcesarmobile I'm just looking for the version where
|
that version already have it enabled, it was a prerelease, 2.5.0 is also released now, but it's the same code. |
@jcesarmobile @obkdev I've had the demo app linked above running into the 3rd day without a white-screen (so far without |
@christoceacy any update on your demo app? Interested if I should follow your lead and move to this. |
Still no white-screen in 4 days straight. |
Thanks @christoceacy. Are you using this branch of the code in your test? |
@factionfour I don’t understand your question. I’ve not mentioned anything about “a branch” here. |
sorry @christoceacy - what I meant was are you using this fork, the 2.5.0 version or the 4.1.0 version? |
@factionfour See the package.json of my test app linked above. It's |
@jcesarmobile I've still not experienced a whitescreen with the test app linked above running continuously for 17 days on my iPhone X. I've still not added |
But the preference is there because despite the app being woken up by the plugin, it can't execute any javascript (the app wakes up, but not the webview, or if it's woken up it doesn't execute any code while the app is in background). The sample app only logs messages and it requires to move ~200m to wake it up, so it's hard to tell if the javascript is being executed or not. |
Ok. BackgroundGeolocation exposes methods to write to the plugin’s SQLite log database. I’ll have the js callback write messages to the native logger. |
@jcesarmobile I know this is a long issue, but what should be doing to prevent the app from "suspending" in the background? Basically I get the "white screen" a-lot with In terms of background location, is it not possible to have a background SDK like Pilgrim or a location awarness platform or another SDK work with the webview? |
@gregavola my cordova-background-geolocation-lt works fine tracking location in background with this |
This PR updates the way that the wksuspendinbackground setting is applied so that it is compatible with iOS 12.2+ and with earlier versions.
As with the previous implementation this does not keep your app running in the background abnormally, it just means that javascript processing isn't immediately terminated. If you have a valid background capability enabled (ie audio, gps) and your app is active then it will stay alive unless the OS decides to suspend it as it would any other app running in the background.
This functionality is essential for a number of use cases as if javascript processing just stops the second the app is in the background then many apps and types of completely valid functionality fail:
I have submitted updates to my own apps already to the app store with no issues or errors encountered. If you had the wksuspendinbackground value set in config.xml previously this PR will make it work again as expected.