Right now, WebContentReader::readWebArchive for Mac supports having image MIME type but this doesn't seem be ever used in practice. We have an explicit code path to handle image files/content in the pasteboard as well. Remove this code so that iOS & Mac can share code.
Created attachment 320713 [details] Cleanup
Created attachment 320722 [details] Added missing includes
Comment on attachment 320722 [details] Added missing includes View in context: https://bugs.webkit.org/attachment.cgi?id=320722&action=review > Source/WebCore/editing/WebContentReader.h:55 > +#if PLATFORM(COCOA) > void addFragment(RefPtr<DocumentFragment>&&); > #endif I still think this could be available to all platforms. There really isn't anything cocoa specific about it. > Source/WebCore/editing/WebContentReader.h:59 > bool readWebArchive(SharedBuffer*) override; Perhaps this should be in a #if ENABLE(LEGACY_WEB_ARCHIVE).
Comment on attachment 320722 [details] Added missing includes View in context: https://bugs.webkit.org/attachment.cgi?id=320722&action=review >> Source/WebCore/editing/WebContentReader.h:55 >> #endif > > I still think this could be available to all platforms. There really isn't anything cocoa specific about it. Yeah, let's just do that. >> Source/WebCore/editing/WebContentReader.h:59 >> bool readWebArchive(SharedBuffer*) override; > > Perhaps this should be in a #if ENABLE(LEGACY_WEB_ARCHIVE). Perhaps. I think Darin's intention was implementing these functions on all platforms. I'm gonna leave them alone for now.
Created attachment 320736 [details] Refactoring
Created attachment 320742 [details] Fixed API tests
Comment on attachment 320742 [details] Fixed API tests View in context: https://bugs.webkit.org/attachment.cgi?id=320742&action=review > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:119 > + const String& type = mainResource->mimeType(); auto? > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:125 > + if (DocumentLoader* loader = frame.loader().documentLoader()) auto? > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:128 > + String markupString = String::fromUTF8(mainResource->data().data(), mainResource->data().size()); auto? > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:164 > + addFragment(createFragmentFromText(context, [text precomposedStringWithCanonicalMapping])); Should use dot notation since precomposedStringWithCanonicalMapping is a property. > Source/WebCore/editing/ios/WebContentReaderIOS.mm:102 > - auto newFragment = frame.document()->createDocumentFragment(); > + Ref<DocumentFragment> newFragment = frame.document()->createDocumentFragment(); Why remove the auto?
Created attachment 320816 [details] Patch for landing
Thanks for the review. Fixed them all.
Comment on attachment 320816 [details] Patch for landing Wait for EWS.
Created attachment 320826 [details] Patch for landing
Comment on attachment 320826 [details] Patch for landing Wait for EWS.
(In reply to Sam Weinig from comment #7) > Comment on attachment 320742 [details] > Fixed API tests > > View in context: > https://bugs.webkit.org/attachment.cgi?id=320742&action=review > > > Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:164 > > + addFragment(createFragmentFromText(context, [text precomposedStringWithCanonicalMapping])); > > Should use dot notation since precomposedStringWithCanonicalMapping is a > property. Ugh... apparently text is a String instances. I'd leave square brackets for now instead of explicitly casing it to NSString.
Created attachment 320830 [details] Patch for landing
Comment on attachment 320830 [details] Patch for landing Clearing flags on attachment: 320830 Committed r222062: <http://trac.webkit.org/changeset/222062>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34693428>