[webkit-reviews] review granted: [Bug 57909] REGRESSION: Drag & Drop Gmail Attachments doesn't work : [Attachment 88343] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 57909: REGRESSION: Drag & Drop Gmail Attachments doesn't work
https://bugs.webkit.org/show_bug.cgi?id=57909

Attachment 88343: Patch
https://bugs.webkit.org/attachment.cgi?id=88343&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88343&action=review

r=me as is, but you’ll probably have to merge with my change to this file and
you may want to consider changes based on the comments

> Source/WebCore/platform/DragData.h:121
> +    NSPasteboard* pasteboard() { return m_pasteboard.get(); }

We’re supposed to format Objective-C type pointer expressions as “NSPasteboard
*” rather than “NSPasteboard*”.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:85
> + at interface NSView (WebNSViewDetails)

The right prefix to use in WebKit2 is WK rather than Web, so this should be
WKNSViewDetails.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1458
> +// This code is needed to support drag and drop. AppKit calls _hitTest on
all the views in the application
> +// if there is no match on the dragTypes.

I don’t understand what you mean by “on all views in the application” here. I
think the comment should say that we need to override this to allow us to
support drags of all types without declaring all the types in advance. Unless
that’s not the reason.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1464
> +    NSView *hitView = [super _hitTest:point dragTypes:types];
> +    if (!hitView && [[self superview] mouse:*point inRect:[self frame]])
> +	   return self;
> +    return hitView;

I think we could instead just say:

    if ([[self superview] mouse:*point inRect:[self frame]])
	return self;
    return nil;

Then we would not need to declare the WebNSViewDetails category above.