Summary: | [Cocoa] REGRESSION (r198177): Cannot paste an image when the pasteboard format is MIME type | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Enrica Casucci <enrica> | ||||
Component: | HTML Editing | Assignee: | Enrica Casucci <enrica> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | beidson, darin | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Enrica Casucci
2016-06-09 15:55:54 PDT
Created attachment 280962 [details]
Patch
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? 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. Committed revision 201956. |