[webkit-reviews] review granted: [Bug 178897] DOMWindow::dispatchEvent() does not reset the event's dispatch flag : [Attachment 325163] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 27 11:03:41 PDT 2017


Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 178897: DOMWindow::dispatchEvent() does not reset the event's dispatch flag
https://bugs.webkit.org/show_bug.cgi?id=178897

Attachment 325163: Patch

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




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

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

> Source/WebCore/page/DOMWindow.cpp:2017
> +    event.setCurrentTarget(nullptr);
> +    event.setEventPhase(Event::NONE);
> +    event.resetPropagationFlags();

It’s not a great pattern to have this list of three things that needs to be
done; I wish there was some way to structure things so it was not possible to
forget one of these. For example, is there any call site that needs to reset
the propagation flags without also setting the event phase to none? If not,
then it seems we should be calling one function that does both rather than
calling two functions.

We are treating the Event object as a sort of "data holder" here; maybe
EventDispatcher could provide public functions that can do the slightly higher
level combinations correctly? Or maybe Event itself could have a bit more
policy and bit less "setters for every field" in the public functions


More information about the webkit-reviews mailing list