Mail attachment switched to TIFF and baloons in size after markup.
Created attachment 451446 [details] Patch
Comment on attachment 451446 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451446&action=review > Source/WebKit/ChangeLog:3 > + Mail attachment switched to TIFF and baloons in size after markup. "balloons" > Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm:157 > +NSData* Attachment::enclosingImageNSData() const all your nsdata stars are on the wrong side > Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm:160 > + return NULL; nil in a variety of places > Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm:216 > + auto attachment = page()->attachmentForIdentifier(m_context.controlledImageAttachmentID()); > + auto itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:attachment->enclosingImageNSData() typeIdentifier:attachment->utiType()]); I think there might be cases where we don't have an API attachment backing a controlled image? but I'm not sure. Maybe fall back to the old thing if you fail to get the attachment? Or maybe Wenson will tell you I'm wrong.
Comment on attachment 451446 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451446&action=review > Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm:171 > + NSData *data = [fileWrapper regularFileContents]; > + if (!data) > + return NULL; > + > + return data; Nit - I would just write this `return fileWrapper.regularFileContents;` >> Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm:216 >> + auto itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:attachment->enclosingImageNSData() typeIdentifier:attachment->utiType()]); > > I think there might be cases where we don't have an API attachment backing a controlled image? but I'm not sure. Maybe fall back to the old thing if you fail to get the attachment? Or maybe Wenson will tell you I'm wrong. My understanding was that we would only allow mark up on images that are attachment-backed (i.e., backed by <object> in legacy WK1 compose). I did verify that in legacy Mail compose, remote images cannot be marked up (by replying to an email with remote images). But at any rate, using the image bitmap data as a fallback seems sensible!
Created attachment 451455 [details] Patch
Comment on attachment 451455 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451455&action=review > Source/WebKit/UIProcess/API/APIAttachment.cpp:106 > +NSData* Attachment::enclosingImageNSData() const Nit - * is still on the wrong side here. > Source/WebKit/UIProcess/API/APIAttachment.cpp:108 > + return nullptr; Nit - `return nil;`
Created attachment 451487 [details] Patch for landing
The Windows build failures in https://ews-build.webkit.org/#/builders/12/builds/72990 seem legit.
Created attachment 451490 [details] Patch
Created attachment 451540 [details] Patch for landing
Committed r289541 (247072@main): <https://commits.webkit.org/247072@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 451540 [details].
<rdar://problem/88758362>