The default bug view has changed. See this FAQ.

ondrop gets wrong results when dragging multiple files

RESOLVED FIXED in mozilla1.9

Status

()

Core
Widget: Gtk
--
major
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Dolske, Assigned: Michael Ventnor)

Tracking

Trunk
mozilla1.9
x86
Linux
Points:
---
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
Found this with my APNG Edit extension, which lets you drag files into the extension's window (eg, individual PNG frames for the animation). On Windows and OS X this works fine. Linux, not so much. I can see that multiple items are being dropped, but each has the same bogus path.

My nsDragAndDrop callback looks something like:

    onDrop : function (event, dropData, session) {
        var dataList = dropData.dataList;
        for (var i = 0; i < dataList.length; i++) {
            dump("loop")
            var item = dataList[i].first;
            if (item.flavour.contentType == "application/x-moz-file") {
                var file = item.data.QueryInterface(Ci.nsIFile);
                dump(file.path);
            }
        }
     }

Dragging in two files gives output like:

loop
/home/blah/file1file:///home/blah/file2
loop
/home/blah/file1file:///home/blah/file2

Something early on is creating the right number of nsIFiles, but isn't setting the path correctly. Almost looks like a JS array is being used as a string, resulting in all the paths being glued together in a blob.
(Reporter)

Comment 1

9 years ago
Looks like the problem is in widget/src/gtk2/nsDragService.cpp, nsDragService::GetData()...

473                         // Sometimes there are linebreaks in the file URLs. Yes, linebreaks in the file URLs.
474                         fileUrl.StripChars("\r\n");

The fileURL string here seems to be a new-line separated list of files being dragged. And maybe some extra junk, I get some garbage-like characters after the files, and an assert:

###!!! ASSERTION: Not a UTF-8 string. This code should only be used for converting from known UTF-8 strings.: 'Error', file ../../dist/include/string/nsUTF8Utils.h, line 110
Component: Drag and Drop → Widget: Gtk
QA Contact: drag-drop → gtk
(Reporter)

Comment 2

9 years ago
This code was recently added by bug 229327.
Blocks: 229327
Yeah, Michael's regression.
Actually, I guess it's not a regression, since Michael actually added the first support for dropping files with GTK, so this didn't work before either.

So this bug is really about adding support for multi-file dropping with GTK.
(Reporter)

Comment 5

9 years ago
Well, yes and no... The onDrop handler I have accepts both x-moz-url and x-moz-file. Without 229327, I can drag multiple files into the window, and they're handled as multiple x-moz-url items. With 229327, I get multiple x-moz-file items but their paths are broken.

And, hmm, I tried commenting out the the x-moz-file entry in my GetSupportedFlavors() callback as a workaround... Works on Linux, but not OS X. Ugh.
(Assignee)

Comment 6

9 years ago
Created attachment 315481 [details] [diff] [review]
Patch

The helper function makes fixing this slightly easier.
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
(Assignee)

Comment 7

9 years ago
Comment on attachment 315481 [details] [diff] [review]
Patch

Hmm, I can't set multiple review. I'll have to ask vlad on IRC and compare his review workload with Roc's.
Attachment #315481 - Flags: superreview?(roc)
Attachment #315481 - Flags: review?(roc)
Attachment #315481 - Flags: review?(vladimir)
(Assignee)

Comment 8

9 years ago
vlad, if you get around to this before Roc, can you also mark sr and cancel my request for Roc?
This doesn't seem to handle multiple files. Does it?
(Assignee)

Comment 10

9 years ago
It certainly does. The GetData() function is actually called once per item, not once per drag, thats what the index parameter is for, as I found out just now.
Comment on attachment 315481 [details] [diff] [review]
Patch

ah right!
Attachment #315481 - Flags: superreview?(roc)
Attachment #315481 - Flags: superreview+
Attachment #315481 - Flags: review?(vladimir)
Attachment #315481 - Flags: review?(roc)
Attachment #315481 - Flags: review+
(Assignee)

Comment 12

9 years ago
Comment on attachment 315481 [details] [diff] [review]
Patch

We should take this for platform parity regarding multiple-drag-and-drop. We cant deny dolske his APNG Edit! :-)
Attachment #315481 - Flags: approval1.9?
(Reporter)

Comment 13

9 years ago
w00t. Tried it out, and fixes my problem. Thanks!
Comment on attachment 315481 [details] [diff] [review]
Patch

This definitely needs tests to land.
Attachment #315481 - Flags: approval1.9? → approval1.9-
(Assignee)

Comment 15

9 years ago
dolske, roc, do you know of a good way to test this? Do we already have drag-and-drop tests?
Dunno, ask Neil
(Reporter)

Comment 17

9 years ago
Hmm, not really sure if it can be fully automated, since Mozilla can't really initiate dragging icons from the Gnome desktop. Might have to rely on Litmus for that. Simulating a DND within (between?) a Mozilla window might be possible, although I dunno if that goes through this code.
(Assignee)

Comment 18

9 years ago
The only way to test this is through Litmus, since it relies on external factors.

1. Open Nautilus, select multiple image files.
2. Drag it to an editor (Thunderbird or Composer works best since Firefox has restrictions on this action)
3. Verify that an <img> is created for each of the image files dragged.
Flags: in-litmus?
(Assignee)

Updated

9 years ago
Attachment #315481 - Flags: approval1.9- → approval1.9?
Comment on attachment 315481 [details] [diff] [review]
Patch

a=beltzner, that'll do!
Attachment #315481 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
mozilla/widget/src/gtk2/nsDragService.cpp 	1.25 
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
You need to log in before you can comment on or make changes to this bug.