Bug 12959 – REGRESSION: Edit -> Copy not enabled on standalone images

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Matt Lilek]

Load a standalone image such as http://webkit.org/images/icon-gold.png.  The Edit -> Copy menu item isn't enabled.  This is a regression from Safari 2.0.4.

[Mark Rowe (bdash)]

<rdar://problem/5045713>

[John Sullivan]

Actually this was already in radar as<rdar://problem/4044786>. This was caused by the shift to use HTML pages for standalone images.

[Andrew Wellington]

Created attachment 13884 [details]
Proposed patch

This patch adds a new pasteboard method to copy an image based on the image node and URL and then calls that method when copying in an ImageDocument.

Comment on attachment 13884 [details]
Proposed patch

Standalone image documents are mutable (via JS) after they're opened. This means that this can deref a 0 pointer or set image to 0:

+        Node *image = doc->body()->firstChild();

It also means that renderer() here may be 0 or not a RenderImage:

+    RenderImage* renderer = static_cast<RenderImage*>(imageNode->renderer());

Please don't add another image:
+        * editing/pasteboard/resources/icon-gold.png: Added.

You can use editing/resources/abe.jpg.

\ No newline at end of file

Please add it.

Stars should be next to C++ types:
+        Document *doc = m_frame->document();
+        Node *image = doc->body()->firstChild();

+    void writeImage(Node *imageNode, const KURL& url);

+void Pasteboard::writeImage(Node *imageNode, const KURL& url)

But next to Obj-C object variables' names:
+    NSArray* types = writableTypesForImage();

r- because in its current form, the patch makes it possible for a website to crash the browser.

[Andrew Wellington]

Created attachment 13901 [details]
Proposed Patch 2

Update based on comments from Mitz

[Andrew Wellington]

Comment on attachment 13901 [details]
Proposed Patch 2

Submitting an updated patch

[Andrew Wellington]

Created attachment 13906 [details]
Proposed patch 3

A few more changes after conversation with Mitz

Comment on attachment 13906 [details]
Proposed patch 3

Looks good to me, but I think someone else needs to review it.

[Justin Garcia]

Comment on attachment 13906 [details]
Proposed patch 3

I think some code sharing between Pasteboard::writeImage(const HitTestResult&) and writeImage(Node*, KURL) would have been possible, also you probably could have created a public getter for ImageDocument's m_imageElement instead of getting its body's first child, but r=me.

[Andrew Wellington]

Committed in r20802