[webkit-reviews] review denied: [Bug 39725] [Chromium] Dragging over an element with scrollbars should scroll the element when dragging near edges : [Attachment 57298] patch v1 - fix style violation

[bugzilla-daemon at webkit.org]

Jian Li <jianli at chromium.org> has denied MORITA Hajime <morrita at google.com>'s
request for review:
Bug 39725: [Chromium] Dragging over an element with scrollbars should scroll
the element when dragging near edges
https://bugs.webkit.org/show_bug.cgi?id=39725

Attachment 57298: patch v1 - fix style violation
https://bugs.webkit.org/attachment.cgi?id=57298&action=review

------- Additional Comments from Jian Li <jianli at chromium.org>
WebCore/platform/graphics/IntRect.cpp:57
 +  IntSize IntRect::directionTo(const WebCore::IntPoint& target) const
The name directionTo seems to be confusing. Will it be better to distanceTo?
Also better to comment what distance to rectangle is meant.
In addition, I am thinking if this routine is only needed in chromium platform,
probably we do not want to add it to IntRect.*.

WebCore/platform/graphics/IntRect.h:119
 +	IntSize directionTo(const WebCore::IntPoint& target) const;
The argument name 'target' can be omitted.

WebKit/chromium/public/WebView.h:172
 +	// Notified the WebView that a dragging is going on.
Notified => Notifies

WebKit/chromium/public/WebView.h:173
 +	virtual void dragSourceMovedTo(
No need to fold into multiple lines.

WebKit/chromium/src/WebViewImpl.cpp:1550
 +  void WebViewImpl::dragSourceMovedTo(
ditto.

WebKit/chromium/src/WebViewImpl.cpp:1678
 +	WebCore::FrameView* frameView = mainFrameImpl()->frameView();
Do we need to check if frameView is NULL?

WebKit/chromium/src/WebViewImpl.cpp:1684
 +	bounds.setHeight(bounds.height() - margin*2);
Should have one space before and after '*'.

WebKit/chromium/src/WebViewImpl.cpp:1686
 +	bounds.setWidth(bounds.width() - margin*2);
ditto.

WebKit/chromium/src/WebViewImpl.h:136
 +	virtual void dragSourceMovedTo(
ditto.

WebKit/chromium/src/WebViewImpl.h:331
 +	static const int kScrollMarginRatio = 20;
This const definition should be moved to WebViewImpl::scrollForDragging. Also
please comment why choosing value 20?

WebKit/chromium/src/WebViewImpl.h:353
 +	void scrollForDragging(const WebPoint& clientPoint);
The argument name can be omitted.