Bug 21605

Summary: Support for HTML5 "hashchange" event
Product: WebKit Reporter: Tuom Larsen <tuom.larsen>
Component: Page LoadingAssignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Enhancement CC: arv, beidson, info, kangax, kuvik3, leavengood, me, mike, pmuellr, scottt.tw, techrazy.yang
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Fix + Layout Test darin: review+

Description Tuom Larsen 2008-10-14 16:38:27 PDT
Please consider adding support for observing .hash change in Location object.

More info:
http://www.whatwg.org/specs/web-apps/current-work/#history-traversal (6th bullet)
http://ejohn.org/blog/javascript-in-internet-explorer-8/ (HTML 5: window.location.hash)
Comment 1 Julien Chaffraix 2008-11-23 16:42:49 PST
Confirming the bug and moving its severity to enhancement.
Comment 2 NovakP 2009-07-03 07:53:21 PDT
Firefox has added support for "hashchange" event in latest nightly

https://bugzilla.mozilla.org/show_bug.cgi?id=385434

Is this going to be implemented soon in WebKit?
Comment 3 Ryan Leavengood 2009-07-17 00:22:40 PDT
I am interested in implementing this. I'll probably need to make use of the tests from the Firefox implementation and/or play with IE8 a bit since there seems to be some trickiness.

I expect the WebKit code part of it should not be too hard.
Comment 4 Brady Eidson 2009-08-06 22:24:42 PDT
This is in radar as part of <rdar://problem/5838863>

Assigning to myself as I have a patch for this.

It implements the behavior as specc'ed.  Firefox doesn't seem to have any surprises the spec didn't cover.   I can't test IE8 but the event itself wasn't too crazy complex.
Comment 5 Brady Eidson 2009-08-06 22:31:04 PDT
Created attachment 34251 [details]
Fix + Layout Test
Comment 6 Darin Adler 2009-08-07 07:07:41 PDT
Comment on attachment 34251 [details]
Fix + Layout Test

> +    HashChangeEventTask(PassRefPtr<Document> document)
> +    : m_document(document)
> +    {
> +        ASSERT(m_document);
> +    }

We normally indent that one tab stop.

> +    if (equalIgnoringRef(url, m_URL) && !equalIgnoringNullity(url.ref(), m_URL.ref())) {
> +        Document* currentDocument = frame()->document();
> +        currentDocument->postTask(HashChangeEventTask::create(currentDocument));
> +    }

If equalIgnoringRef(url, m_URL) ever false here?

r=me
Comment 7 Brady Eidson 2009-08-07 08:37:45 PDT
Fixed the style.  

I *think* the URLs should always be equalIgnoringRef().  I added an ASSERT then checked in, not thinking it through because it's too early and I haven't had enough coffee yet.

I'm running the layout tests again post-ASSERT and hopefully it won't be firing all the time for some case we never thought of.  If it's a problem I'll remove it.

Sending        LayoutTests/ChangeLog
Adding         LayoutTests/fast/loader/hashchange-event-expected.txt
Adding         LayoutTests/fast/loader/hashchange-event.html
Sending        WebCore/ChangeLog
Sending        WebCore/dom/EventNames.h
Sending        WebCore/html/HTMLAttributeNames.in
Sending        WebCore/html/HTMLBodyElement.cpp
Sending        WebCore/html/HTMLFrameSetElement.cpp
Sending        WebCore/loader/FrameLoader.cpp
Sending        WebCore/platform/text/PlatformString.h
Sending        WebCore/platform/text/StringImpl.cpp
Sending        WebCore/platform/text/StringImpl.h
Transmitting file data ............
Committed revision 46892.