Bug 42940 - popstate events are lost when network connection is in progress
: popstate events are lost when network connection is in progress
Status: RESOLVED FIXED
: WebKit
History
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
: http://www.bcherry.net/playground/pus...
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2010-07-24 18:28 PST by
Modified: 2010-11-19 00:07 PST (History)


Attachments
Patch (6.68 KB, patch)
2010-10-08 11:58 PST, Mihai Parparita
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-07-24 18:28:38 PST
User history navigation (back/forward buttons) while there is network traffic does not always fire a popstate event.  These events are completely lost, and can get an application into the wrong state, where the user navigated through their history, but the app was never notified.  Seen in Chrome 5 and 6 and Safari 5 on OS X 10.6.  Mozilla's Firefox 4 implementation of popstate does not show this behavior.

Reproduced here by downloading an image for 1s in the popstate handler, which prevents further popstates from firing while the image downloads: http://www.bcherry.net/playground/pushstate
------- Comment #1 From 2010-07-28 09:53:10 PST -------
<rdar://problem/8245251>
------- Comment #2 From 2010-09-28 18:51:41 PST -------
We're experiencing a similar error at Facebook with our implementation of HTML5 history. We have a workaround that verifies that the current location actually matches the most recently fired popstate event, but that's an ugly hack and we'd greatly prefer that multiple presses of "Back" actually generated the correct number of popstate events.
------- Comment #3 From 2010-10-08 09:22:24 PST -------
Looking into this.
------- Comment #4 From 2010-10-08 11:58:36 PST -------
Created an attachment (id=70273) [details]
Patch
------- Comment #5 From 2010-10-08 12:01:16 PST -------
Darin or Brady, could you take a look at this?
------- Comment #6 From 2010-10-08 12:23:35 PST -------
(From update of attachment 70273 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=70273&action=review

> WebCore/loader/FrameLoader.cpp:661
> +    m_frame->document()->setReadyState(Document::Loading);

Is this change detectable by reading document.readyState?  If so, we should add a test that shows the change.
------- Comment #7 From 2010-10-08 12:39:35 PST -------
(In reply to comment #6)
> (From update of attachment 70273 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70273&action=review
> 
> > WebCore/loader/FrameLoader.cpp:661
> > +    m_frame->document()->setReadyState(Document::Loading);
> 
> Is this change detectable by reading document.readyState?  If so, we should add a test that shows the change.

Without this change a couple of tests in fast/loader/stateobjects fail (document-destroyed-navigate-back-with-fragment-scroll.html and document-destroyed-navigate-back.html) since they navigate back to a page that had a state object attached to its history entry. I don't think it's otherwise directly testable from layout tests though, since no script runs between FrameLoader::didBeginDocument and Document::implicitOpen.
------- Comment #8 From 2010-10-08 12:44:21 PST -------
(From update of attachment 70273 [details])
Okiedokes.  This looks good to me, but it wouldn't hurt to have a pushState/popState expert look it over.
------- Comment #9 From 2010-10-08 16:35:20 PST -------
(From update of attachment 70273 [details])
LGTM
------- Comment #10 From 2010-10-08 17:02:09 PST -------
(From update of attachment 70273 [details])
Clearing flags on attachment: 70273

Committed r69432: <http://trac.webkit.org/changeset/69432>
------- Comment #11 From 2010-10-08 17:02:15 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #12 From 2010-10-11 14:17:00 PST -------
Confirmed fixed for FB, thanks Mihai.
------- Comment #13 From 2010-10-11 16:19:42 PST -------
Fixed on my test case as well.  Thanks to everyone involved!