[Webkit-unassigned] [Bug 39725] [Chromium] Dragging over an element with scrollbars should scroll the element when dragging near edges

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=39725


Jian Li <jianli at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #57298|review?                     |review-
               Flag|                            |




--- Comment #4 from Jian Li <jianli at chromium.org>  2010-06-01 17:43:11 PST ---
(From update of attachment 57298)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.