Bug 40027 – location.href and outgoing referrer not updated properly by pushState/replaceState

• Component: Page Loading
• Status: RESOLVED FIXED

[Darin Fisher (:fishd, Google)]

location.href not updated properly by pushState/replaceState

location.href (FrameLoader::m_url) and FrameLoader::m_outgoingReferrer remain
set to whatever the URL of the original document was.  This seems undesirable.

From http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#dom-location-href:
  "The href attribute must return the current address of the associated Document
   object, as an absolute URL."

From http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#dom-history-pushstate:
  "If the third argument is present, set the document's current address to the
   absolute URL that was found earlier in this algorithm."

This to me implies that location.href should according to the spec be updated
just as the document's URL and documentURI properties are updated as a result
of pushState/replaceState being called with a valid third parameter.

[Darin Fisher (:fishd, Google)]

Firefox nightlies seem to update location.href and send a HTTP-Referer corresponding to the updated URL.

[Darin Fisher (:fishd, Google)]

Chromium bug report:
http://code.google.com/p/chromium/issues/detail?id=45361

[Brady Eidson]

I agree.  We update the "current address of the associated Document object" by... updating the URL of the associated Document*.  Sadly FrameLoader has all sorts of crazy notions of URLs of its own.

We really should unravel all of that and have one authoritative source on what URL is current, and FrameLoader should ask that authoritative source when it needs to know.

I think Document/DocumentLoader is the correct place for that.

Adam, thoughts?

[Darin Fisher (:fishd, Google)]

BTW, the simple fix for this bug is to call FrameLoader::setURL() from Document::updateURLForPushOrReplaceState().  I have a patch for that, but I haven't posted it yet because I haven't finished the tests for it.

[Darin Fisher (:fishd, Google)]

Created attachment 57692 [details]
v1 patch

[Brady Eidson]

Comment on attachment 57692 [details]
v1 patch

I suppose I'm kind of surprised that all the layouttests pass with this change.  That's actually good news for nuking the redundancy in the future.

r+!

[Darin Fisher (:fishd, Google)]

Comment on attachment 57692 [details]
v1 patch

whoops, i left out expected results.

[Brady Eidson]

lol

[Joey Hurst]

Thanks Darin for the quick follow-up on this bug.

I thought of another problem this could cause for developers using pushState.  Assume the following scenario:
1) Visit http://www.foobar.com/
2) http://www.foobar.com/one (via pushState)
3) http://www.foobar.com/two (via pushState)
4) Visit http://www.someothersite.com/
5) Hit the back button twice.

Logically, this should land the user at /one.  However, if the page isn't in the back-forward cache and the user rapidly clicks the back button, it is possible that page script will miss the popstate event from /two to /one, and the user will be stuck looking at state /two when the URL indicates /one.

This could be fixed by inspecting the location object at some point and updating the page state to match, but as Darin pointed out, the location object isn't reliable.  I haven't been able to find an alternative way to grab the actual location in this scenario.

Once could try placing a popstate handler near the top of the document and hoping for the best, but the error case is rather bad and I can't think of a way that one could track the issue (since the problem stems from the developer missing an event).

It looks like you've already started on a fix, so this comment is intended more for other web developers trying to use pushState in Webkit...

Thanks!

[Darin Fisher (:fishd, Google)]

Created attachment 57715 [details]
v2 patch

Now with test results, more tests, and a fix for an existing test :-/

[Darin Fisher (:fishd, Google)]

(In reply to comment #9)

Hi Joey,

The patch in this bug makes the location update properly.  If you are trying to workaround the current behavior of WebKit, please try looking at document.URL instead.  This patch just makes location get updated when we update document.URL.

I don't understand the issue with clicking back rapidly.  We should always dispatch popstate event for the final back navigation.

[Brady Eidson]

Comment on attachment 57715 [details]
v2 patch

This one makes me sadder, but is more what I was expecting.

[WebKit Commit Bot]

Comment on attachment 57715 [details]
v2 patch

Clearing flags on attachment: 57715

Committed r60608: <http://trac.webkit.org/changeset/60608>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brady Eidson]

<rdar://problem/8330168>