Target: 1.8 -> 1.10 due to feature and UI freeze.

-> 1.12 due to feature and UI freeze.

The mozilla bug has been fixed, I would be glad to hack on this one.
Can you give me an idea of what I need to do?

It might be as easy as adding a context menu entry which uses the EphyCommandManager with "cmd_copyImageContents".

So as far as I got so far, I have to do the magic in the ui definition and ephy-window.c, and of course add a function in src/popups-commands.c.

Thing is that I'm not sure how to use EphyCommandManager with "cmd_copyImageContents".

embed = ephy_window_get_active_embed ()
ephy_command_manager_do_command (EPHY_COMMAND_MANAGER (embed), "cmd_...")

Created attachment 70850 [details] [review]
Implements the copying but only that, it seems to be either broken or badly coded

I did the 'technically correct' implementation of this (I think) but it doesn't work, Galeon and Kazehakase doesn't have this enabled.
Maybe it's still broken?

Anyway I'm attaching this patch so if it's fixed soon it will be either applicable or available as reference.

I thought this should work, don't know why it doesn't :(

Tried again with HEAD, doesn't work :(. 
Galeon doesn't have an example of this.

Created attachment 342803 [details] [review]
Support for copying image Now it supports the copying of image to clipboard. https://bugzilla.gnome.org/show_bug.cgi?id=169747

Created attachment 342805 [details] [review]
Support for copying of images https://bugzilla.gnome.org/show_bug.cgi?id=169747

Review of attachment 342805 [details] [review]:

Thanks for this patch! It's encouraging that you were able to fix this bug so quickly. We need to do some more work to get a usable patch, though, as something has gone wrong here: your patch doesn't apply on master at all. Open the patch file and take a look: it contains only whitespace changes, and doesn't actually add any of the code that I see you've written. Unsurprisingly, it doesn't apply on master:

$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
$ git pull
Already up-to-date.
$ git bz apply 169747
Bug 169747 - support for copying images

342805 - Support for copying of images https://bugzilla.gnome.org/show_bug.cgi?id=169747

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Support for copying of images https://bugzilla.gnome.org/show_bug.cgi?id=169747
fatal: sha1 information is lacking or useless (ephy-window.c).
error: could not build fake ancestor
Patch failed at 0001 Support for copying of images https://bugzilla.gnome.org/show_bug.cgi?id=169747
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Support-for-copying-of-images-httpsbugzillagnomeor-vmpTt0.patch

I think what went wrong is you probably created multiple git commits on your branch, but attached a patch for only the most-recent commit. What you want to do is rebase all of your commits together into one new commit. Since we're only dealing with what should be one single commit here, the easiest way to do that would be to just run 'git reset master' so that you discard your new commits while leaving the files untouched so you don't lose your code changes, then make a new commit.

Anyway, fixing the patch to apply is the most important issue here, but I'll leave a couple code style comments as well:

::: ephy-window.c
@@ +1476,3 @@
                                                     "popup");
 
  if(webkit_hit_test_result_context_is_image (hit_test_result)){

Leave a space after the if and before the {

if (webkit_hit_test_result_context_is_image (hit_test_result)) {

@@ +1478,2 @@
  if(webkit_hit_test_result_context_is_image (hit_test_result)){
+     WebKitContextMenuItem *image_item;

This line is indented too far. Epiphany uses two spaces for each level of indentation. It should be aligned with the line below it.

@@ +1479,3 @@
+     WebKitContextMenuItem *image_item;
+   image_item = find_item_in_context_menu (context_menu, WEBKIT_CONTEXT_MENU_ACTION_COPY_IMAGE_TO_CLIPBOARD);
+    if(image_item) {

Again, leave a space after the if:

if (image_item) {

Also, this line is indented one character too many.

@@ +1480,3 @@
+   image_item = find_item_in_context_menu (context_menu, WEBKIT_CONTEXT_MENU_ACTION_COPY_IMAGE_TO_CLIPBOARD);
+    if(image_item) {
+      is_image=TRUE;

Leave spaces around the assignment operator:

is_image = TRUE;

@@ +1486,3 @@
 
+
+

Careful not to add extra whitespace lines in your patch.

Created attachment 342885 [details] [review]
Support for copying of images https://bugzilla.gnome.org/show_bug.cgi?id=169747

Review of attachment 342885 [details] [review]:

Hi, thanks for working on this!

::: src/ephy-window.c
@@ +1461,3 @@
   gboolean app_mode, incognito_mode;
   gboolean is_document = FALSE;
+  gboolean is_image=FALSE;

Again, please try to match the existing code style. You can see one line above that spaces are expected on both sides of the assignment operator here. :)

@@ +1480,3 @@
+   image_item = find_item_in_context_menu (context_menu, WEBKIT_CONTEXT_MENU_ACTION_COPY_IMAGE_TO_CLIPBOARD);
+   if (image_item) {
+     is_image=TRUE;

You need spaces on the sides of the assignment operator here too.

Now, the result of the hit test results tells you that it's an image, so this assignment should occur whether image_item is found in the WebKit menu or not. Perhaps there might be some situations in which WebKit might understand that you have clicked on an image, but be unable to copy the image to the clipboard for whatever reason. You don't want the other image-related context menu actions to disappear in that case.

But then, you would have nothing at all left inside this conditional, because you're not doing anything else with image_item; in particular, you're not adding it to the Epiphany context menu, which was the point of this bug report, so there's no way to actually use it to copy an image. You told me on IRC that you were able to successfully copy the image into LibreOffice, so we must have had some misunderstanding; the purpose of this context menu item is to directly copy the image bytes, not its URL. I notice now that we do have a Copy item appearing in the context menu, which was maybe confusing to you as well, but it doesn't work on images; that's bug #772106.

@@ +1483,3 @@
+     g_object_unref (image_item);
+    }
+  }

Both of these braces are indented one space too far. Notice that the existing code uses two-space indentation.

Created attachment 343296 [details] [review]
copying images https://bugzilla.gnome.org/show_bug.cgi?id=169747

Review of attachment 343296 [details] [review]:

Yes! Yes, that works, you got it. ;)

::: ephy-window.c
@@ +1478,3 @@
 
+ if (webkit_hit_test_result_context_is_image (hit_test_result)) {
+  

Please remove this extra blank line, I don't think it looks good there. Usually we use blank lines to separate code logically, but I don't think it really ever makes sense to do this on the first line inside a conditional or loop.

@@ +1480,3 @@
+  
+    is_image = TRUE;
+    copy_image_item=find_item_in_context_menu (context_menu, WEBKIT_CONTEXT_MENU_ACTION_COPY_IMAGE_TO_CLIPBOARD);

Again, our style is to leave a space on each side of the = (assignment operator)

@@ +1648,3 @@
     webkit_context_menu_append (context_menu,
                                 webkit_context_menu_item_new_separator ());
+    add_item_to_context_menu (context_menu, copy_image_item);

Now the only question is... where in the context menu should we put it? I think Save As should remain on top. It should probably be either right above or right below Copy Image Address, since it's closely related to that action. Firefox and Chrome both put it right above Copy Image Address, so let's do that.

Created attachment 343340 [details] [review]
support for copying the images to clipboard https://bugzilla.gnome.org/show_bug.cgi?id=169747

Pushed. Congrats on closing your first bug. And it had been open for nearly twelve years... wow.

Attachment 343340 [details] pushed as a5d821c - support for copying the images to clipboard https://bugzilla.gnome.org/show_bug.cgi?id=169747