[chromium] correctly handle move drag operations
Created attachment 51400 [details] Patch
This is to work around the fact that the "move" maps to DragOperationGeneric (see https://bugs.webkit.org/show_bug.cgi?id=33697). This is consistent with what happens in WebKit/WebKit/win/.
> --- a/WebKit/chromium/src/WebViewImpl.cpp > +++ b/WebKit/chromium/src/WebViewImpl.cpp > -unsigned long WebViewImpl::createUniqueIdentifierForRequest() { > +WebDragOperation WebViewImpl::dragTargetDragging(WebCore::DragData* dragData, bool isEntering) > +{ I think the creation of DragData could also be shared in this function. DragData dragData( m_currentDragData.get(), clientPoint, screenPoint, static_cast<DragOperation>(operationsAllowed)); > --- a/WebKit/chromium/src/WebViewImpl.h > +++ b/WebKit/chromium/src/WebViewImpl.h > + // Consolidate some common code between starting a drag over a target and > + // updating a drag over a target. If we're starting a drag, |isEntering| > + // should be true. > + WebDragOperation dragTargetDragging(WebCore::DragData*, bool isEntering); Probably it might be better to call it as dragTargetDragEnterOrOver. In addition, since WebKit does not have line length, we do not need to split the long comment into 3 tidy lines.
Created attachment 51473 [details] Patch
(In reply to comment #3) > I think the creation of DragData could also be shared in this function. Done. > Probably it might be better to call it as dragTargetDragEnterOrOver. > In addition, since WebKit does not have line length, we do not need to split > the long comment into 3 tidy lines. I renamed the function to dragTargetDragEnterOrOver, but kept the comment as 3 lines because that matches the style in the rest of the file.
Comment on attachment 51473 [details] Patch Since https://bugs.webkit.org/show_bug.cgi?id=33697 got r+, I'm going to rev this patch to no longer need the GENERIC -> MOVE conversions.
Created attachment 51604 [details] Patch
Created attachment 51605 [details] Patch
Comment on attachment 51605 [details] Patch > diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h > + // Consolidate some common code between starting a drag over a target and > + // updating a drag over a target. If we're starting a drag, |isEntering| > + // should be true. > + WebDragOperation dragTargetDragEnterOrOver(const WebPoint& clientPoint, > + const WebPoint& screenPoint, > + bool isEntering); "bool isEntering" should be an enum, so it is clearer at the calling site what the function is being passed (instead of "true/false" be passed into the function).
Created attachment 51698 [details] Patch
(In reply to comment #9) > "bool isEntering" should be an enum, so it is clearer at the calling site what > the function is being passed (instead of "true/false" be passed into the > function). Done. New patch up.
Comment on attachment 51698 [details] Patch > diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h > + // Consolidate some common code between starting a drag over a target and > + // updating a drag over a target. If we're starting a drag, |isEntering| Single space after . in comments (in WK land). Sorry, I should have caught this last time.
Created attachment 51704 [details] Patch
(In reply to comment #12) > (From update of attachment 51698 [details]) > > > diff --git a/WebKit/chromium/src/WebViewImpl.h b/WebKit/chromium/src/WebViewImpl.h > > + // Consolidate some common code between starting a drag over a target and > > + // updating a drag over a target. If we're starting a drag, |isEntering| > > Single space after . in comments (in WK land). Sorry, I should have caught this > last time. Fixed throughout the file. If this is really something that's important, it should be in the style checker.
Created attachment 51707 [details] Patch for landing
Comment on attachment 51707 [details] Patch for landing Clearing flags on attachment: 51707 Committed r56603: <http://trac.webkit.org/changeset/56603>
All reviewed patches have been landed. Closing bug.