45823 - [D&D] Dragging non-link image to the Finder creates text clipping

Status: RESOLVED FIXED

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

Description

[Andrew Thompson]

Great work fixing up Image and Link D&D bugs, guys.

Currently, dragging an image that is inside a link (<a>) works great, but on
Macintosh with the Jul 17/18 nightly dragging an image that is *not* inside a
link creates a text clipping with the name of the image inside it, rather than
downloading the image!

Obviously this doesn't match the spec:
http://www.mozilla.org/docs/refList/user-interface/specs/dragndrop/

Comment 1

[Andrew Thompson]

Some images cannot be dragged to the desktop at all:

e.g. http://www.w3c.org/

Try to drag the w3c logo to the desktop - its won't drag to the desktop it snaps
back.

Contrast www.palm.com. 

Try to drag the round Palm logo to the desktop. If you can get the drag to start 
at all, you will see the text clipping behaviour I mentioned in the initial 
description.

Comment 2

[Mike Pinkerton (not reading bugmail)]

this is a dupe of another bug, i don't know who has it though.

Comment 3

[Andrew Thompson]

Looks similar to:
http://bugzilla.mozilla.org/show_bug.cgi?id=44260

But the symptoms are not the same.

44260 says that linked images create a clipping with the alt text in them
(which in my experience is no longer true)
This bug says *unlinked* images create a clipping with the *filename* in them.

Could be the same underlying cause....

Comment 4

[Peter Trudelle]

->pinkerton

Comment 5

[Mike Pinkerton (not reading bugmail)]

changing qa contact to eli, since this is D&D. Eli, didn't you already file 
this bug? Please mark it a dupe if you did, or tell me that I'm hallucinating 
and that this one is new.

Comment 6

[Mike Pinkerton (not reading bugmail)]

brade did the image work, looks like hers

Comment 7

[Kathleen :Brade]

cc [e-mail]; Kevin--do you think you could help us look into this bug?  
This bug is probably caused by a relative url being dragged instead of an 
absolute url.

Comment 8

[Zach Lipton [:zach]]

Marking confirmed

Comment 9

[Kathleen :Brade]

reassign to Ben since this fix will be in navigatorDD.js file; Ben, I'm going on 
sabbatical so I won't be able to fix this for 6 weeks.  Sorry!

pinkerton--ben may need some help with this one since it's mac-specific

Comment 10

[johng]

nav triage team:
as long as save as works, we can live with this, unhappily.  Pinkerton, if you 
care, reassign to yourself

Comment 11

[Matthew T (active 1999-2002)]

Build: 2000090820, MacOS 9.0

What I got:
* text clipping
* name of clipping = some approximation of ALT text (e.g. `>' in the ALT text
  appeared as `>', and not `>' as it should have done), surrounded by [] and
  followed by ` clipping'
* contents of clipping = the same approximation of ALT text, surrounded by []
* Comments field of clipping = nothing

What I expected:
* picture clipping
* name of clipping = replacement text as calculated by the
  getAltSomethingSomething function (sorry, can't remember its proper name), with
  colons converted to dashes, followed by ` clipping'
* Comments field for clipping = URL of image.

Taking QA, since Eli has gone.

Comment 12

[Jesse Ruderman]

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

Comment 13

[Jesse Ruderman]

Note that the reporter of bug 89082 thought you should get the image file after 
the drag, whereas mpt suggested in this bug that you should get an image 
clipping.  (Are those the same thing?  I'm not a Mac person.)

Comment 14

[Andrew Thompson]

It doesn't *really* matter. An image clipping is just a built in kind image 
format in any case. 

But actually, given the image you're dragging is (99%) likely to be a GIF, PNG or 
JPEG I'd say just download the image and give it the filename it has on the 
server. 

There are a couple of reasons for this:

* I'm not as interested in what the alt text is as I am in what the creator of 
the image named it. Most images don't have good alt text, and Matthew once spent 
a long time convincing me that a lot of images shouldn't have alt text at all.

* Picture clipping are encoded in the old Mac image format (old as in versions < 
Mac OS X) PICT. Given the image on the server is PNG/GIF/JPEG in all probability 
converting it to a PICT could either mess up the palette or force the user to go 
through another round of lossy compression to change it back into a JPEG. PICTs 
may also be a larger size than the original. Plus I'd rather have the image in 
the format it was intended. I'm highly unlikely to want to work with a PICT if 
I'm a web designer (and ordinary users probably just want to save the image "as 
is").

* Mac users can open any of those formats, either by using Mozilla, or by using 
Quicktime, its not like it being a clipping confers an advantage...

Now, if we were offering the ability to drag and drop arbitrary regions on a page 
and have the browser save those, maybe a clipping would work. But that's another 
bug: specifically bug 41480.

Comment 15

[adelano]

> It doesn't *really* matter. An image clipping is just a built in kind image 
> format in any case. 

Image clipping is less useful because it can't conveniently be opened in another image 
application; it can only be imported into one via the clipboard. Also, file utilities such as Acme 
CMM Widgets can't be used to audit the image information (size, type, color info, etc), since it's 
not the actual file.

Embedding the alt text or url into the file's info field would be nice. Actually, it would be very 
nice.

> Plus I'd rather have the image in the format it was intended. I'm highly
> unlikely to want to work with a PICT if I'm a web designer (and ordinary users
> probably just want to save the image "as is").

Ding!

Comment 16

[Kathleen :Brade]

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

Comment 17

[Matthew T (active 1999-2002)]

Reproduced in build 2001111508, Mac OS 9.1. --> pchen after talking with Ben.

Comment 18

[Asa Dotzler [:asa]]

->XP Apps default assignee

Comment 19

[Peter Trudelle]

->xpapps d&d

Comment 20

[Blake Ross]

Whatever happened to copy image? I thought someone (anthonyd?) was working on
that awhile ago.

Comment 21

[Not interested in Mozilla any more]

Gosh, this one's been around a long time... Any idea when it's going
to get fixed :(

Comment 22

[Pete Boyd]

is this a duplicate of bug 97413?

Comment 23

[Greg K.]

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

Comment 24

[Terri Preston]

qa contact -> pmac

Comment 25

[Simon Fraser [no longer active]]

Taking

Comment 26

[Simon Fraser [no longer active]]

Attachment: widget changes for dragging images to the Finder on Mac

This patch allows dragging images to the Finder, for Chimera (it's a patch
against the Chimera branch). The main changes were to add HFS Promise flavors
to the drag, and to handle them by creating a file, then using
nsIWebBrowserPersist to save the image to the file, given its source URL.

This patch requires the patch in attachment 110631 [details] [diff] [review] in bug 45824 to work.

Comment 27

[Robert Accettura [:raccettura]]

Sweet.. Can't wait to see this implemented.  Can we get the ball rolling to
review this?  I can't wait.  This has been one of the biggest setbacks mozilla
has IMHO.

Now Windows doesn't seem to have dragable images yet either...

Comment 28

[Simon Fraser [no longer active]]

No longer future.

Comment 29

[Kathleen :Brade]

Comment on attachment 110633 [details] [diff] [review]
widget changes  for dragging images to the Finder on Mac

remove the printf's or put it in a DEBUG block.

This line has inconsistent spacing compared to nearly identical lines 3 and 4
above it:
::AddDragItemFlavor ( mDragRef, itemIndex, macOSFlavor, NULL, 0, flags );


For this block, I think we need to initialize dataSize since it isn't
guaranteed to be set if GetTransferData has some type of error.
+      PRUint32 dataSize;
+      item->GetTransferData(kFilePromiseURLMime,
getter_AddRefs(imageURLPrimitive), &dataSize);

You don't need to initialize err in this case (I'd just declare it on the
assignment line):
+  OSErr err = noErr;
+
+  AEDesc dropLocAlias = { typeNull, nil };
+  err = ::GetDropLocation(dragRef, &dropLocAlias);

CreatePromisedFile also assigns err twice in a row (if you care about such
minor waste).

For the following line, I prefer the != noError format you use earlier in the
patch.	However, choose whichever format you prefer.
+    if (!(err = ::FSpGetFInfo(fss, &finderInfo)))

This only addresses Cocoa widget changes; non-cocoa Mac builds will still have
problems, right?

Comment 30

[Simon Fraser [no longer active]]

Attachment: widget/src/cocoa patch

This patch addresses brade's and mike's comments, and, as an extra holiday
bonus, adds 'PICT' data to the drag so that you can drag images directly into
PhotoShop. At last, we have 4.x parity, just 4 years later!

Comment 31

[Kathleen :Brade]

Comment on attachment 110812 [details] [diff] [review]
widget/src/cocoa patch

r=brade

however, this block may need some minor adjustment to account for a 0-sized
pichandle:
+      PicHandle picture = nsnull;
+      image->ConvertToPICT(&picture);
+      if (!picture) return cantGetFlavorErr; 
+
+      PRInt32 pictSize = ::GetHandleSize((Handle)picture);
+      char* pictData = nsMemory::Alloc(pictSize);
+      if (pictData) {
+	 ::BlockMoveData(*picture, pictData, pictSize);    // doesn't move
memory
+	 *outData = pictData;
+	 *outDataSize = pictSize;
+	 retVal = noErr;
+      }
+      else
+	 retVal = cantGetFlavorErr;

Comment 32

[Ed Sabol]

> however, this block may need some minor adjustment to account for a 0-sized
> pichandle:

Brade, sounds simple enough. Is this what you mean?

+      PRInt32 pictSize = ::GetHandleSize((Handle)picture);
+      if (!pictSize) return cantGetFlavorErr; 

It it too late to get this in 1.3?

Comment 33

[Kathleen :Brade]

Ed Sabol--yes, that is what I meant
And yes, I'd guess it's too late to get this into 1.3 but I'm not sure.

Comment 34

[Ed Sabol]

We have a really nice patch here. It just needs one little tweak, I think. I
hope we can get this landed for 1.4a now that Simon is supposedly back...

Comment 35

[HARUNAGA Hirotoshi]

It seems to me this was fixed by Bug 193053.

Comment 36

[Simon Fraser [no longer active]]

Yup.

Comment 37

[HARUNAGA Hirotoshi]

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