Bug 21605 - Support for HTML5 "hashchange" event
Summary: Support for HTML5 "hashchange" event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2008-10-14 16:38 PDT by Tuom Larsen
Modified: 2009-08-07 08:37 PDT (History)
11 users (show)

See Also:


Attachments
Fix + Layout Test (14.22 KB, patch)
2009-08-06 22:31 PDT, Brady Eidson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.