[webkit-reviews] review granted: [Bug 122588] Make EventDispatcher::dispatch comprehensible : [Attachment 213856] Fixed the regression

[bugzilla-daemon at webkit.org]

Andreas Kling <akling at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 122588: Make EventDispatcher::dispatch comprehensible
https://bugs.webkit.org/show_bug.cgi?id=122588

Attachment 213856: Fixed the regression
https://bugs.webkit.org/attachment.cgi?id=213856&action=review

------- Additional Comments from Andreas Kling <akling at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213856&action=review


r=me, I love that you are doing this.

> Source/WebCore/ChangeLog:38
> +	   (WebCore::Node::handleLocalEvents): Now takes Event& now.

now + now!

> Source/WebCore/dom/EventContext.cpp:87
> +    if (m_relatedTarget.get() && event.isMouseEvent())
> +	   toMouseEvent(event).setRelatedTarget(m_relatedTarget.get());
> +    else if (m_relatedTarget.get() && event.isFocusEvent())
> +	   toFocusEvent(event).setRelatedTarget(m_relatedTarget.get());

I would wrap this in a single null check like so:
if (m_relatedTarget) {
    ...
}

> Source/WebCore/dom/EventDispatcher.cpp:102
> +static void callDefaultEventHanldersInTheBubblingOrder(Event& event, const
EventPath& path)

Typo, hanlders.

> Source/WebCore/dom/EventDispatcher.cpp:186
> +    if (isHTMLInputElement(m_node.get()))
> +	   toHTMLInputElement(m_node.get())->willDispatchEvent(*m_event.get(),
clickHandlingState);

I'd use *m_node instead of m_node.get() here.

> Source/WebCore/dom/EventDispatcher.cpp:196
> +	  
toHTMLInputElement(m_node.get())->didDispatchClickEvent(*m_event.get(),
clickHandlingState);

Same here.