[Webkit-unassigned] [Bug 188464] Support drag-and-drop for input[type=color]

[bugzilla-daemon at webkit.org]

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

--- Comment #9 from Andy Estes <aestes at apple.com> ---
Comment on attachment 346911
  --> https://bugs.webkit.org/attachment.cgi?id=346911
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346911&action=review

> Source/WebCore/page/DragController.cpp:348
> +static bool isColorInput(Node& node)

This function is slightly misnamed, because it's also checking whether the input element is not disabled.

> Source/WebCore/platform/ios/DragImageIOS.mm:258
> +    RetainPtr<UIGraphicsImageRenderer> render = adoptNS([allocUIGraphicsImageRendererInstance() initWithSize:imageRect.size()]);

This would be a good place to use auto.

> Source/WebCore/platform/mac/DragImageMac.mm:346
> +    RetainPtr<NSImage> dragImage = adoptNS([[NSImage alloc] initWithSize:NSMakeSize(swatchWidth, swatchWidth)]);

Ditto.

> Source/WebKit/UIProcess/ios/DragDropInteractionState.mm:297
> +            RetainPtr<NSString> title = (NSString *)source.linkTitle;
> +            RetainPtr<NSURL> url = (NSURL *)source.linkURL;
> +            dragItem.previewProvider = [title, url, draggingCenter] () -> UIDragPreview * {

You can avoid some retain count churn by WTFMove()ing title and url in the capture list. Even better, you could avoid these local variables entirely by writing the capture list like this: [title = retainPtr((NSString *)source.linkTitle), url = retainPtr((NSURL *)source.linkURL)].

> Source/WebKit/UIProcess/ios/DragDropInteractionState.mm:307
> +            RetainPtr<UIImage> image = source.image;
> +            dragItem.previewProvider = [image] () -> UIDragPreview * {

Ditto for image.

> Source/WebKit/WebProcess/WebCoreSupport/WebPlatformStrategies.cpp:280
> +    WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPasteboardProxy::SetPasteboardColor(pasteboardName, color), Messages::WebPasteboardProxy::SetPasteboardColor::Reply(newChangeCount), 0);

sendSync() can time out, and if so I think you'd end up with an uninitialized newChangeCount. You should either initialize newChangeCount to 0 or handle sendSync() failing.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180810/70d22a59/attachment.html>