-
Notifications
You must be signed in to change notification settings - Fork 986
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
CB-14045 - reinit url after app freezes on 4.5.x #418
Conversation
* test reinit url * refactor: split function + check if appUrl is not empty * Added tests.
Codecov Report
@@ Coverage Diff @@
## 4.5.x #418 +/- ##
=======================================
Coverage 63.45% 63.45%
=======================================
Files 14 14
Lines 1691 1691
Branches 284 284
=======================================
Hits 1073 1073
Misses 618 618 Continue to review full report at Codecov.
|
I have no idea what this PR is actually doing without following the links. Does this cherry pick a commit from somewhere else for this release branch? If so, please say so in the PR description. The PR title should also be about what the commit fixes, not what issue is merged into what. Include that as additional information in the PR description please. |
@janpio I just updated the title and description. Does it look OK now? |
yep, now I can follow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK.
Please somebody merge it |
Does this commit work with CodePush using dynamic url? |
I suggest you try cordova-ios@nightly with CodePush. |
Now merged onto 4.5.x, in case we ever publish 4.5.6 patch release before Cordova 9. |
I think it'll be fine, unless it gets into the frozen state, in which case it will reload with the original URL instead of the dynamic CodePush one. I would be inclined to consider that a big enough change in behaviour that it's not a patch-level release anymore |
Do you mean that that the change is a "big enough change in behaviour" or something else is a "big enough change in behaviour"? |
I think this change to the URL behaviour is a change that will likely affect 3rd party plugins (CodePush in particular), even if only in certain edge cases |
I will revert this change in 4.5.x branch, then. For anyone who needs this functionality before we release Cordova 9, I recommend using cordova-ios@nightly. |
Thank @dpogue. Do you suggest a different approach to this fix in Cordova 9? |
As discussed in PR #418, we decided the change was too drastic for a patch release.
I tried using this fix but it still freezes after a week of being in the background. Its hard to debug as I don't know how to do crash reporting on a build from XCode. I'm not sure how to replicate this as a week debug cycle is hard. |
Step 1: Create an issue instead of commenting here, this will get ignored and forgotten. |
Fix for CB-14045 (reinit url after app freezes) was mistakenly raised on 4.5.x in PR #363, merged on 4.5.x, reverted, then merged on
master
.I think this fix should have also been part of 4.5.x.
I am raising this PR just in case we do another cordova-ios@4.5.x patch release.
The commit ID on
master
is: 6e790af