[webkit-reviews] review granted: [Bug 98139] Disambiguate innerFramePoint and mainFramePoint : [Attachment 175377] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 21 07:18:51 PST 2012


Julien Chaffraix <jchaffraix at webkit.org> has granted Allan Sandfeld Jensen
<allan.jensen at digia.com>'s request for review:
Bug 98139: Disambiguate innerFramePoint and mainFramePoint
https://bugs.webkit.org/show_bug.cgi?id=98139

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

------- Additional Comments from Julien Chaffraix <jchaffraix at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=175377&action=review


r=me.

> Source/WebCore/ChangeLog:14
> +	   Some, but not all call-sites have been all cleaned up in this patch,
and they should be
> +	   audited by the individual ports.

You don't do any clean-up now.

> Source/WebCore/ChangeLog:39
> +	   (WebCore::RenderLayer::hitTest):

Please don't forget to fill those before landing.

> Source/WebCore/rendering/HitTestResult.cpp:200
>  HitTestResult::HitTestResult(const LayoutPoint& point) :
HitTestLocation(point)

Let's fix the style of this line while we touch it (the parent class
constructor call should be on its own line).

> Source/WebKit/qt/Api/qwebframe.cpp:1601
> +    // FIXME: This should probably use roundedPointInMainFrame if it is to
match the documentation of QWebHitTestResult.

Yeah, FIXMEs and follow-up bugs are the way to go. This way you get more
focused reviews and don't get stuck on different port's policy or reviewers.


More information about the webkit-reviews mailing list