You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2022/05/17 19:47:36 UTC

[GitHub] [cordova-ios] JH7 opened a new pull request, #1235: Fix #1232: Reload issue resulting in white screen

JH7 opened a new pull request, #1235:
URL: https://github.com/apache/cordova-ios/pull/1235

   <!--
   Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
   
   http://cordova.apache.org/contribute/contribute_guidelines.html
   
   Thanks!
   -->
   
   Fixes #1232
   Fixes meteor/meteor#11811
   
   ### Platforms affected
   
   iOS
   
   ### Motivation and Context
   <!-- Why is this change required? What problem does it solve? -->
   <!-- If it fixes an open issue, please link to the issue here. -->
   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
   <!-- Describe your changes in detail -->
   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
   <!-- Please describe in detail how you tested your changes. -->
   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
   - [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
   - [x] 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](https://help.github.com/articles/closing-issues-using-keywords/))
   - [ ] I've updated the documentation if necessary
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-ios] delki8 commented on pull request #1235: Fix #1232: Reload issue resulting in white screen

Posted by GitBox <gi...@apache.org>.
delki8 commented on PR #1235:
URL: https://github.com/apache/cordova-ios/pull/1235#issuecomment-1156814943

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-ios] addonflare commented on pull request #1235: Fix #1232: Reload issue resulting in white screen

Posted by GitBox <gi...@apache.org>.
addonflare commented on PR #1235:
URL: https://github.com/apache/cordova-ios/pull/1235#issuecomment-1146704121

   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 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-ios] JH7 commented on pull request #1235: Fix #1232: Reload issue resulting in white screen

Posted by GitBox <gi...@apache.org>.
JH7 commented on PR #1235:
URL: https://github.com/apache/cordova-ios/pull/1235#issuecomment-1167382109

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-ios] JH7 closed pull request #1235: Fix #1232: Reload issue resulting in white screen

Posted by GitBox <gi...@apache.org>.
JH7 closed pull request #1235: Fix #1232: Reload issue resulting in white screen
URL: https://github.com/apache/cordova-ios/pull/1235


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org