[webkit-reviews] review granted: [Bug 178056] dragenter and dragleave shouldn't use the same data transfer object : [Attachment 323117] Skipped the test on Windows
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Oct 7 23:41:04 PDT 2017
Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 178056: dragenter and dragleave shouldn't use the same data transfer object
https://bugs.webkit.org/show_bug.cgi?id=178056
Attachment 323117: Skipped the test on Windows
https://bugs.webkit.org/attachment.cgi?id=323117&action=review
--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 323117
--> https://bugs.webkit.org/attachment.cgi?id=323117
Skipped the test on Windows
View in context: https://bugs.webkit.org/attachment.cgi?id=323117&action=review
> Source/WebCore/page/DragController.cpp:697
> + if (!destinationOperation)
> + operation = defaultOperationForDrag(sourceOperation);
> + else if (!(sourceOperation & *destinationOperation)) // The element
picked an operation which is not supported by the source
> operation = DragOperationNone;
> - }
> + else
> + operation = *destinationOperation;
Stylistic question about * vs. using the value() function on optionals. I
almost always use the named function, but I am not sure it’s better.
> Source/WebCore/page/EventHandler.cpp:2277
> +bool EventHandler::dispatchDragEnterOrDragOverEvent(const AtomicString&
eventType, Element& target, const PlatformMouseEvent& event,
> + std::unique_ptr<Pasteboard>&& pasteborad, DragOperation sourceOperation,
bool draggingFiles, std::optional<DragOperation>& destinationOperation)
Out arguments are kind of ugly. Sure would be nice if it could all be in the
return value instead of a return value plus an out argument.
> Source/WebCore/page/EventHandler.cpp:2289
> +bool EventHandler::updateDragAndDrop(const PlatformMouseEvent& event,
std::function<std::unique_ptr<Pasteboard>()> makePasteboard, DragOperation
sourceOperation, bool draggingFiles, std::optional<DragOperation>&
destinationOperation)
Ditto.
Also, a std::function can be expensive to copy, so it would be nice if it was a
const&, assuming it’s always called synchronously and not saved for later.
More information about the webkit-reviews
mailing list