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

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Joel Einbinder]

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

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 406378 [details]
Patch

[EWS Watchlist]

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

[Darin Adler]

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?

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

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