RESOLVED FIXED 28260
onhashchange property cannot be set from javascript 
https://bugs.webkit.org/show_bug.cgi?id=28260
Summary onhashchange property cannot be set from javascript
Brady Eidson
Reported 2009-08-13 09:22:34 PDT
onhashchange property cannot be set from javascript. window.onhashchange = eventHandler; ...and... document.body.onhashchange = eventHandler; ...should both work. In radar as <rdar://problem/7138591> Patch is forthcoming...
Attachments
Fix + layout test (10.73 KB, patch)
2009-08-13 09:51 PDT, Brady Eidson 		eric: review+
eric: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Brady Eidson
Comment 1 2009-08-13 09:51:27 PDT
Created attachment 34752 [details] Fix + layout test
Eric Seidel (no email)
Comment 2 2009-08-13 10:04:32 PDT
Comment on attachment 34752 [details] Fix + layout test I'm confused by your test case. You never clear the previous listeners, I would expect them to be called again for every new change. Except from the code it looks like there is only one listener, and it's on the window object. The test doesn't make that very clear. If there is only one listener ever it seems like we should explicitly check for that, or check that body.onhashchange == window.onhashchange, etc. after setting. In fact, we don't even really need to test that it gets called, only that if you set one, they're all equal, and that if any of them are set it gets called (although testing each as you have is fine too). Is the hash supposed to be the new value or the old value during the callback? Seems we should test that. The change looks fine, but I think the test case could/should be made more clear.
Eric Seidel (no email)
Comment 3 2009-08-13 10:05:20 PDT
Comment on attachment 34752 [details] Fix + layout test Actually, I certainly don't need to see this again. so here's your r+. But I do hope you make some of the above mentioned test improvements when landing.
Brady Eidson
Comment 4 2009-08-13 14:19:44 PDT
window.onhashchange, body.onhashchange, and <body onhashchange=""> all refer to the exact same entity. This is why you don't need to "clear" it - each stage of the test overwrites the previous value. Checking equality might be an interesting addition to the test, but actually testing the function being called seems like it covers me code, as well. In practice the hash value at the time of event delivery is irrelevant since: 1 - The hashchange event is defined to be asynchronous. 2 - The event itself has no knowledge of the old or new value - it's just a simple event. I'll add the equality checks to the test before landing. Thanks!
Brady Eidson
Comment 5 2009-08-13 14:36:05 PDT
Sending LayoutTests/ChangeLog Sending LayoutTests/fast/dom/Window/window-properties-expected.txt Adding LayoutTests/fast/loader/onhashchange-attribute-listeners-expected.txt Adding LayoutTests/fast/loader/onhashchange-attribute-listeners.html Sending WebCore/ChangeLog Sending WebCore/html/HTMLBodyElement.cpp Sending WebCore/html/HTMLBodyElement.h Sending WebCore/html/HTMLBodyElement.idl Sending WebCore/html/HTMLFrameSetElement.cpp Sending WebCore/html/HTMLFrameSetElement.h Sending WebCore/html/HTMLFrameSetElement.idl Sending WebCore/page/DOMWindow.cpp Sending WebCore/page/DOMWindow.h Sending WebCore/page/DOMWindow.idl Transmitting file data .............. Committed revision 47233.
Note You need to log in before you can comment on or make changes to this bug.