Bug 33160 - history.pushState should clear the entire forward history
Summary: history.pushState should clear the entire forward history
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-04 10:52 PST by Darin Fisher (:fishd, Google)
Modified: 2010-01-15 00:20 PST (History)
3 users (show)

See Also:


Attachments
v1 patch (5.43 KB, patch)
2010-01-14 14:44 PST, Darin Fisher (:fishd, Google)
beidson: review-
Details | Formatted Diff | Diff
v2 patch: insertItemAfterCurrent removed (5.20 KB, patch)
2010-01-14 23:55 PST, Darin Fisher (:fishd, Google)
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2010-01-04 10:52:24 PST
history.pushState should clear the entire forward history

see http://www.w3.org/Bugs/Public/show_bug.cgi?id=8629

i believe it would be better if history.pushState cleared
the entire forward history regardless of which documents
are involved.  this way the forward browser button will
become disabled in response to pushState, which seems
desirable to allow pushState to be used to simulate a
page load.  (reference fragment navigations clear the
entire forward session history.)

also, firefox has implemented pushState by clearing the
entire forward history.  when i mentioned this on the whatwg
mailing list, they were surprised to find that the feature
was spec'd differently, and Jonas said that he preferred how
they had implemented pushState.

i think webkit should change to match firefox.
Comment 1 Brady Eidson 2010-01-07 15:47:49 PST
With no one having shipped an implementation and therefore compatibility not yet being an issue, I would personally r- any patch that changes our behavior without the spec being changed.

Why did mozilla disregard the spec for their implementation (or did their implementation predate the spec)?

In your w3.org bug, you state "There is also support in the WebKit
community for changing WebKit to match Firefox instead of what
is currently spec'd."

I'm curious - I don't feel strongly about this one way or the other - but what support in the WebKit community are you referencing?  Besides this bugzilla, I haven't heard about this yet.
Comment 2 Darin Fisher (:fishd, Google) 2010-01-12 09:44:13 PST
> I'm curious - I don't feel strongly about this one way or the other - but what
> support in the WebKit community are you referencing?  Besides this bugzilla, I
> haven't heard about this yet.

Maciej gave his support for this change.  (Sorry that doesn't necessarily equate to WebKit community support.)  I think that might have been private email :(
Comment 3 Darin Fisher (:fishd, Google) 2010-01-12 09:45:59 PST
Also, I should have included this link to the whatwg discussion on this topic:
http://www.mail-archive.com/whatwg@lists.whatwg.org/msg19615.html
Comment 4 Brady Eidson 2010-01-12 10:54:31 PST
This is an easy change and it makes sense to align with "the other implementor" on this.

That said, we really need to keep lobbying on the spec...
Comment 5 Darin Fisher (:fishd, Google) 2010-01-12 11:11:39 PST
> That said, we really need to keep lobbying on the spec...

I've already spoken with Ian Hickson, and he is also supportive of changing the spec.  I believe that changing our implementation to match Firefox will help support the case of having the spec corrected.
Comment 6 Darin Fisher (:fishd, Google) 2010-01-14 14:44:39 PST
Created attachment 46608 [details]
v1 patch
Comment 7 Ian 'Hixie' Hickson 2010-01-14 22:36:47 PST
Spec is updated.
Comment 8 Brady Eidson 2010-01-14 22:45:07 PST
Comment on attachment 46608 [details]
v1 patch

The indirection of insertItemAfterCurrent() is no longer necessary.

The only reason the private insertItemAfterCurrent() was added in the first place was to differentiate between a pushState() and an addItem().  Now that the distinction is removed, I'd rather see the body of insertItemAfterCurrent() moved back into addItem(), and have pushState() call addItem().

(Otherwise, looks good)
Comment 9 Darin Fisher (:fishd, Google) 2010-01-14 23:45:52 PST
> The indirection of insertItemAfterCurrent() is no longer necessary.

Yeah, sounds good.
Comment 10 Darin Fisher (:fishd, Google) 2010-01-14 23:55:07 PST
Created attachment 46646 [details]
v2 patch: insertItemAfterCurrent removed
Comment 11 Darin Fisher (:fishd, Google) 2010-01-15 00:20:55 PST
Landed as http://trac.webkit.org/changeset/53324