Bug 40833 – [GTK] The GTK+ EventSender needs support for beginDragWithFiles

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Martin Robinson]

This is required for a bunch of drag-and-drop tests to pass.

[Martin Robinson]

*** Bug 34246 has been marked as a duplicate of this bug. ***

[Adam Bergkvist]

Created attachment 74376 [details]
proposed patch

The patch has been tested with LayoutTests/fast/files (requires building with --blob, see build fix https://bugs.webkit.org/show_bug.cgi?id=49724).

[WebKit Review Bot]

Attachment 74376 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebKitTools/ChangeLog', u'WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp', u'WebKitTools/DumpRenderTree/gtk/EventSender.cpp', u'WebKitTools/DumpRenderTree/gtk/EventSender.h']" exit_code: 1
WebKitTools/DumpRenderTree/gtk/EventSender.cpp:421:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Bergkvist]

Created attachment 74393 [details]
updated patch

Fixed style error.

[Martin Robinson]

We should wait to land this about File support is implemented. After that I think we should unskip as many of the files that are failing in the Skipped list because of this issue that we can.

[Martin Robinson]

Created attachment 78296 [details]
Patch

[Martin Robinson]

I've merged my earlier work on this issue with Adam's patch and together we've  unskipped tests and organized the remaining failures. Attached is the result of that work.

[Martin Robinson]

Comment on attachment 78296 [details]
Patch

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

> LayoutTests/platform/gtk/Skipped:5355
> +editing/pasteboard/file-drag-to-editable.html:

If everything else is okay, I can fix this broken Skipped line before landing.

[Gustavo Noronha (kov)]

Comment on attachment 78296 [details]
Patch

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

Looks good!

> Tools/DumpRenderTree/gtk/EventSender.cpp:439
> +    GFile* testFile = g_file_new_for_uri(mainFrameURI);

We should be able to use GRefPtr here, since we're using GOwnPtr for the chars =)?

> Tools/DumpRenderTree/gtk/EventSender.cpp:452
> +        parentDirectory = g_file_new_for_path(currentDirectory.get());

You're leaking the parentDirectory returned by g_file_get_parent() here. Perhaps fix this just by using GRefPtr on it as well =).

[Martin Robinson]

(In reply to comment #9)
> You're leaking the parentDirectory returned by g_file_get_parent() here. Perhaps fix this just by using GRefPtr on it as well =).

Thanks for the review! I will fix these issues before landing by switching all raw GFile references to GRefPtr.

[Martin Robinson]

Committed r76031: <http://trac.webkit.org/changeset/76031>