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

Status: REOPENED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[James Henstridge]

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

[James Henstridge]

Attachment: Support text/uri-list drags.

Comment 2

[James Henstridge]

Attachment: 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

[James Henstridge]

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.

Comment 4

[Christopher Blizzard (:blizzard)]

Attachment: remerged patch

Patch that is remerged with the tip.

Comment 5

[Christopher Blizzard (:blizzard)]

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

Comment 6

[James Henstridge]

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

[Christopher Blizzard (:blizzard)]

I tried loading is as chrome off the command line.

Comment 8

[James Henstridge]

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="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
           xmlns:chrome="http://www.mozilla.org/rdf/chrome#">
    <RDF:Seq about="urn:mozilla:package:root">
      <RDF:li resource="urn:mozilla:package:temp"/>
    </RDF:Seq>
    <RDF:Description about="urn:mozilla:package:temp"
          chrome:displayName="Temp"
          chrome:author="temp"
          chrome:name="temp">
    </RDF:Description>
  </RDF:RDF>
4. add the following line to dist/bin/chrome/installed-chrome.txt:
  content,install,url,resource:/chrome/temp/content/
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

[James Henstridge]

Attachment: 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
obsolete)

Comment 10

[Christopher Blizzard (:blizzard)]

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

r/sr=blizzard

Comment 11

[James Henstridge]

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 12

[Brian Ryner (not reading)]

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

r=bryner

Comment 13

[James Henstridge]

I suppose now it is ready to check in :)

Comment 14

[Christopher Blizzard (:blizzard)]

Checked in.  Thanks, guys!

Comment 15

[Mike Pinkerton (not reading bugmail)]

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

[James Henstridge]

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

[Christopher Blizzard (:blizzard)]

Attachment: patch fixing pinkerton's issues

Comment 18

[James Henstridge]

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.

Comment 19

[Boris Zbarsky [:bzbarsky]]

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

Comment 20

[Jonathan]

This looks like it could use a new evaluation

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:enndeakin, could you have a look please?
For more information, please visit auto_nag documentation.