210043 - Context menu "copy image" (not URL) to paste into image editing software

Status: RESOLVED FIXED

(Firefox :: Menus, enhancement, P2)

Description

[p.cranston]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6

When right clicking on an image it allows you to view image, copy image
location, save image, set as wallpaper etc, but there is no option to copy the
image to the clipboard. It would be really handy to be able to do this, ie: copy
image, paste into paint shop pro, fireworks etc, next image, copy -> paste
etc... To save each image and then browse to the location is really slow...

Reproducible: Always

Steps to Reproduce:
1. Right click on image.
2. Scream in frustration at the inability to copy the image to the clipboard...  :o)


Actual Results:  
I screamed in frustration at the inability to copy the image to the clipboard...
 :o)

Expected Results:  
Given the option to copy the image to the clipboard... :o)

Comment 1

[Jesse Ruderman]

This is the same as bug 135300 for Seamonkey.  As I said there, I think "Copy
image location" on context menus should be replaced by "Copy image".

Comment 2

[seph]

That is why I still use Internet Explorer myself. I really love Mozilla, but I 
need to be able to copy images to work on my website. You would think that out 
of all of these new versions they could just add a copy image command, but they 
seem to refuse to fix it.

Comment 3

[scratch]

I disagree that it should replace copy image location.  that functionality is
also very useful, so they both should be there.

Comment 4

[Jonadab the Unsightly One (Nathan Eady)]

Agree that "copy image location" is at least as useful, but ideally both
should be present.

Recommend Hardware/OS to be All/All, as I don't know of any specific reason 
to exclude this functionality on other platforms, if it is implemented.

Comment 5

[Simon Paquet [:sipaq]]

Taking QA Contact as designated owner of Firebird-Menus. Sorry for bugspam.

Comment 6

[Dai Toyama]

Could someone please change the OS from "Windows XP" to "All"?  I could use this
feature on Linux, too.

Comment 7

[Jesse Ruderman]

The backend to copy images already exists on Windows but not on Linux.  Getting
the backend working on Linux should not block adding this context menu item
(replacing Copy Image Location) on Windows, because there will probably always
be a platform on which Mozilla runs but can't copy images.

Comment 8

[David P James]

Well Konqueror can do it - right-click on an image, select the "Copy To" menu
group and party on. There is also a "Save Image As" as well. Galeon does not
have this but Galeon does have the ability to open the image in another app,
such as the GIMP.

Comment 9

[Jed Brown]

This definitally needs to be in gecko.

There is an extension here: http://extensionroom.mozdev.org/#copyimage
that does this, although I am un-aware as to what other platforms besides
windows this works on.

Comment 10

[Jo Hermans]

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

Comment 11

[Shaun Rotman]

there is a way to copy image, although not the conventional "right click and
choose copy" method, WHICH I WAS EXISTED! anywho, you can copy the image by
taking the mozilla window and having the other program you want to copy the
image to also up on teh screen. now that both windows are visible, just DRAG the
image right off the webpage INTO the program you want to copy it to! it works,
ive tried it! but still i sit in anger like the rest of you when i say that WE
WANT THE RIGHT-CLICK-COPY option! hope this helps some of you out there.

Comment 12

[Jon Henry]

Updating OS/Hardware as requested.

Comment 13

[HJ]

I don't like it when they remove "Copy Image Location". Why not add a new menu
item? What if people need to copy the location of that image? Install yet
another extension? mozilla is more and more becomming 'puzzleware'. Great for
new users. Glue your own extensions to make a perfectly working/functional MSIE
clone...

Comment 14

[Jon Henry]

I have a patch for this, but before I bother people asking for review and such
it has a couple issues. First, for some reason the Copy Image functionality
causes builds made using GCC to crash when you exit (doing a debug build now to
try and track it down). Second, am I to understand from bug 21747 that this
feature should not be enabled on linux (yet), because cmd_CopyImageContents
doesn't work there? Finally, data copied using this menu option won't paste into
Thunderbird (but will paste into MS Word, Paint Shop Pro, and other apps I
tested on). I have no idea why, but it might be an oversight in the new image
paste code in MailNews and Thunderbird. Any words of advice on any of these
things would be appreciated.

Comment 15

[Jon Henry]

Attachment: Proposed patch

This is the patch, which crashes when copying data then quitting in a MingW/GCC
build.

Comment 16

[Jon Henry]

Attachment: Stack trace

This is the stack trace from the crash.

Comment 17

[Jon Henry]

Attachment: Patch v1.1

This is essentially the same patch, but with #ifdefs to keep it from affecting
Linux (I've received confirmation that the copy image code does indeed not work
on Linux). There is still the crash in GCC builds, on which I'm about to file a
bug.

Comment 18

[Jon Henry]

Bug 228278 filed on the related crash issue.

Comment 19

[Robert Parenton]

It appears that everything in the patch is already in the code except for the
changes to browser.dtd

Comment 20

[Robert Parenton]

(In reply to comment #19)
> It appears that everything in the patch is already in the code except for the
> changes to browser.dtd

Scratch that comment, I misread the code.

Comment 21

[Blake Ross]

Does copy image work on Mac?

Comment 22

[Robert Parenton]

Blake,

I built with the Patch v1.1, and it works great on Windows, and doesn't get used
on Linux (which is the correct behavior).  Getting copy image to work on Linux
won't be easy, as it depends on which desktop environment or window manager you
are using.

Comment 23

[Blake Ross]

Robert: does it work on *Mac*?  Your patch doesn't disable the context menu item
on mac.

Comment 24

[Robert Parenton]

Blake: Unfortunately I have no way to test on Mac, so I don't know if it works
there.  I just wanted to note that jhenry's patch did in fact work properly on
Windows and Linux since nobody had actually confirmed that.

Comment 25

[Blake Ross]

Fix checked in. Thanks for the patch!

Comment 26

[alanjstr]

If this doesn't work on Linux (and we're unsure about Mac), then why is it
considered fixed on anything other than Windows?

Comment 27

[Robert Parenton]

Reopening as this is only fixed on Windows (and maybe Mac - can anybody verify
if it works on Mac).

We can't fix this until Bug 21747 has been fixed.

OS -> Linux

Comment 28

[Steven Garrity [:sgarrity]]

I have a mac I can test this on (G3 iMac with 10.3.2) - I don't see it in the
nightly - is it there? If not, can someone help me an OS X build?

Comment 29

[Jon Henry]

Blake checked the patch in last night, so it should show up in today's (Feb
17th) nightlies.

Thanks for getting this in for me. I tried to get some testing of it in the
Mozillazine forums, but for whatever reason all the volunteers ran Windows. Let
us know if this works out Steven.

Comment 30

[Blake Ross]

It apparently works on Mac.

Bug 21747 is the bug to implement this on Linux.  Resolving once again...

Comment 31

[alanjstr]

Bug 21747 is about implementing the backend, not the front-end, thus it was a
dependency for this bug.  So either this should stay open, or a new bug should
be filed for the Linux front-end.

Comment 32

[Brian 'netdragon' Bober]

In my opinion, a new bug should be filed for the Linux front end and be made to
depend on bug 21747 as this bug seems to be pretty much fixed. All that needs to
be done is remove the #ifdef XP_UNIX statements for Linux, which will occur when
bug 21747 is fixed, and therefore a more specific bug would be "Enable 'copy
image'  context menu code for XP_UNIX when backend is complete" and be made to
depend on bug 21747 and the dependency be removed from this bug.

Comment 33

[Robert Parenton]

Bug 234716 has been filed to enable the front-end on Linux after Bug 21747 has
been fixed.

Comment 34

[Jon Henry]

Out of curiosity, how was it determined that this works on macs? Pike emailed me
with this URL: http://www.mozilla.org/build/mac.html#build_defines, which seems
to suggest the code would be ifdef'd out on macs (since OS X counts as a unix
aparrantly). I have no idea if this applies to the XUL preprocessor though, and
if so how to code around it, since when I was developing this patch the
preprocessor seemed to accept only the most rudimentary #ifdef constructs. Good
catch though, Pike.

Comment 35

[Steffen Wilberg]

all.js uses these lines to include code for Unix only:
#ifndef XP_MACOSX
#ifdef XP_UNIX
http://lxr.mozilla.org/mozilla/source/modules/libpref/src/init/all.js#1494

But need it the other way round, include Mac and exclude Unix. I don't know if
it's possible to do that without duplicating the code:
#ifndef XP_UNIX
  // all platforms but Unix and Mac
#endif
#ifdef XP_MACOSX
  // Mac
#endif
http://lxr.mozilla.org/mozilla/source/config/preprocessor.txt (checkin comment:
"Preprocessor documentation, believe it or not.")

Comment 36

[Steffen Wilberg]

This is not fixed on Mac. The code has #ifndef XP_UNIX, and that implies _not
Mac_, see comment 34 and comment 35. I already said in comment 35 how the code
has to look like if we want to exlude Unix but include Mac.
Reopening.
The question is: Does this work on Mac if we enable it? Maybe we should just try
it on the trunk and ask somebody to test it. Mike?

Comment 37

[Kevin Ar18]

How about using "copy" instead of "copy image"  By using "copy" it would provide
consistency with other copy operations for items that the user clicks on (like
highlighted text); also it would be a shorter word than "copy image" allowing
the user to distinguish it from "copy image location" and thus find it quicker
on the context menu.
The only downside is that some people might not know what "copy" is copying. 

Comment 38

[chris jones]

As a possible work around on linux, if you highlight the image and press ctrl-c
you can paste it into openoffice.  This does not seem to work for other programmes.

Comment 39

[Worcester12345]

needs aviary landing keyword

Comment 40

[Jon Henry]

Why does it need aviary-landing? This appears to work the same on the trunk as
it did in 1.0...the fact that Linux and Mac don't work is unrelated.

Comment 41

[OstGote!]

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

Comment 42

[Asaf Romano (gone)]

wee, we're #iifndefing OS X soemthing which _does_ work on mac, upcomming patch.

Comment 43

[Asaf Romano (gone)]

Attachment: enable the command on windows, carbon and cocoa widget

Comment 44

[Asaf Romano (gone)]

Assuming the linux part is bug 234716, taking.

Comment 45

[Robert Parenton]

(In reply to comment #44)
> Assuming the linux part is bug 234716, taking.

The Linux part is Bug 234716, but it depends on the backend in Bug 21747

Comment 46

[Asaf Romano (gone)]

OK, so bug 135300 has changed the way "Copy Image" works, not to someting we
want/need. Need to fix that as well

Comment 47

[Asaf Romano (gone)]

Comment on attachment 180572 [details] [diff] [review]
enable the command on windows, carbon and cocoa widget

OK, i'm doing the other part in bug 291110, this one doesn't need more changes.

Comment 48

[Asa Dotzler [:asa]]

Comment on attachment 180572 [details] [diff] [review]
enable the command on windows, carbon and cocoa widget

a=asa

Comment 49

[Asaf Romano (gone)]

Checking in base/Makefile.in;
/cvsroot/mozilla/browser/base/Makefile.in,v  <--  Makefile.in
new revision: 1.14; previous revision: 1.13
done
Checking in base/content/browser-context.inc;
/cvsroot/mozilla/browser/base/content/browser-context.inc,v  <-- 
browser-context.inc
new revision: 1.12; previous revision: 1.11
done
Checking in base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.417; previous revision: 1.416
done