138049 - Advertise OS-native image data when dragging an image

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Mike Pinkerton (not reading bugmail)]

We already advertise OS-native image formats on the clipboard (DIB on Win32/PICT
on MacOS). Now we need to advertise them during a drag.

This involves:
- getting the nsIImage from the DOM node and putting it into the transferable
when the drag begins
- converting the nsIImage to OS-native format when asked for (on demand)

all this code has been written, all I need to do is hook up the pieces.

Comment 1

[Mike Pinkerton (not reading bugmail)]

Attachment: advertise CF_DIB and PICT

Comment 2

[Conrad Carlen (not reading bugmail)]

Comment on attachment 80070 [details] [diff] [review]
advertise CF_DIB and PICT

>@@ -674,5 +676,6 @@
>         NS_ASSERTION ( retVal == noErr, "SDIFD failed in DragSendDataProc" );
>     }
>-    nsMemory::Free ( data );
>+    if ( data )
>+      nsMemory::Free ( data );
>   } // if valid refcon

If this is something that can happen > once, assign nsnull to data.

>+nsContentAreaDragDrop::GetImageFrame(nsIContent* aContent, nsIDocument *aDocument, nsIPresContext *aPresContext,
>+                                        nsIPresShell *aPresShell, nsIImageFrame** aImageFrame)
>+{
>+	NS_ENSURE_ARG_POINTER(aImageFrame); 
>+	*aImageFrame = nsnull;
>+
>+	nsresult rv = NS_ERROR_FAILURE;
>+
>+	if (aDocument) {
>+		// Make sure the presentation is up-to-date
>+		rv = aDocument->FlushPendingNotifications();
>+		if (NS_FAILED(rv))
>+			return rv;
>+	}
>+
>+	if (aContent && aDocument && aPresContext && aPresShell) {
>+		nsIFrame* frame = nsnull;
>+		rv = aPresShell->GetPrimaryFrameFor(aContent, &frame);
>+		if (NS_SUCCEEDED(rv) && frame) {
>+			nsCOMPtr<nsIAtom> type;
>+			frame->GetFrameType(getter_AddRefs(type));
>+			if (type.get() == nsLayoutAtoms::imageFrame) {
>+				nsIImageFrame* imageFrame;
>+				rv = frame->QueryInterface(NS_GET_IID(nsIImageFrame), (void**)&imageFrame);
>+				if (NS_FAILED(rv)) {
>+					NS_WARNING("Should not happen - frame is not image frame even though type is nsLayoutAtoms::imageFrame");
>+					return rv;
>+				}
>+				*aImageFrame = imageFrame;

The AddRef'ing or not through here is manual. I'd rather use
nsCOMPtr<nsIImageFrame> imageFrame and then NS_ADDREF(*aImageFrame =
imageFrame) if success.

>+			}
>+			else
>+				return NS_ERROR_FAILURE;

r=ccarlen

Comment 3

[Mike Pinkerton (not reading bugmail)]

to address conrad's 2nd comment, frame are not addref'd, so using a comPtr in
this case will assert. 

cc'ing alecf for a possible sr.

Comment 4

[Alec Flett]

Comment on attachment 80070 [details] [diff] [review]
advertise CF_DIB and PICT

sr=alecf

Comment 5

[Mike Pinkerton (not reading bugmail)]

mother, may i?

Comment 6

[scottputterman]

marking adt1.0.0- for beta..  Let's take a look at this again for rtm.

Comment 7

[Mike Pinkerton (not reading bugmail)]

i'm sorry, this is needed by embedding clients. it must go on the branch.

Comment 8

[Jaime Rodriguez, Jr.]

Changing to [adt2 RTM]. adt1.0.0- for the 1.0 branch. Pls verify this on the
trunk, as we will need it shortly.

Comment 9

[Terri Preston]

pinkerton's comments on how to verify:
on the trunk, try dragging images around to various parts of the app.
> nothing should be different from before.
> 
> on mac, you can drag images to the desktop, and a PICT clipping will be
> created. on win32, you can drag images to AOL mail compose.

Verified these behaviors on Mac and windows trunk 

Comment 10

[Michael Dunn]

[Internal Reference]
http://bugscape.netscape.com/show_bug.cgi?id=4705

Comment 11

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) for approval to checkin to the 1.0 branch.  Please
get drivers a= again, since their previous approval is more than 3 days old. 
After, checking in, please add the fixed1.0 keyword.

Comment 12

[Judson Valeski]

please checkin to the 1.0.1 branch ASAP. once there please remove the
mozilla1.0.1+ keyword and add the fixed1.0.1 keyword.

Comment 13

[Mike Pinkerton (not reading bugmail)]

landed on branch yesterday.

Comment 14

[Terri Preston]

Verified fixed on win XP branch build 2002082209 and Mac OS X (Jaguar) branch
build 2002082205