Bug 158590 – [Cocoa] REGRESSION (r198177): Cannot paste an image when the pasteboard format is MIME type

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Enrica Casucci]

This regressed with http://trac.webkit.org/changeset/198177 where EditorIOS.mm and EditorMac.mm where modified inadvertently, postponing adding the resources to the document loader.

rdar://problem/25471371

[Enrica Casucci]

Created attachment 280962 [details]
Patch

[Darin Adler]

Comment on attachment 280962 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=280962&action=review

Sorry about my part in letting this get broken!

> Source/WebCore/editing/ios/EditorIOS.mm:583
> +    String resourceUrl = [URL isFileURL] ? [URL absoluteString] : resource->url();

Should name this resourceURL, with capital letters for the acronym.

> Source/WebCore/editing/ios/EditorIOS.mm:588
> +    auto imageElement = m_frame.document()->createElement(HTMLNames::imgTag, false);

This is an unnecessarily roundabout way to create an image element. Should instead just write:

    auto imageElement = HTMLImageElement::create(*m_frame.document());

> Source/WebCore/editing/ios/EditorIOS.mm:589
> +    imageElement->setAttribute(HTMLNames::srcAttr, resourceUrl);

For slightly better performance this can use setAttributeWithoutSynchronization instead of setAttribute.

> Source/WebCore/editing/mac/EditorMac.mm:636
> +    String resourceUrl = resource->url().string();

Same comment as above.

> Source/WebCore/editing/mac/EditorMac.mm:641
> +    auto imageElement = document().createElement(HTMLNames::imgTag, false);
> +    imageElement->setAttribute(HTMLNames::srcAttr, resourceUrl);

Same comments as above.

But also, can we (at least eventually) find a way to share more of this copied and pasted code between iOS and Mac?

> Source/WebCore/page/Settings.h:147
> +    WEBCORE_EXPORT void setPreferMimeTypeForImages(bool);
> +    bool preferMimeTypeForImages() const { return m_preferMimeTypeForImages; }

I think it should be MIMEType rather than MimeType.

> Source/WebCore/page/Settings.h:326
> +    bool m_preferMimeTypeForImages : 1;

Ditto.

> Source/WebCore/testing/InternalSettings.idl:66
> +    [RaisesException] void setPreferMimeTypeForImages(boolean preferMimeTypeForImage);

I think it should be MIMEType rather than MimeType.

> LayoutTests/ChangeLog:9
> +        * editing/pasteboard/image-in-iframe-expected.txt: Added.

Can we make this a reference test instead of a render-tree-dumping test?

[Enrica Casucci]

Thanks for the review. I've addressed all your comments.
I tried making a ref test, but I keep getting problems with the caret image.
I'd rather leave this as render tree test.

[Enrica Casucci]

Committed revision 201956.