Bug 215032

Summary:

REGRESSION(r261570): [GTK] Fails to send drop event to JavaScript

Product:

WebKit

Reporter:

Joel Einbinder <einbinder>

Component:

WebKitGTK

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

berto, bugs-noreply, cgarcia, darin, ews-watchlist, gustavo, pnormand

Priority:

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Linux

Attachments:

Description	Flags
Html drag and drop demo	none
Patch	darin: review+

Description Joel Einbinder 2020-07-31 14:14:55 PDT

Created attachment 405738 [details]
Html drag and drop demo

I noticed that drag and drop stopped working for me in WebKitGTK. The dragster event fires, but not the dragover/drop events. See the attached file or this link for a minimal repro: https://code.joel.tools/vAKOQRY7GDW/

I bisected it to the commit "[GTK] Rework drag and drop handling in preparation for GTK4"
https://github.com/WebKit/webkit/commit/230c1d8a9da11527b22eb7531bccc46efa0356c2

Comment 1 Carlos Garcia Campos 2020-08-10 10:02:42 PDT

Are you sure this regressed in r261570? I've debugged this and the problem seems to be that we end up writing custom data to the drag and drop pasteboard, something we don't support (because I thought it couldn't happen). So, I think this regressed in r261792, when we enabled custom data support. In any case, I'll check if we can support custom data for drag and drop too, or simply not use it and fallback to write strings.

Comment 2 Carlos Garcia Campos 2020-08-11 06:26:57 PDT

Created attachment 406378 [details]
Patch

Comment 3 EWS Watchlist 2020-08-11 06:27:48 PDT

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

Comment 4 Darin Adler 2020-08-11 14:20:58 PDT

Comment on attachment 406378 [details]
Patch

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

> Source/WebCore/platform/gtk/PasteboardGtk.cpp:337
> +                for (auto& type : customData.orderedTypes())
> +                    types.add(type);

Are these guaranteed to be ASCII lowercase?

> Source/WebCore/platform/gtk/PasteboardGtk.cpp:348
> +        return copyToVector(types);

This will yield the types in a random order, based on hashing. I think we probably want this sorted.

> Source/WebCore/platform/gtk/PasteboardGtk.cpp:462
> +            const auto& customData = data[0];

Why is it correct to look only at the first item?

Comment 5 Carlos Garcia Campos 2020-08-12 00:27:44 PDT

Comment on attachment 406378 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/platform/gtk/PasteboardGtk.cpp:337
>> +                    types.add(type);
> 
> Are these guaranteed to be ASCII lowercase?

Yes, DataTransfer normalizes the given types.

>> Source/WebCore/platform/gtk/PasteboardGtk.cpp:348
>> +        return copyToVector(types);
> 
> This will yield the types in a random order, based on hashing. I think we probably want this sorted.

Why? types is a ListHashSet, not a HashSet.

>> Source/WebCore/platform/gtk/PasteboardGtk.cpp:462
>> +            const auto& customData = data[0];
> 
> Why is it correct to look only at the first item?

It's not necessarily correct, but we don't support multiple items in the pasteboard, neither for clipboard nor for drag and drop. In this case it's correct, because writeCustomData is only called for dnd operations from DataTransfer::commitToPasteboard() that always passes a vector with a single element.

Comment 6 Carlos Garcia Campos 2020-08-12 00:36:01 PDT

Committed r265542: <https://trac.webkit.org/changeset/265542>