Bug 156356 – pageshow only fires the first time the back button is pressed

• Component: DOM
• Status: RESOLVED FIXED

[Jeff Carlsen]

Repro steps:

1. Go to page A
2. Click a link to page B
3. Click the back button (pageshow fires as expected)
4. Click a link to page B again (or any other page)
5. Click the back button (pageshow does not fire)

I discovered this because on the page I'm working on, I want to fetch some fresh data when they return to a page through the back button. I do this by listening for pageshow and checking the event.persisted property.

[Alexey Proskuryakov]

Could you please provide a test case, or a link to a test page? I'm pretty sure that I tested this some time before, and it worked as expected, so it may be tricky to reproduce from a description alone.

[Paulo Pereira]

Hi, I don't know if it's the exact same situation that is happening to Jeff Carlsen. But I am having the exact same behavior and I noticed this is also happening to "pagehide" event.

After some heavy debugging I figured out why this is happening and I created this test case example reproducing it: 

1. Go to http://paulovpereira.github.io/pageshow-webkit/ in an IOS device (I tested in ios 9 and 10)
2. Click in the navigate link
3. press back button
4. press forward button
5. keep going back and forth and check the events being triggered and logged in the body

The only difference between the pages is the existence of an iframe element and this is causing the event not to be fired. I hope this help figuring out how to fix the bug.

[Georgii Ivankin]

Second this. Just having 2 pages, both with 

window.addEventListener("pageshow", function(event) {
    console.info('pageshow. persisted: ' + event.persisted);
});

and navigating back-forward between them, is enough to reproduce. 

People has been reporting this on SO as well: http://stackoverflow.com/q/10106457/697388 
http://stackoverflow.com/q/33804292/697388
http://stackoverflow.com/questions/11979156/mobile-safari-back-button#comment44257718_12652160


Version 10.0.1 (12602.2.14.0.7) on MacOS Sierra.

[Radar WebKit Bug Importer]

<rdar://problem/29256489>

[Lucas Forschler]

Mass move bugs into the DOM component.

[Hansol Kim]

Hi, I just stumbled upon this issue after struggling hours trying to find a possible workaround but could find none so far. After discovering there's already a bug filed for this issue, I'm surprised nothing happened even after 4 years now... are there any updates regarding this or plans to fix?

[Chris Dumez]

(In reply to Hansol Kim from comment #6)
> Hi, I just stumbled upon this issue after struggling hours trying to find a
> possible workaround but could find none so far. After discovering there's
> already a bug filed for this issue, I'm surprised nothing happened even
> after 4 years now... are there any updates regarding this or plans to fix?

Thanks for bringing this to our attention. It looks like this bug fell through the cracks. The bug indeed still reproduces in the latest WebKit and I am looking into it.

[Chris Dumez]

Created attachment 400366 [details]
Patch

[Chris Dumez]

Created attachment 400372 [details]
Patch

[Chris Dumez]

Comment on attachment 400372 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400372&action=review

> LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt:38
> +http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didFinishLoading

This is a progression, it makes back/forward navigations more consistent with regular navigations. For e.g., if you look at LayoutTests/http/tests/loading/redirect-methods-expected.txt:
"""
main frame - didFinishLoadForFrame
http://127.0.0.1:8000/loading/redirect-methods.html - didFinishLoading
"""

We normally call didFinishLoadForFrame for the main frame *before* calling * didFinishLoading*, which makes more sense.

[Alexey Proskuryakov]

Comment on attachment 400372 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400372&action=review

> LayoutTests/ChangeLog:13
> +        * fast/history/resources/page-cache-helper.html:

Comment?

>> LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt:38
>> +http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didFinishLoading
> 
> This is a progression, it makes back/forward navigations more consistent with regular navigations. For e.g., if you look at LayoutTests/http/tests/loading/redirect-methods-expected.txt:
> """
> main frame - didFinishLoadForFrame
> http://127.0.0.1:8000/loading/redirect-methods.html - didFinishLoading
> """
> 
> We normally call didFinishLoadForFrame for the main frame *before* calling * didFinishLoading*, which makes more sense.

Progressions of this kind are scary, as clients rely on quirks like this. But I don't know what kind of testing we could to qualify the fix do besides automated tests.

[Chris Dumez]

Created attachment 400377 [details]
Patch

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #11)
> Comment on attachment 400372 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=400372&action=review
> 
> > LayoutTests/ChangeLog:13
> > +        * fast/history/resources/page-cache-helper.html:
> 
> Comment?

I added an explanation.

> 
> >> LayoutTests/http/tests/loading/main-resource-delegates-on-back-navigation-expected.txt:38
> >> +http://127.0.0.1:8000/loading/main-resource-delegates-on-back-navigation.html - didFinishLoading
> > 
> > This is a progression, it makes back/forward navigations more consistent with regular navigations. For e.g., if you look at LayoutTests/http/tests/loading/redirect-methods-expected.txt:
> > """
> > main frame - didFinishLoadForFrame
> > http://127.0.0.1:8000/loading/redirect-methods.html - didFinishLoading
> > """
> > 
> > We normally call didFinishLoadForFrame for the main frame *before* calling * didFinishLoading*, which makes more sense.
> 
> Progressions of this kind are scary, as clients rely on quirks like this.
> But I don't know what kind of testing we could to qualify the fix do besides
> automated tests.

[Chris Dumez]

Created attachment 400383 [details]
Patch

[Chris Dumez]

Resetting review flag since I had to make a small change to make API test happy. This change is more conservative, we call checkCompleted() for subframes in FrameLoader::open(). For the main frame, we keep calling checkCompleted() where we used to to avoid behavior changes.

[EWS]

Committed r262221: <https://trac.webkit.org/changeset/262221>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400383 [details].