[webkit-reviews] review granted: [Bug 57921] Move the MouseEventWithHitTestResults::targetNode() method on to EventHandler. : [Attachment 88370] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Alice Boxhall
<aboxhall at chromium.org>'s request for review:
Bug 57921: Move the MouseEventWithHitTestResults::targetNode() method on to
EventHandler.
https://bugs.webkit.org/show_bug.cgi?id=57921

Attachment 88370: Patch
https://bugs.webkit.org/attachment.cgi?id=88370&action=review

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88370&action=review

Ugh... so each port implements a part of EventHandler?	 That's so ugly.  We'd
definitely need to do a lot of refactoring here.  r=me provided you revert the
change to make subframeForHitTestResult a member of EventHandler or explain why
that's needed.

> Source/WebCore/page/EventHandler.cpp:1099
> -Frame* subframeForHitTestResult(const MouseEventWithHitTestResults&
hitTestResult)
> +Frame* EventHandler::subframeForHitTestResult(const
MouseEventWithHitTestResults& hitTestResult)

Why does this function need to be a member of EventHandler?

> Source/WebCore/page/EventHandler.h:271
> +    static Frame* subframeForHitTestResult(const
MouseEventWithHitTestResults&);

Again, I don't think this function needs to be a member function.