Uploaded image for project: 'Apache Cordova'
  1. Apache Cordova
  2. CB-11554

too "brutal" app reload when title is empty

Details

    Description

      This patch
      https://github.com/apache/cordova-plugin-wkwebview-engine/commit/815ed0741b9ae30b343d6429bd8ff2ad37ec5790
      introduces

      {if title is empty then reload the app}

      .

      It was really hard for me to find the reason why the app I'm working on kept reloading on each resume, while I had an older build of my app not having this bad feature. Shouldn't there be at least a warning message explaining why the app reloads? It'd be even better to find some better criteria.

      Attachments

        Issue Links

          Activity

            iroh Chris Wells made changes -
            Workflow Classic - editable closed [ 14077141 ] classic default workflow [ 14085770 ]
            iroh Chris Wells made changes -
            Workflow classic default workflow [ 13200299 ] Classic - editable closed [ 14077141 ]
            shazron Shazron Abdullah made changes -
            Status Resolved [ 5 ] Closed [ 6 ]
            githubbot ASF GitHub Bot added a comment -

            Github user alexfoxy commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            I am having an odd issue where my view is reloading but my view remains blank. If I inspect it using safari I can see there is a DOM and I can see the elements being highlighted on the device when I hover over them.

            <img width="698" alt="screen shot 2017-01-16 at 09 16 04" src="https://cloud.githubusercontent.com/assets/151772/21977077/9ba8b34a-dbcc-11e6-8d0d-4015c120e687.png">

            I can reproduce fairly easily. Any suggestions?

            githubbot ASF GitHub Bot added a comment - Github user alexfoxy commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 I am having an odd issue where my view is reloading but my view remains blank. If I inspect it using safari I can see there is a DOM and I can see the elements being highlighted on the device when I hover over them. <img width="698" alt="screen shot 2017-01-16 at 09 16 04" src="https://cloud.githubusercontent.com/assets/151772/21977077/9ba8b34a-dbcc-11e6-8d0d-4015c120e687.png"> I can reproduce fairly easily. Any suggestions?
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user pwbs commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            @mccraigmccraig thank you for your reply. I'll try to find some time for that next month...

            githubbot ASF GitHub Bot added a comment - Github user pwbs commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 @mccraigmccraig thank you for your reply. I'll try to find some time for that next month...
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user mccraigmccraig commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            so that's pretty occasional then, which will make it hard to track down (i haven't seen WSOD at all recently)

            my approach when i first worked on this was to fork this repo and create a build with debug logging wherever i thought it might be useful, wait for the bug to occur and then immediately grab the console logs off of my phone

            can you do that ? put some logging in the `onAppWillEnterForeground` callback etc

            githubbot ASF GitHub Bot added a comment - Github user mccraigmccraig commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 so that's pretty occasional then, which will make it hard to track down (i haven't seen WSOD at all recently) my approach when i first worked on this was to fork this repo and create a build with debug logging wherever i thought it might be useful, wait for the bug to occur and then immediately grab the console logs off of my phone can you do that ? put some logging in the `onAppWillEnterForeground` callback etc
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user pwbs commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            I've just witnessed it again: this time there's no doubt that it's `about:blank`.

            However this time I have some weird messages in Safari's console:
            <img width="1146" alt="screen shot 2016-12-22 at 17 37 59" src="https://cloud.githubusercontent.com/assets/15014298/21432820/764c3e40-c86d-11e6-84c2-3c71bdcbc64b.png">

            githubbot ASF GitHub Bot added a comment - Github user pwbs commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 I've just witnessed it again: this time there's no doubt that it's `about:blank`. However this time I have some weird messages in Safari's console: <img width="1146" alt="screen shot 2016-12-22 at 17 37 59" src="https://cloud.githubusercontent.com/assets/15014298/21432820/764c3e40-c86d-11e6-84c2-3c71bdcbc64b.png">
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user mccraigmccraig commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            if it is `about:blank` that would mean one of

            1. the `onAppWillEnterForeground` callback isn't firing
            2. there is a bug in the `onAppWillEnterForeground` callback https://github.com/apache/cordova-plugin-wkwebview-engine/blob/master/src/ios/CDVWKWebViewEngine.m#L129
            3. the `reload` method doesn't work

            if it's `[1]` or `[3]` i'll be flummoxed. it could be `[2]` but i can't spot anything by visual inspection

            i think some more information is needed somehow

            githubbot ASF GitHub Bot added a comment - Github user mccraigmccraig commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 if it is `about:blank` that would mean one of 1. the `onAppWillEnterForeground` callback isn't firing 2. there is a bug in the `onAppWillEnterForeground` callback https://github.com/apache/cordova-plugin-wkwebview-engine/blob/master/src/ios/CDVWKWebViewEngine.m#L129 3. the `reload` method doesn't work if it's ` [1] ` or ` [3] ` i'll be flummoxed. it could be ` [2] ` but i can't spot anything by visual inspection i think some more information is needed somehow
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user pwbs commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            Since it's a non-inspectable version, I can't be sure that it's actually `about:blank`, although I'd bet it is.

            (I'm using iOS10 and with the inspectable versions I build with Xcode 8.

            {0,1}

            systematically crash when the user wants to upload a file; so it's not convenient at all to use those versions. Hence, the non-inspectable versions are built using Xcode 7; but I can't transfer them to my iOS10 iPhone because iOS10 is not supported on Xcode 7...)

            githubbot ASF GitHub Bot added a comment - Github user pwbs commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 Since it's a non-inspectable version, I can't be sure that it's actually `about:blank`, although I'd bet it is. (I'm using iOS10 and with the inspectable versions I build with Xcode 8. {0,1} systematically crash when the user wants to upload a file; so it's not convenient at all to use those versions. Hence, the non-inspectable versions are built using Xcode 7; but I can't transfer them to my iOS10 iPhone because iOS10 is not supported on Xcode 7...)
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user mccraigmccraig commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            hmm - that would seem to indicate that it is a different bug, or that the WSOD has some other URL than `about:blank` ?

            githubbot ASF GitHub Bot added a comment - Github user mccraigmccraig commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 hmm - that would seem to indicate that it is a different bug, or that the WSOD has some other URL than `about:blank` ?
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user pwbs commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            I've just observed it with a non-inspectable version: it's not recovering after back/foregrounding the app. :-/

            githubbot ASF GitHub Bot added a comment - Github user pwbs commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 I've just observed it with a non-inspectable version: it's not recovering after back/foregrounding the app. :-/
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user mccraigmccraig commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            @pwbs i think i saw this once too - though i have never been able to reproduce it in an inspectable environment

            my first hypothesis would be that the WKWebView content process is dying some time after the `onAppWillEnterForeground` has made it's check, and since there is only a single check WSOD results

            assuming you can reproduce the problem, what happens if, after WSOD, you background then foreground the app ? does it recover ?

            githubbot ASF GitHub Bot added a comment - Github user mccraigmccraig commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 @pwbs i think i saw this once too - though i have never been able to reproduce it in an inspectable environment my first hypothesis would be that the WKWebView content process is dying some time after the `onAppWillEnterForeground` has made it's check, and since there is only a single check WSOD results assuming you can reproduce the problem, what happens if, after WSOD, you background then foreground the app ? does it recover ?
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            githubbot ASF GitHub Bot added a comment -

            Github user pwbs commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            Anyone has an idea why I still encounter `about:blank` white screen of death?
            It's quite hard to reproduce, it took me months before I could observe it with a Safari-inspectable version (so I could be sure it was a `about:blank` and not something else). It's even harder to observe in an Xcode-inspectable environment...

            githubbot ASF GitHub Bot added a comment - Github user pwbs commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 Anyone has an idea why I still encounter `about:blank` white screen of death? It's quite hard to reproduce, it took me months before I could observe it with a Safari-inspectable version (so I could be sure it was a `about:blank` and not something else). It's even harder to observe in an Xcode-inspectable environment...
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]
            shazron Shazron Abdullah made changes -
            Resolution Fixed [ 1 ]
            Status Open [ 1 ] Resolved [ 5 ]

            Commit 88054455c24f0d66e907c1019c7baa96c64ebf5e in cordova-plugin-wkwebview-engine's branch refs/heads/master from shazron
            [ https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-wkwebview-engine.git;h=8805445 ]

            CB-11554 - fixed unit tests

            jira-bot ASF subversion and git services added a comment - Commit 88054455c24f0d66e907c1019c7baa96c64ebf5e in cordova-plugin-wkwebview-engine's branch refs/heads/master from shazron [ https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-wkwebview-engine.git;h=8805445 ] CB-11554 - fixed unit tests
            githubbot ASF GitHub Bot added a comment -

            Github user asfgit closed the pull request at:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]

            Commit a1da5d22c091ed1d30536ccb31f43a107d89ed4c in cordova-plugin-wkwebview-engine's branch refs/heads/master from shazron
            [ https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-wkwebview-engine.git;h=a1da5d2 ]

            CB-11554 - too 'brutal' app reload when title is empty

            This closes #16

            jira-bot ASF subversion and git services added a comment - Commit a1da5d22c091ed1d30536ccb31f43a107d89ed4c in cordova-plugin-wkwebview-engine's branch refs/heads/master from shazron [ https://git-wip-us.apache.org/repos/asf?p=cordova-plugin-wkwebview-engine.git;h=a1da5d2 ] CB-11554 - too 'brutal' app reload when title is empty This closes #16
            githubbot ASF GitHub Bot added a comment -

            Github user mccraigmccraig commented on the issue:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            that all looks reasonable - i'll start testing it

            githubbot ASF GitHub Bot added a comment - Github user mccraigmccraig commented on the issue: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 that all looks reasonable - i'll start testing it
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ] This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]

            Circular load is fine for about:blank since it should only be triggered on foreground enter (thus no crazy infinite loop), and load time is negligible since its blank of course.

            shazron Shazron Abdullah added a comment - Circular load is fine for about:blank since it should only be triggered on foreground enter (thus no crazy infinite loop), and load time is negligible since its blank of course.
            githubbot ASF GitHub Bot added a comment -

            GitHub user shazron opened a pull request:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16

            CB-11554 - too 'brutal' app reload when title is empty

            You can merge this pull request into a Git repository by running:

            $ git pull https://github.com/shazron/cordova-plugin-wkwebview-engine CB-11554

            Alternatively you can review and apply these changes as the patch at:

            https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16.patch

            To close this pull request, make a commit to your master/trunk branch
            with (at least) the following in the commit message:

            This closes #16


            commit 6d39ff1fa0a27aeefdfb1e36af49481f18279b98
            Author: Shazron Abdullah <shazron@apache.org>
            Date: 2016-08-27T01:05:22Z

            CB-11554 - too 'brutal' app reload when title is empty


            githubbot ASF GitHub Bot added a comment - GitHub user shazron opened a pull request: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16 CB-11554 - too 'brutal' app reload when title is empty You can merge this pull request into a Git repository by running: $ git pull https://github.com/shazron/cordova-plugin-wkwebview-engine CB-11554 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-plugin-wkwebview-engine/pull/16.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #16 commit 6d39ff1fa0a27aeefdfb1e36af49481f18279b98 Author: Shazron Abdullah <shazron@apache.org> Date: 2016-08-27T01:05:22Z CB-11554 - too 'brutal' app reload when title is empty
            githubbot ASF GitHub Bot made changes -
            Remote Link This issue links to "GitHub Pull Request #16 (Web Link)" [ 54686 ]

            I will proceed with the fix as I outlined above.

            shazron Shazron Abdullah added a comment - I will proceed with the fix as I outlined above.
            pwbs Philippe Wang added a comment -

            I don't have any new information about this issue.

            pwbs Philippe Wang added a comment - I don't have any new information about this issue.
            shazron Shazron Abdullah added a comment - - edited

            The tests check for "shouldReloadWebView" returning:
            1. YES for an about:blank page
            2. NO for a (non about:blank page) with document.title as empty string
            3. YES for any page where document.title is nil

            Circular loads will need to be detected as well (in case your desired page is already about:blank)

            shazron Shazron Abdullah added a comment - - edited The tests check for "shouldReloadWebView" returning: 1. YES for an about:blank page 2. NO for a (non about:blank page) with document.title as empty string 3. YES for any page where document.title is nil Circular loads will need to be detected as well (in case your desired page is already about:blank)
            shazron Shazron Abdullah added a comment - - edited

            The code needs to be re-factored so it can be testable – namely these lines: https://github.com/apache/cordova-plugin-wkwebview-engine/blob/dce192abdf24d631a37e1ace9fe5b72b44c74ecf/src/ios/CDVWKWebViewEngine.m#L106-L108
            ... should exist in method called "shouldReloadWebView" which returns a BOOL, delete the reloadIfRequired method, and the rest of the code would be:

            - (void) onAppWillEnterForeground:(NSNotification*)notification {
                    if ([self shouldReloadWebView]) {
                         NSLog(@"%@", @"CDVWKWebViewEngine reloading!");
                         [wkWebView reload];
                 }
             }
            
            shazron Shazron Abdullah added a comment - - edited The code needs to be re-factored so it can be testable – namely these lines: https://github.com/apache/cordova-plugin-wkwebview-engine/blob/dce192abdf24d631a37e1ace9fe5b72b44c74ecf/src/ios/CDVWKWebViewEngine.m#L106-L108 ... should exist in method called "shouldReloadWebView" which returns a BOOL, delete the reloadIfRequired method, and the rest of the code would be: - (void) onAppWillEnterForeground:(NSNotification*)notification { if ([self shouldReloadWebView]) { NSLog(@ "%@" , @ "CDVWKWebViewEngine reloading!" ); [wkWebView reload]; } }
            shazron Shazron Abdullah added a comment - - edited

            Two proposed solutions from the discussion in https://github.com/apache/cordova-plugin-wkwebview-engine/pull/11:

            1. Check

              BOOL reload = (title == nil); 

            instead of

             BOOL reload = ((title == nil) || [title isEqualToString:@""]); 

            2. Check if window.location is "about:blank" since that is the white screen of death

            pwbs any more info or research findings to add?

            shazron Shazron Abdullah added a comment - - edited Two proposed solutions from the discussion in https://github.com/apache/cordova-plugin-wkwebview-engine/pull/11: 1. Check BOOL reload = (title == nil); instead of BOOL reload = ((title == nil) || [title isEqualToString:@""]); 2. Check if window.location is "about:blank" since that is the white screen of death pwbs any more info or research findings to add?

            Note: write unit tests that test CB-9888 reload condition and an empty document title (no reload).

            shazron Shazron Abdullah added a comment - Note: write unit tests that test CB-9888 reload condition and an empty document title (no reload).
            shazron Shazron Abdullah made changes -
            Assignee Shazron Abdullah [ shazron ]
            vladimir.kotikov Vladimir Kotikov made changes -
            Labels iOS ios triaged iOS ios regression triaged
            vladimir.kotikov Vladimir Kotikov made changes -
            Remote Link This issue links to "Discussion link (Web Link)" [ 50312 ]
            vladimir.kotikov Vladimir Kotikov made changes -
            Link This issue is broken by CB-9888 [ CB-9888 ]
            vladimir.kotikov Vladimir Kotikov made changes -
            Link This issue is duplicated by CB-9888 [ CB-9888 ]
            vladimir.kotikov Vladimir Kotikov made changes -
            Link This issue is duplicated by CB-9888 [ CB-9888 ]
            vladimir.kotikov Vladimir Kotikov made changes -
            Labels iOS ios triaged
            vladimir.kotikov Vladimir Kotikov made changes -
            Affects Version/s 1.0.4-dev [ 12337944 ]
            vladimir.kotikov Vladimir Kotikov made changes -
            Field Original Value New Value
            Component/s Plugin WKWebViewEngine [ 12327630 ]
            pwbs Philippe Wang created issue -

            People

              shazron Shazron Abdullah
              pwbs Philippe Wang
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: