[webkit-reviews] review denied: [Bug 28260] onhashchange property cannot be set from javascript : [Attachment 34752] Fix + layout test

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Brady Eidson <beidson at apple.com>'s
request for review:
Bug 28260: onhashchange property cannot be set from javascript
https://bugs.webkit.org/show_bug.cgi?id=28260

Attachment 34752: Fix + layout test
https://bugs.webkit.org/attachment.cgi?id=34752&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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.