[webkit-reviews] review granted: [Bug 169789] Returning false in event handler must not cancel CustomEvent of type beforeunload : [Attachment 304953] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 169789: Returning false in event handler must not cancel CustomEvent of
type beforeunload
https://bugs.webkit.org/show_bug.cgi?id=169789

Attachment 304953: Patch

https://bugs.webkit.org/attachment.cgi?id=304953&action=review




--- Comment #10 from Darin Adler <darin at apple.com> ---
Comment on attachment 304953
  --> https://bugs.webkit.org/attachment.cgi?id=304953
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=304953&action=review

>>> Source/WebCore/bindings/js/JSEventListener.cpp:184
>>> +		     return;
>> 
>> Why do we use event->type() == eventNames().beforeunloadEvent here but
is<BeforeUnloadEvent>(*event) above?
> 
> To state the obvious, we have to check is<BeforeUnloadEvent> above, because
it guards a cast and the code uses data that is not stored in an event that
just happens to be named "beforeunload" but was not generated by the DOM with
the appropriate underlying class.
> 
> So whatever we decide, we must not change that to just checking the event
type.

OK, I remember now, this is exactly what the change is all about! You can have
a custom event of type "beforeunload" and it has to work. I believe this is
covered by the test cases.