Support multi item text/x-moz-url drags as text/uri-list on mozilla/gtk

Assigned to



16 years ago
15 years ago


(Reporter: James Henstridge, Assigned: blizzard)



Firefox Tracking Flags

(Not tracked)



(3 attachments, 2 obsolete attachments)



16 years ago
Currently gtk builds of mozilla only support multi item drags within a single
instance of the application.  The following patch adds support for providing
multi item text/x-moz-url drags as a text/uri-list drag (the format used by
Nautilus and various other file managers).

This bug is a counterpart of bug 107651, which added support for text/uri-list
drop support to mozilla.

If the drag is multi-item, and text/x-moz-url is one of the supported flavours,
the drag is provided as text/uri-list instead (if both are provided, the single
item text/x-moz-url data is chosen in preference to the text/uri-list on the drop).

It also makes a change to nsDragService::IsTargetContextList so that the
internal flavour type is only used for drags within the same app.

Comment 1

16 years ago
Created attachment 57523 [details] [diff] [review]
Support text/uri-list drags.

Comment 2

16 years ago
Created attachment 57525 [details]
sample XUL file to demonstrate multi item DND

This sample XUL file can be used to demonstrate multi item DnD.  Copy it to
somewhere in the chrome dir and start two instances with "mozilla -chrome
chrome://...", and perform drags between them.

Comment 3

16 years ago
One possible change to the patch might be so that multi item drags are only
provided with flavours application/x-moz-internal-item-list (the internal
format) and text/uri-list.  With my patch, all the other targets are advertised
as well, which may not be desirable.
Ever confirmed: true
Keywords: patch

Comment 4

16 years ago
Created attachment 57967 [details] [diff] [review]
remerged patch

Patch that is remerged with the tip.

Comment 5

16 years ago
That test XUL doesn't work for me.  Any ideas?

Comment 6

16 years ago
Weird.  I will test things out a bit on my end (using a trunk build).  Did you
make sure to load the XUL file as chrome? (just typing the url into the location
bar of a mozilla window doesn't seem to work).

What do you think about limiting the advertised drag flavours to just
application/x-moz-internal-item-list and text/uri-list for multi item drags?

Comment 7

16 years ago
I tried loading is as chrome off the command line.

Comment 8

16 years ago
Just built mozilla from the trunk with your updated patch, and the drags worked
as expected.

The detailed steps I used were:

1. create dist/bin/chrome/temp/content subdirectory

2. copy the XUL attachment to that directory as text-uri-list-dnd.xul

3. create contents.rdf file in that directory with the following content:
  <?xml version="1.0"?>
  <RDF:RDF xmlns:RDF=""
    <RDF:Seq about="urn:mozilla:package:root">
      <RDF:li resource="urn:mozilla:package:temp"/>
    <RDF:Description about="urn:mozilla:package:temp"
4. add the following line to dist/bin/chrome/installed-chrome.txt:
5. start two copies of mozilla with the following command:
  ./mozilla -chrome chrome://temp/content/text-uri-list-dnd.xul

Dragging the button to the bottom drop frame of the same window does a drop
using the internal drag format (you can tell because the titles are displayed in
the frame, which get stripped when the data is transmitted as text/uri-list). 
When draging to the other window, the text/uri-list format is used.

If I change the URLs to file:// ones, I can drag the files to the desktop
(handled by nautilus) and they get copied.

Comment 9

16 years ago
Created attachment 58515 [details] [diff] [review]
3rd go at text/uri-list drags

Here is a third version of the patch, which changes
nsDragService::GetSourceList to only advertise the
aplication/x-moz-internal-item-list and text/uri-list targets for multiple item
drags.	This should make it easy to add support for new multi item flavours if
they are needed in the future.

(hmm. bugzilla doesn't seem to allow the submitter of a bug to mark patches as


16 years ago
Attachment #57523 - Attachment is obsolete: true


16 years ago
Attachment #57967 - Attachment is obsolete: true

Comment 10

16 years ago
Comment on attachment 58515 [details] [diff] [review]
3rd go at text/uri-list drags

Attachment #58515 - Flags: superreview+

Comment 11

16 years ago
Thanks for reviewing the patch.  I don't have a mozilla cvs account, so can't
check it in myself.  Chris: could you check the patch in for me?  Then this
report can be marked fixed.
Comment on attachment 58515 [details] [diff] [review]
3rd go at text/uri-list drags

Attachment #58515 - Flags: review+

Comment 13

16 years ago
I suppose now it is ready to check in :)

Comment 14

16 years ago
Checked in.  Thanks, guys!
Last Resolved: 16 years ago
Resolution: --- → FIXED
sorry for coming in late here, but i see a couple of error-case issues:

+CreateUriList(nsISupportsArray *items, gchar **text, gint *length)
+  PRUint32 i, count;
+  GString *uriList = g_string_new(NULL);

if this fails, we'll crash later on in the routine. no checks are made to ensure
it succeeded.

+    CreateUriList(mSourceDataItems, &uriList, &length);
+    gtk_selection_data_set(aSelectionData, aSelectionData->target,
+                           8, (guchar *)uriList, length);
+    g_free(uriList);
+    return;

same thing here. no checks that uriList is valid before passing it to gtk and
then freeing it.

any way to get this fixed or should we open a new bug?

Comment 16

16 years ago
If g_string_new() fails, then it will abort() (like anything else using glib
1.2's memory management).  Basically any gtk call may fail in this way.  I don't
think it is worth fixing but if you do, then a new bug should probably be filed.

Comment 17

16 years ago
Created attachment 64325 [details] [diff] [review]
patch fixing pinkerton's issues

Comment 18

16 years ago
just tried out the test XUL file in the mozilla-0.9.8 release, and the interapp
drags are not functioning :(  Will need to look at what is going on.

Looks like the drag is being accepted, but the data isn't received.
Resolution: FIXED → ---
*** Bug 179939 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.