[webkit-reviews] review granted: [Bug 33224] Chains of history items representing same-document navigation need to always remember that association : [Attachment 47388] v1 patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has granted Darin Fisher (:fishd, Google)
<fishd at chromium.org>'s request for review:
Bug 33224: Chains of history items representing same-document navigation need
to always remember that association
https://bugs.webkit.org/show_bug.cgi?id=33224

Attachment 47388: v1 patch
https://bugs.webkit.org/attachment.cgi?id=47388&action=review

------- Additional Comments from Brady Eidson <beidson at apple.com>
In reviewing this, I recalled a certain detail of the spec and dug in to it to
remind myself.	The spec states:  
"If the specified entry is a state object or the first entry for a Document,
the user agent must activate that entry."

I wonder if the document-destroyed-navigate-back test actually covers this. 
For example, if the sequence of entries for a document was:

1 - www.example.com
2 - www.example.com#someHash
3 - www.example.com#someHash (with a state object, via pushState())

Then going back from #3 to #2 should not generate a popstate because entry #2
is not a state object entry, and is also not the first entry for the document.
Going back from #2 to #1 *should* generate a popstate event because entry #1 is
the first entry, even though it is not a state object entry.

Note that this is unrelated to your patch, and I don't think your patch changes
WebKit's behavior whether we had it right or wrong.

I'm just curious, and thought I'd mention it.

Your patch looks fine. r+