[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


Darin Fisher (:fishd, Google) <fishd at chromium.org> changed:

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




--- Comment #7 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2010-06-02 15:29:21 PST ---
(From update of attachment 57634)
WebKit/chromium/public/WebView.h:173
 +      virtual void dragSourceMovedTo(const WebPoint& clientPoint, const WebPoint& screenPoint, WebDragOperation) = 0;
nit: please use the same formatting as dragSourceEndedAt

WebKit/chromium/src/WebViewImpl.cpp:1550
 +  void WebViewImpl::dragSourceMovedTo(
nit: this method impl should be placed after dragSourceEndedAt
to match the declaration order.

WebKit/chromium/src/WebViewImpl.cpp:1557
 +  
nit: only one new line between method impls.

WebKit/chromium/src/WebViewImpl.cpp:1676
 +  static IntSize distanceTo(const WebCore::IntPoint& point, const WebCore::IntRect& rect)
nit: no need for WebCore:: prefix since there is a using namespace WebCore
at the top of this file.  also, please move static helper functions up to
the top of the file after the static data declarations.

WebKit/chromium/src/WebViewImpl.cpp:1676
 +  static IntSize distanceTo(const WebCore::IntPoint& point, const WebCore::IntRect& rect)
nit: how about naming this distanceToRect?

how about this as a comment:
// Computes the distance from a point outside a rect to the nearest edge of the rect.

(I would also assert that the point is not contained within the rect if that
is indeed appropriate.)

WebKit/chromium/src/WebViewImpl.cpp:1695
 +      WebCore::FrameView* view = mainFrameImpl()->frameView();
nit: no WebCore:: prefix needed

WebKit/chromium/src/WebViewImpl.cpp:1693
 +      static const int kScrollMargin = 30;
nit: constants should not have the k prefix.  just do scrollMargin.

WebKit/chromium/src/WebViewImpl.h:136
 +      virtual void dragSourceMovedTo(const WebPoint& clientPoint, const WebPoint& screenPoint, WebDragOperation);
please list this method in the same order that it is declared
in WebView.h

WebKit/chromium/src/WebViewImpl.cpp:1690
 +  void WebViewImpl::scrollForDragging(const WebPoint& clientPoint)
this method should be defined just above hideSelectPopup() so that
the order of methods in the .cpp file matches the order of the
methods in the .h file.

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