329564 - Context-menu "Copy Image" doesn't work

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Régis Caspar]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060306 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060306 Firefox/1.6a1

when right clicking an image and choosing "Copy Image", an error occurs when trying to paste it in a image-aware program (like MSpaint).

Reproducible: Always

Steps to Reproduce:
1. Right-click an image
2. Select "Copy Image" from the context-menu 
3. Launch MSPaint
4. Try to paste the copied image

Actual Results:  
message box stating "Error getting the Clipboard Data!" 

Expected Results:  
image gets pasted

Comment 1

[Gérard Talbot]

I went to 
http://www.mozilla.org/
and then right-clicked the Firefox image (feature-logos1.png), then chose "Copy Image" context-menu item, then launched+opened MS-Paint and then pasted the image without a problem.
And I'm using Seamonkey 1.5a rv:1.9a1 build 2006030609 under XP Pro SP2.

WORKSFORME

Comment 2

[(not currently active) Ted Mielczarek]

Seamonkey is not Firefox.  Specifically, Seamonkey is not built using thebes.

Comment 3

[(not currently active) Ted Mielczarek]

I see this on
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060303 Firefox/1.6a1

Comment 4

[Ria Klaassen (not reading all bugmail)]

When I rightclick on an image, choose Copy Image, open Paint or Irfanview, try to paste the image, I get the message: "Error collecting clipboard data" (if I translate it correctly). Then, after closing Firefox, I get a crash: TB16035247H  TB16035286E.
I don't get the crash when I don't try to paste it in Paint.

Comment 5

[Ria Klaassen (not reading all bugmail)]

Sorry; using Windows XP.

Comment 6

[Oliver Saier]

(In reply to comment #4)
Similar to Bug 228278

Comment 7

[Peter van der Woude [:Peter6]]

for what I can find this never worked in the official cairo builds (Gaius)

Comment 8

[Bengt-Erik Soderstrom]

(In reply to comment #7)
> for what I can find this never worked in the official cairo builds (Gaius)
> 
So it seems. I tested in my today's build of Firefox and when pasting into my favourite program for image viewing/processing (PMView) that one crashed!
Also trying to paste into other programs like Word is not working.
I think this is a very serious bug, which should be treated at least as a topcrash- as the bug can crash other programs!
My build data:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060322 Firefox/1.6a1

.mozconfig:
. $topsrcdir/browser/config/mozconfig
ac_add_options --enable-application=browser
ac_add_options --enable-optimize="-O2 -GA"
ac_add_options --enable-crypto
ac_add_options --disable-tests
ac_add_options --disable-debug
ac_add_options --disable-accessibility
ac_add_options --disable-activex
ac_add_options --disable-activex-scripting
ac_add_options --disable-xpconnect-idispatch
ac_add_options --enable-static
ac_add_options --disable-shared
ac_add_options --disable-places
ac_add_options --enable-svg
ac_add_options --enable-canvas
ac_add_options --enable-default-toolkit=cairo-windows

WinXP, Visual C++ 2005 Express

Comment 9

[Mark Romero]

Confirmed. I am not able to copy and image from firefox to any other program.

Comment 10

[Bengt-Erik Soderstrom]

For Firefox builds where Cairo is disabled, copying/pasting images works fine.

Comment 11

[Justin Kerk]

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

Comment 12

[[e-mail]]

Just opening and closing the clipboard viewer makes it crash for me (WinXP).

Comment 13

[chris hofmann]

I don't crash, but do see the "error getting the clipboard data" and the inability to copy and image using 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060601 Minefield/3.0a1

Comment 14

[Stuart Parmenter]

Attachment: fix

the old code scares me and leaks and is gross.

Comment 15

[Stuart Parmenter]

Comment on attachment 233888 [details] [diff] [review]
fix

er, wrong patch...

Comment 16

[Stuart Parmenter]

Attachment: fix

for real this time

Comment 17

[[e-mail]]

Comment on attachment 233889 [details] [diff] [review]
fix

>+    inImage->UnlockImagePixels(PR_FALSE);
>+    return NS_OK;
>+
>+    // Wow the old code is broken.  I'm not touching it.
>+    // It should probably lock/unlock the same object....
>+#else
...
>   inImage->UnlockImagePixels ( PR_FALSE );
>   return result;
>+#endif
Was that comment referring to the image pixels by any chance? ;-)

Comment 18

[[e-mail]]

Comment on attachment 233889 [details] [diff] [review]
fix

>+    header->biHeight        = -inImage->GetHeight();
The clipboard doesn't seem to like top-down bitmaps.

And I crash when closing the clipboard viewer. Want a stack?

Comment 19

[Stuart Parmenter]

afaik the crash isn't (directly) really related to this patch.  i've seen a crash in another part of (really really broken) widget code related to keeping an image on the clipboard on exit.  We should file a seperate bug on that.

What platform are you on?  top-down images work for me here..

Comment 20

[Stuart Parmenter]

(In reply to comment #17)
> (From update of attachment 233889 [details] [diff] [review] [edit])
> >+    inImage->UnlockImagePixels(PR_FALSE);
> >+    return NS_OK;
> >+
> >+    // Wow the old code is broken.  I'm not touching it.
> >+    // It should probably lock/unlock the same object....
> >+#else
> Was that comment referring to the image pixels by any chance? ;-)
> 

I was thinking more along the lines of:
::GlobalLock((HGLOBAL) *outBitmap))
::GlobalUnlock((HGLOBAL)outBitmap);

Comment 21

[[e-mail]]

(In reply to comment #20)
>I was thinking more along the lines of:
>::GlobalLock((HGLOBAL) *outBitmap))
>::GlobalUnlock((HGLOBAL)outBitmap);
Oh, those are there too; you can't use the HGLOBAL without them.

(In reply to comment #19)
>afaik the crash isn't (directly) really related to this patch.  i've seen a
>crash in another part of (really really broken) widget code related to keeping
>an image on the clipboard on exit.  We should file a seperate bug on that.
This is on exit of clipbrd.exe, not on exit of Gecko...
[This inline spellchecker is neat, it tells me how to spell separate!]

>What platform are you on?  top-down images work for me here..
XPSP2, but I can try W2K if you like.

Note that if I use a debugger to poke a positive height into the bitmap header then it does work, although the image is of course upside-down.

Comment 22

[Stuart Parmenter]

Attachment: another fix

this one does it without the -

Comment 23

[Stuart Parmenter]

(In reply to comment #21)
> (In reply to comment #20)
> >I was thinking more along the lines of:
> >::GlobalLock((HGLOBAL) *outBitmap))
> >::GlobalUnlock((HGLOBAL)outBitmap);
> Oh, those are there too; you can't use the HGLOBAL without them.
> 
notice that they both take a HGLOBAL and that we're passing in different values

Comment 24

[[e-mail]]

Comment on attachment 234084 [details] [diff] [review]
another fix

Yup, this seems to work fine for me.

Sorry for not spotting the missing * in the old GlobalUnlock call.