45824 - Initiating drag of image that is not also a link to desktop is very hard

Status: RESOLVED FIXED

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

Description

[Andrew Thompson]

Steps to reproduce:

1. goto http://www.palm.com
2. Try to drag the round palm logo image to the desktop.

Try several more times :-)

On MacOS with the Jul 17/18 nightly its very hard to initiate that drag.
It maybe works 1 time in 5.

(cc: pinkerton for Drag and Drop)

Comment 1

[Mike Pinkerton (not reading bugmail)]

again, this is another dupe of a bug that someone has, but not sure who

Comment 2

[Andrew Thompson]

Mike - did you mean this bug?
http://bugzilla.mozilla.org/show_bug.cgi?id=42046

It looks similar but 42046 is specific to composer.

Comment 3

[Peter Trudelle]

->pinkerton

Comment 4

[Mike Pinkerton (not reading bugmail)]

qa -> eli. again, is this a dupe of another bug you already filed?

Comment 5

[Eli Goldberg]

I thought that I wrote a bug about this, but I can't find it. Confirming.

Comment 6

[Mike Pinkerton (not reading bugmail)]

i don't know if we even support that yet. brade?

Comment 7

[Kathleen :Brade]

bouncing this bug back to pinkerton since it has to do with initiation of a drag.

I'm not sure we'll be able to fix this for 6.0.

To be able to drag images more reliably, I find that I have to click and hold for 
a moment before I move the mouse for the drag.  :-(

Pink--any way you can make this work any better?

Comment 8

[Mike Pinkerton (not reading bugmail)]

since everything else works fine, this has to be in the JS, which is out of my 
department.

--> mcafee since he's virtual gramps for triage.

Comment 9

[Kathleen :Brade]

Kevin--any chance you could help us out with this bug?  I'll be going on a 6 week 
leave soon and may not be able to get to this before I go.  I think this could be 
fixed by checking explicitly if we're in an image (after we check if it's a 
link).  Thanks!

Comment 10

[Andrew Thompson]

OK, here's the relevant function from navigatorDD.js:

var contentAreaDNDObserver = {
265   onDragStart: function (aEvent)
266     {  
267       var htmlstring = null;
268       var textstring = null;
269       var isLink = false;
270       var domselection = window._content.getSelection();
271       if (domselection && !domselection.isCollapsed && 
272           domselection.containsNode(aEvent.target,false))
273         {
274           // the window has a selection so we should grab that rather than 
looking for specific elements
275           htmlstring = domselection.toString("text/html", 128+256, 0);
276           textstring = domselection.toString("text/plain", 0, 0);
277         }
278       else 
279         {
280           switch (aEvent.target.localName) 
281             {
282               case 'AREA':
283               case 'IMG':
284                 var imgsrc = aEvent.target.getAttribute("src");
285                 var baseurl = window._content.location.href;
286                 // need to do some stuff with the window._content.location 
(path?) 
287                 // to get base URL for image.
288 
289                 textstring = imgsrc;
290                 htmlstring = "<img src=\"" + textstring + "\">";
291                 if ((linkNode = this.findEnclosingLink(aEvent.target))) {
292                   htmlstring = '<a href="' + linkNode.href + '">' + 
htmlstring + '</a>';
293                   textstring = linkNode.href;
294                 }
295                 break;
296               case 'A':
297                 isLink = true;
298                 if (aEvent.target.href)
299                   {
300                     textstring = aEvent.target.getAttribute("href");
301                     htmlstring = "<a href=\"" + textstring + "\">" + 
textstring + "</a>";
302                   }
303                 else if (aEvent.target.name)
304                   {
305                     textstring = aEvent.target.getAttribute("name");
306                     htmlstring = "<a name=\"" + textstring + "\">" + 
textstring + "</a>"
307                   }
308                 break;
309               default:
310               case '#text':
311               case 'LI':
312               case 'OL':
313               case 'DD':
314                 var node = enclosingLink(aEvent.target);
315                 textstring = "";
316                 //select node now!
317                 if (node)
318                   textstring = node.href;
319                 if (textstring != "")
320                 {
321                   htmlstring = "<a href=\"" + textstring + "\">" + textstring 
+ "</a>";
322                   var parent = node.parentNode;
323                   if (parent)
324                   {
325                     var nodelist = parent.childNodes;
326                     var index;
327                     for (index = 0; index<nodelist.length; index++)
328                     {
329                       if (nodelist.item(index) == node)
330                         break;
331                     }
332                     if (index >= nodelist.length)
333                         throw Components.results.NS_ERROR_FAILURE;
334                     if (domselection)
335                     {
336                       domselection.collapse(parent,index);
337                       domselection.extend(parent,index+1);
338                     }
339                   }
340                 }
341                 else
342                   throw Components.results.NS_ERROR_FAILURE;
343                 break;
344             }
345         }
346   
347       var flavourList = { };
348       flavourList["text/html"] = { width: 2, data: htmlstring };
349       if (isLink) 
350         flavourList["text/x-moz-url"] = { width: 2, data: textstring + " " + 
"( TEMP TITLE )" };
351       flavourList["text/unicode"] = { width: 2, data: textstring };
352       return flavourList;
353     },

How this actually behaves in practise is utterly wacko to me, but maybe someone 
can explain it:

If you drag an image that isn't a link:
You execute lines 275 and 276. i.e. its treat as a selection not as part of the  
img case

If you drag a plain link:
You'll execute lines 314 thru 343, i.e. its treat as the 'default:' case *not* as 
the <a> the link case.

If you drag an image that is a link:
You'll execute lines 284 thrue 293, including the 'if' i.e. its recognised as an 
img and then as a link.

If your drag some ordinary text, you'll execute lines 275 and 276 (i.e. you have 
a selection).

Of these 4 cases only the last 2 seem right. The first two seem a little wacko to 
me.

I think part of the reason its difficult to drag unlinked images is that it must 
be needing you to create a selection first - its not ever managing to use the img 
case.

As to why links get handled by the default case and not the <a> case I really 
don't know. I guess because its actually near impossible for the rag to start on 
an <a> as these are invisible? Perhaps the drag in fact always starts on some 
content of the <a> thus the default case is invoked?

Comment 11

[sairuh (rarely reading bugmail)]

spam: migrating drag'n'drop bugs to Terri.

Comment 12

[Paul Chen]

nav triage team:

Although nice to have, don't think we have enough time to fix for beta1, marking
nsbeta1-

Comment 13

[Paul Chen]

Marking nsbeta1- bugs as future to get off the radar

Comment 14

[Chris McAfee]

mac, pchen

Comment 15

[Asa Dotzler [:asa]]

-> default assignee

Comment 16

[George]

I've been hating this bug since the pre-1.0 release - and in every version I've
used (3 so far) on any Mac I've used, I still can't download an image to the
desktop by dragging, no matter how many times I try.

Comment 17

[sairuh (rarely reading bugmail)]

->d'n'd

Comment 18

[Simon Fraser [no longer active]]

Attachment: nsContentAreaDragDrop fixes

This patch makes it easiser to drag images, and also appends image source URL
data to the drag for platforms that want it to allow drgging files to the file
system.

The main fix for image draggability was to deal with the case in which the
image is selected (which is easy to do by clicking it); I look at the selection
endpoints to determine if just an image is in the selection, and, in that case,
fall through to the code that handles a drag of the unselected image.

Comment 19

[Simon Fraser [no longer active]]

Taking.

Comment 20

[Kathleen :Brade]

Comment on attachment 110631 [details] [diff] [review]
nsContentAreaDragDrop fixes

I don't understand this:
+    trans->SetTransferData(kFilePromiseURLMime, imageUrlPrimitive,
imageURL.Length() * sizeof(PRUnichar));
+  }http://www.mozilla.org/

Is the mozilla url a bad paste?

The rest looks fine to me; should pink or jfrancis also review?

Comment 21

[Simon Fraser [no longer active]]

Whoops; it was a bad drag and drop ;)
Seems that if you stop in gdb while debugging D&D, when you continue, the drop
goes into the wrong window.

Comment 22

[Simon Fraser [no longer active]]

Attachment: nsContentAreaDragDrop.cpp patch

This patch fixes an issue in the previous patch when selection focus is before
selection anchor (the user selected backwards).

Comment 23

[Kathleen :Brade]

Comment on attachment 110810 [details] [diff] [review]
nsContentAreaDragDrop.cpp patch

r=brade

Comment 24

[Simon Fraser [no longer active]]

This was checked in along with the fixes in bug 193053.