The blob conversion & sanitation upon paste doesn't work when copying & pasting from Microsoft Word for Mac 2011 as reported by the latest comment on the bug 124391. I can reproduce this issue locally as well.
<rdar://problem/36484908>
P1 due to the security/privacy implications.
Unfortunately, we can't make the image available in this case because Microsoft Word is putting only file URLs in the HTML content, and we can't grant access those files in the web content process. Supporting this would involve starting a new web content process, parse the HTML to be pasted, then UI process to add sandbox extensions for those files in the web content process in which the content is pasted.
For now, I'm going to address the bug that we're expoing file URLs in the pasted content. Since Microsoft Word for Mac 2016 has been out for two years, the recommended path for users who are using 2011 version is to upgrade to 2016 version, which does put webarchive into the pasteboard, and image pasting works as expected in the latest version of Safari.
Created attachment 331838 [details] WIP
That's interesting. I had assumed you solved bug 124391 by reading in the files referenced by the file URLs :)
Created attachment 331895 [details] Fixes the bug
Comment on attachment 331895 [details] Fixes the bug Oops, I uploaded this patch too early.
Created attachment 331897 [details] Fixes the bug
Comment on attachment 331897 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=331897&action=review > Source/WebCore/ChangeLog:27 > + (WebCore::WebContentReader::readHTML): Fixed the bug by sanitizing the markup, and stripping away file URLs. Hm...this is a bit more aggressive than just stripping away file URLs, since we're stripping away all URLs that are not one of { http:, https:, data: } by using shouldReplaceSubresourceURL as the filter. I understand that maintaining a whitelist of URLs to allow is safer than the reverse approach, though...but let's make it clear in the ChangeLog. > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:533 > + removeSubresourceURLAttributes(fragment, [] (const URL& url ) { Nit - extra space after URL& url > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:552 > + removeSubresourceURLAttributes(fragment, [] (const URL& url ) { Nit - extra space after URL& url > Source/WebCore/editing/markup.h:54 > +String sanitizeMarkup(const String&, std::optional<std::function<void(DocumentFragment&)>> fragmentSanitizer = std::nullopt); Nit - I think we generally prefer to use WTF::Function over std::function. > Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:117 > +TEST(PasteHTML, StripsHTTPURLs) Did you mean "StripsFileURLs"? > Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:131 > +TEST(PasteHTML, DoesNotStripHTTPURLsWhenCustomPasteboardDataIsDisabled) Ditto, looks like this was meant to be "DoesNotStripFileURLsWhenCustomPasteboardDataIsDisabled"
Created attachment 331940 [details] Patch for landing
(In reply to Wenson Hsieh from comment #10) > Comment on attachment 331897 [details] > Fixes the bug > > View in context: > https://bugs.webkit.org/attachment.cgi?id=331897&action=review > > > Source/WebCore/ChangeLog:27 > > + (WebCore::WebContentReader::readHTML): Fixed the bug by sanitizing the markup, and stripping away file URLs. > > Hm...this is a bit more aggressive than just stripping away file URLs, since > we're stripping away all URLs that are not one of { http:, https:, data: } > by using shouldReplaceSubresourceURL as the filter. Good point. Fixed the change log. > I understand that maintaining a whitelist of URLs to allow is safer than the > reverse approach, though...but let's make it clear in the ChangeLog. > > > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:533 > > + removeSubresourceURLAttributes(fragment, [] (const URL& url ) { > > Nit - extra space after URL& url Fixed. > > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:552 > > + removeSubresourceURLAttributes(fragment, [] (const URL& url ) { > > Nit - extra space after URL& url Fixed. > > Source/WebCore/editing/markup.h:54 > > +String sanitizeMarkup(const String&, std::optional<std::function<void(DocumentFragment&)>> fragmentSanitizer = std::nullopt); > > Nit - I think we generally prefer to use WTF::Function over std::function. Really? I thought we wanted to use std::function instead of WTF::Function going forward. Whatever. I'd use WTF::Function. > > Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:117 > > +TEST(PasteHTML, StripsHTTPURLs) > > Did you mean "StripsFileURLs"? Fixed. > > Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:131 > > +TEST(PasteHTML, DoesNotStripHTTPURLsWhenCustomPasteboardDataIsDisabled) > > Ditto, looks like this was meant to be > "DoesNotStripFileURLsWhenCustomPasteboardDataIsDisabled" Fixed.
Comment on attachment 331897 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=331897&action=review >>> Source/WebCore/editing/markup.h:54 >>> +String sanitizeMarkup(const String&, std::optional<std::function<void(DocumentFragment&)>> fragmentSanitizer = std::nullopt); >> >> Nit - I think we generally prefer to use WTF::Function over std::function. > > Really? I thought we wanted to use std::function instead of WTF::Function going forward. Whatever. I'd use WTF::Function. Just asked Fil Pizlo — it's WTF::Function.
Comment on attachment 331940 [details] Patch for landing Rejecting attachment 331940 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: urce22.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource22.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/UnifiedSource8-mm.o /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource8-mm.mm normal x86_64 objective-c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/6171151
Created attachment 331949 [details] Patch for landing
Comment on attachment 331949 [details] Patch for landing Clearing flags on attachment: 331949 Committed r227351: <https://trac.webkit.org/changeset/227351>
All reviewed patches have been landed. Closing bug.