Last Comment Bug 109798 - Support multi item text/x-moz-url drags as text/uri-list on mozilla/gtk
: Support multi item text/x-moz-url drags as text/uri-list on mozilla/gtk
Status: REOPENED
:
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 Linux
: -- normal with 1 vote (vote)
: ---
Assigned To: Christopher Blizzard (:blizzard)
: John Morrison
Mentors:
: 179939 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2001-11-12 16:09 PST by James Henstridge
Modified: 2002-11-13 16:00 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Support text/uri-list drags. (6.06 KB, patch)
2001-11-12 16:11 PST, James Henstridge
no flags Details | Diff | Splinter Review
sample XUL file to demonstrate multi item DND (2.23 KB, text/plain)
2001-11-12 16:14 PST, James Henstridge
no flags Details
remerged patch (6.00 KB, patch)
2001-11-15 12:50 PST, Christopher Blizzard (:blizzard)
no flags Details | Diff | Splinter Review
3rd go at text/uri-list drags (9.41 KB, patch)
2001-11-20 00:32 PST, James Henstridge
bryner: review+
blizzard: superreview+
Details | Diff | Splinter Review
patch fixing pinkerton's issues (844 bytes, patch)
2002-01-10 11:18 PST, Christopher Blizzard (:blizzard)
no flags Details | Diff | Splinter Review

Description James Henstridge 2001-11-12 16:09:09 PST
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 2001-11-12 16:11:14 PST
Created attachment 57523 [details] [diff] [review]
Support text/uri-list drags.
Comment 2 James Henstridge 2001-11-12 16:14:39 PST
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 James Henstridge 2001-11-12 16:17:17 PST
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) 2001-11-15 12:50:51 PST
Created attachment 57967 [details] [diff] [review]
remerged patch

Patch that is remerged with the tip.
Comment 5 Christopher Blizzard (:blizzard) 2001-11-15 12:52:21 PST
That test XUL doesn't work for me.  Any ideas?
Comment 6 James Henstridge 2001-11-17 17:21:41 PST
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) 2001-11-18 15:58:28 PST
I tried loading is as chrome off the command line.
Comment 8 James Henstridge 2001-11-19 03:54:27 PST
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 2001-11-20 00:32:44 PST
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
obsolete)
Comment 10 Christopher Blizzard (:blizzard) 2002-01-03 21:45:19 PST
Comment on attachment 58515 [details] [diff] [review]
3rd go at text/uri-list drags

r/sr=blizzard
Comment 11 James Henstridge 2002-01-05 19:59:20 PST
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) 2002-01-06 22:35:32 PST
Comment on attachment 58515 [details] [diff] [review]
3rd go at text/uri-list drags

r=bryner
Comment 13 James Henstridge 2002-01-06 23:38:11 PST
I suppose now it is ready to check in :)
Comment 14 Christopher Blizzard (:blizzard) 2002-01-07 16:40:18 PST
Checked in.  Thanks, guys!
Comment 15 Mike Pinkerton (not reading bugmail) 2002-01-09 08:37:35 PST
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 2002-01-10 00:08:29 PST
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) 2002-01-10 11:18:58 PST
Created attachment 64325 [details] [diff] [review]
patch fixing pinkerton's issues
Comment 18 James Henstridge 2002-02-12 01:51:40 PST
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 [:bz] 2002-11-13 16:00:06 PST
*** Bug 179939 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.