618644 - onpopstate after onload should fire with event state of null even if replaceState is fired

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[henryfhchan]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.215 Safari/534.10
Build Identifier: 

A webapp alters all its links to use ajax to load the page.  To ease url
sharing, it uses popstate.  To maintain backwards compatibility, the same pages
are served by pasting in the popstate url.

Consider Example [1]: fetchAjax ignores NULL
1. A user goes to index.html
2. DOM ready fires and the links are assigned an onclick handler "loadLink",
calls pushState with a state object of the new url, and calls "fetchAjax" to
fetch the page using ajax.
3. "fetchAjax" is registered to onpopstate so that it will fetch the ajax page
on a history traversal.
4. The onload event fires.
5. The onpopstate event fires.
6. "fetchAjax" does not fetch any page as the event state is NULL.
7. The user clicks on a link page1.html.
8. The "loadLink" function is called, triggering a pushState and then
fetchAjax.
9. The user clicks on a link page2.html.
10.The "loadLink" function is called, triggering a pushState and then
fetchAjax.
11.The user clicks back on his browser.
12.The opnpopstate even fires, with a event state of page1.html.
13.The fetchAjax fetches page1.html.
14.The user clicks back on his browser again.
PROBLEM:
15.The onpopstate even fires, with a event state of NULL.
16.index.html is not fetched, but the url is now 'index.html'
CAUSE: The handler cannot distinguish between first page load and back and
forth page load as both have the event state of NULL.

Consider Example[2]:
1. A user goes to index.html
2. DOM ready fires and the links are assigned an onclick handler "loadLink",
calls pushState with a state object of the new url, and calls "fetchAjax" to
fetch the page using ajax.
3. "fetchAjax" is registered to onpopstate so that it will fetch the ajax page
on a history traversal.
4. The user clicks a link to page1.html before onload is fired.
5. The onload event fires.
6. The onpopstate event fires.
PROBLEM:
7. index.html is refetched AND the URL stays on page1.html.
8. Going back refetches index.html.
9. Going forward refetches page1.html.

Workaround [1]: Use Example 1, but fetch location.href when event state is
null.
PROBLEM: An ugly flash of content and reloading of images because onpopstate is
called after onload.

Workaround [2]: Use Workaround[1], but add a variable on onload that causes the
first onpopstate to return instead of fetching page.
PROBLEM: What if user aborts the page after dom ready fires but before onload?

Workaround [3]: Use Workaround[1], but add a variable on document parsing
(wrong term?) that causes the first onpopstate to return instead of fetching
page.
PROBLEM: Extra variables and overhead.

Workaround [3]: Fire replaceState on first onpopstate / onload.
PROBLEM[a]: If a user clicks a link page1.html, a pushState is fired.  When
onpopstate / onload fires, the history becomes [index.html, index.html].
index.html is refetched. page1.html disappears totally from the history.
PROBLEM[b]: If a user navigates to page1.html then page2.html, then
onpopstate/onload fires, the history becomes [index.html, page1.html,
index.html].

Workaround [4]: Fire replaceState during document parsing. (wrong term?)
This works in Google Chrome.  window.onpopstate calls with a null event state.
Navigating to a new page and then clicking back will fire event with new event
state.
PROBLEM: Starting with firefox 4b8pre (some build a few days ago), replaceState
caused the window.onload event to fire with the replaced event state.  Causes
same effect as Example 2.

Workaround [5]:
Remember the time for onpopstate and the time for onload.  If the difference is
less than 200 milliseconds abort the onpopstate.
PROBLEM: unreliable.


Solution:
  Fire onpopstate after onload with a null event state, regardless of whatever
replaceState.  And allow replaceState to call before document finishes loading.
Collapse All Comments


Reproducible: Always

Comment 1

[Justin Lebar (not reading bugmail)]

-- Example [1] --

My understanding here is that if:

1. The page starts at a.html, then
2. the page pushState()s to b.html, then
3. the user clicks <back>

the page will get a popState with a null state argument.  This is correct behavior.  If you want to get a different state when you go back to the first page, you'd need to call replaceState().  Put another way, surely if you put a call to replaceState() between (1) and (2) above and then ran the steps in full, you'd want the state argument to reflect the call to replaceState().

-- Example [2] --

> 1. A user goes to index.html
> 2. DOM ready fires and the links are assigned an onclick handler "loadLink",
> calls pushState with a state object of the new url, and calls "fetchAjax" to
> fetch the page using ajax.
> 3. "fetchAjax" is registered to onpopstate so that it will fetch the ajax
> page on a history traversal.
> 4. The user clicks a link to page1.html before onload is fired.
> 5. The onload event fires.
> 6. The onpopstate event fires.
> PROBLEM:
> 7. index.html is refetched AND the URL stays on page1.html.
> 8. Going back refetches index.html.
> 9. Going forward refetches page1.html.

It sounds like your onpopstate handler shouldn't do anything until the second popstate.  This is your workaround [3].  I agree that it's not so pretty, but I don't understand why it's insufficient.

-- Workaround [4] --
> Fire replaceState during document parsing. (wrong term?)
>
> This works in Google Chrome.  window.onpopstate calls with a null event state.
> Navigating to a new page and then clicking back will fire event with new event
> state.
>
> PROBLEM: Starting with firefox 4b8pre (some build a few days ago), replaceState
> caused the window.onload event [jlebar: do you mean onpopstate event?] to
> fire with the replaced event state.  Causes same effect as Example 2.

If I understand correctly, what Firefox does is actually the currently-specified behavior.  This is step 5 in the algorithm for pushState() at [html5].

If you still think Firefox has a bug here, a concrete test case (in the form of an HTML document attached to the bug) would be helpful.  If you think that the spec should be changed, you could try e-mailing the WhatWG list.  But at this point, push/replaceState has seen somewhat wide adoption, so I don't think Hixie will be keen on changing it in significant ways.

In the meantime, I'm going to mark this as INVALID.  Please feel free to re-open.

[html5] http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#dom-history-pushstate

Comment 2

[henryfhchan]

> It sounds like your onpopstate handler shouldn't do anything until the second
> popstate.  This is your workaround [3].  I agree that it's not so pretty, but I
> don't understand why it's insufficient.

It is sufficient, unless suddenly Hixie realizes the problem and kills the first onpopstate.  Or the user cancels the navigation.  And then the first (and unnecessary) onpopstate doesn't fire.

> I agree that it's not so pretty

It's absolutely ugly.


>If I understand correctly, what Firefox does is actually the
> currently-specified behavior.  This is step 5 in the algorithm for pushState()
> at [html5].

[Quote HTML5]
>> If the current document readiness is not yet set to the string "complete", 
>> let the Document's pending state object be another structured clone of the 
>> specified data. (If there was already a pending state object, the previous 
>> one is discarded.)

>> This ensures that the popstate event that will be fired when the document 
>> finally loads will accurately reflect the pushed or replaced state object.

**ARGH** Hixie's making things harder and harder.
When the simpliest thing to do was to discard the whole onpopstate after onload.
Who needs the first onpopstate after onload?
And now he's making it even harder to detect it.

Shouldn't we just break it like we did with tabindex a few years ago?

Comment 3

[henryfhchan]

> the page will get a popState with a null state argument.  This is correct
> behavior.  If you want to get a different state when you go back to the first
> page, you'd need to call replaceState().  Put another way, surely if you put a
> call to replaceState() between (1) and (2) above and then ran the steps in
> full, you'd want the state argument to reflect the call to replaceState().

1. USER: /
2. script: replacestate ("index")
3. USER: page1.html
4. script: pushstate("page1")
5. ONLOAD + ONPOPSTATE: event.state is index
6. script: load index.html
Outcome:
url is page1.html.  The content is index.html
The user is bemused. (and the developer perplexed.)

> you'd want the state argument to reflect the call to replaceState().
No I don't.  You really think so?

> a concrete test case (in the form of an HTML document attached to the bug) 
> would be helpful.
To make enough time for a user to click on something before onload fires requires a large image which either
- needs a slow server (and my one which exhibits this behavior doesn't allow hotlinking) or
- needs a really large file (so large that no-one would host it for me)

you can check out www.wyavtv.org which exhibits this behavior.

Happy debugging for hixie :)

Comment 4

[henryfhchan]

sorry the link should be www.wyavtv.org/index_gray.php

Comment 5

[henryfhchan]

> the page will get a popState with a null state argument.  This is correct
> behavior.  If you want to get a different state when you go back to the first
> page, you'd need to call replaceState().  Put another way, surely if you put a
> call to replaceState() between (1) and (2) above and then ran the steps in
> full, you'd want the state argument to reflect the call to replaceState().

sorry now i get what you mean.  Yes of course. I do want that.

But I don't want the onpopstate after onload to fire with the new replaced state.
Or else how to i discard the useless initial popstate?

Comment 6

[Boris Zbarsky [:bzbarsky]]

> needs a slow server

http://landfill.mozilla.org/ryl/slowResponse.cgi?time=N where N is a number will return a text/plain response (but you don't care; you just care that it's slow) after a sleep(N), in case that helps.

Comment 7

[Justin Lebar (not reading bugmail)]

I think you have a point about being unable to reliably distinguish the first popstate after a load from the following:

* page begins loading
* user cancels before onload
* user triggers pushstate, then clicks back, triggering popstate

Can you write an e-mail to WhatWG?

Comment 8

[henryfhchan]

http://www.wyavtv.org/test-popstate.html

Try clicking on page links (except back/forward/reload test) before page load.
When page load + onpopstate fires, the page you were viewing suddenly becomes index.html.

(click reload test)

Try clicking on page links then back and forward.
The inital onpopstate doesn't fire (the one supposed to fire after onload)
The onpopstate when clicking back and forward before onload don't fire either.
(Thats why i said discard first onpopstate doesn't work)

(click reload test)

Wait for 8 seconds to load.
Try clicking on page links then back and forward.
(It works, but the initial popstate re-fetches index.html (you can see the change from test-popstate.html turns into index.html))

after reading html5 spec it suddenly came to my mind that onpopstate after onload is necessary if you don't specify a url in pushState.

What I propose now is that onpopstate should always fire with the event state at the start of the navigation, i.e. remove and revert the "pending state object".

popstate should still fire even when the document has not loaded and the user navigates. (it should be asynchronous)

and strange that the inital popstate doesn't fire if you clicked back and forward before onload.  bug?

Comment 9

[henryfhchan]

i thought http://www.w3.org/Bugs/Public/show_bug.cgi?id=11468 was enough >.<

Comment 10

[henryfhchan]

http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2010-December/029476.html
um, how do i reply to a thread in whatwg mailing list?

Comment 11

[Justin Lebar (not reading bugmail)]

Sure, an entry in the WhatWG bug tracker is sufficient.  If you want to reply to a thread on the mailing list, just send an e-mail to the list with the same subject.

> What I propose now is that onpopstate should always fire with the event state
> at the start of the navigation, i.e. remove and revert the "pending state
> object".

I don't understand what you mean.

Comment 12

[henryfhchan]

onpopstate should: (propose)
1. fire as early as possible, domready?
2. fire with event state at the start of the navigation algorithm
3. stop having pending state object
4. fire asynchronously, i.e. fires when the user clicks back/forward before document fully loads.

Comment 13

[henryfhchan]

since onpopstate stuff doesn't work until onload, then it's going to make the browser (and user) in a very confused state before it.

do u reckon the spec would be changed before fx4 ships?

because if fx4 ships with it and whatwg realizes the problem and the spec changes, it's going to cause a lot of problems.

plus the fact that if we navigate to page1.html before onload, when onload fires the page content becomes index.html but the url is still page1.html. This might be a security problem (Phishing?).  It's unlikely because same origin should have kicked in.  But still might present some problems, especially with ajax post forms.

are we going to drop it like websockets?

Comment 14

[Justin Lebar (not reading bugmail)]

(In reply to comment 13)
> are we going to drop it like websockets?

No, I don't think so.  The websockets problem was a major security vulnerability.  At worst, this limits the usefulness of the API and makes developers' lives harder.  And anyway, websites are already relying on this API.

Comment 15

[henryfhchan]

*looks like i've been ignored on the mailing lists and the bug tracker*
can someone help me relay the importance of fixing those issues to hixie for me?
Having onpopstate not work before onload is really damn bad.

And the fact that onpopstate doesn't fire after onload if you navigate and click back/forward, is that a bug?
Onpopstate still fires after onload if you just navigate but don't click back/forward.

Comment 16

[Justin Lebar (not reading bugmail)]

(In reply to comment #15)
> *looks like i've been ignored on the mailing lists and the bug tracker*

Please be patient.  Hixie has taken months to reply to some of my e-mails.

Comment 17

[henryfhchan]

onpopstate doesn't fire after onload if you navigate and
click back/forward before onload, is that another bug?

Comment 18

[Hixie (not reading bugmail)]

Is this bug moot once the changes described in http://hacks.mozilla.org/2011/03/history-api-changes-in-firefox-4/ are taken into account?

Comment 19

[Hixie (not reading bugmail)]

I think this bug is now invalid due to the recent changes that applied the above blg post's changes to the spec.

Comment 20

[Justin Lebar (not reading bugmail)]

(In reply to comment #19)
> I think this bug is now invalid due to the recent changes that applied the
> above blg post's changes to the spec.

Agreed.

Comment 21

[henryfhchan]

Actually, the spec changes just address all of the problems stated above.  Unfortunately no one ever noticed I posted this earlier than the bug that led to the spec change. *sigh*.  fixed, not invalid.