Bug 57909 - REGRESSION: Drag & Drop Gmail Attachments doesn't work
Summary: REGRESSION: Drag & Drop Gmail Attachments doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2011-04-05 17:26 PDT by Enrica Casucci
Modified: 2011-04-06 11:59 PDT (History)
5 users (show)

See Also:


Attachments
Patch (5.70 KB, patch)
2011-04-05 17:36 PDT, Enrica Casucci
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2011-04-05 17:26:00 PDT
* STEPS TO REPRODUCE:
1) Open Gmail in Safari
2) Compose a message
3) Drag a file from Finder to the Safari window.

* EXPECTED RESULTS:
The formatting bar should change to 'Drop Files' box and dropping the file should attach it to the message and upload it. 

* ACTUAL RESULTS:
Nothing. The formatting bar doesn't change to the 'Drop Files' box, and if you do drop the file, Safari opens it in the current tab instead of attaching it to the message.
Comment 1 Enrica Casucci 2011-04-05 17:27:09 PDT
<rdar://problem/9103220>
Comment 2 Enrica Casucci 2011-04-05 17:36:26 PDT
Created attachment 88343 [details]
Patch
Comment 3 Simon Fraser (smfr) 2011-04-05 17:48:18 PDT
Comment on attachment 88343 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        We canno change what platformData() returns on Mac, since there are

"cannot"
Comment 4 Darin Adler 2011-04-05 17:54:04 PDT
Comment on attachment 88343 [details]
Patch

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
> +@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.
Comment 5 Enrica Casucci 2011-04-06 11:04:56 PDT
Addressed Darin's comments.
Landed http://trac.webkit.org/changeset/83070
Comment 6 WebKit Review Bot 2011-04-06 11:59:58 PDT
http://trac.webkit.org/changeset/83070 might have broken Qt Linux Release
The following tests are not passing:
media/audio-delete-while-slider-thumb-clicked.html
media/audio-mpeg-supported.html
media/audio-mpeg4-supported.html
media/video-currentTime.html
media/video-pause-immediately.html