Changeset 289541 in webkit

Timestamp:

Feb 10, 2022, 9:04:20 AM (4 years ago)

Author:

Megan Gardner

Message:

Mail attachment switched to TIFF and balloons in size after markup.
​https://bugs.webkit.org/show_bug.cgi?id=236405
rdar://87358910

Reviewed by Wenson Hsieh.

When pulling data from the webprocess to pass to the Sharing Service
for markup, we would convert it to TIFF format. This would cause files
to increase in size, and lose their original formatting, as well as outright
break PDFs. Instead we should be pulling the data from the attachment, which
already exists in the UI process and is the source of truth for that data anyways.
This allows us to give the correct type to the sharing service, and we should
take the data back as the same original type when getting the data back from
markup.

• UIProcess/API/APIAttachment.cpp:

(API::Attachment::enclosingImageData const):

• UIProcess/API/APIAttachment.h:
• UIProcess/API/Cocoa/APIAttachmentCocoa.mm:

(API::Attachment::enclosingImageNSData const):

• UIProcess/mac/WKSharingServicePickerDelegate.mm:

(-[WKSharingServicePickerDelegate sharingService:didShareItems:]):

• UIProcess/mac/WebContextMenuProxyMac.mm:

(WebKit::WebContextMenuProxyMac::setupServicesMenu):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/APIAttachment.h (modified) (1 diff)
• UIProcess/API/Cocoa/APIAttachmentCocoa.mm (modified) (1 diff)
• UIProcess/mac/WKSharingServicePickerDelegate.mm (modified) (2 diffs)
• UIProcess/mac/WebContextMenuProxyMac.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-02-10  Megan Gardner  <megan_gardner@apple.com>
+
+        Mail attachment switched to TIFF and balloons in size after markup.
+        https://bugs.webkit.org/show_bug.cgi?id=236405
+        rdar://87358910
+
+        Reviewed by Wenson Hsieh.
+
+        When pulling data from the webprocess to pass to the Sharing Service
+        for markup, we would convert it to TIFF format. This would cause files
+        to increase in size, and lose their original formatting, as well as outright
+        break PDFs. Instead we should be pulling the data from the attachment, which
+        already exists in the UI process and is the source of truth for that data anyways.
+        This allows us to give the correct type to the sharing service, and we should
+        take the data back as the same original type when getting the data back from
+        markup. 
+
+        * UIProcess/API/APIAttachment.cpp:
+        (API::Attachment::enclosingImageData const):
+        * UIProcess/API/APIAttachment.h:
+        * UIProcess/API/Cocoa/APIAttachmentCocoa.mm:
+        (API::Attachment::enclosingImageNSData const):
+        * UIProcess/mac/WKSharingServicePickerDelegate.mm:
+        (-[WKSharingServicePickerDelegate sharingService:didShareItems:]):
+        * UIProcess/mac/WebContextMenuProxyMac.mm:
+        (WebKit::WebContextMenuProxyMac::setupServicesMenu):
+
 2022-02-10  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebKit/UIProcess/API/APIAttachment.h

@@ -85 +85 @@
 
     RefPtr<WebCore::FragmentedSharedBuffer> enclosingImageData() const;
+#if PLATFORM(COCOA)
+    NSData *enclosingImageNSData() const;
+#endif
     std::optional<uint64_t> fileSizeForDisplay() const;
 

trunk/Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm

@@ -155 +155 @@
 }
 
+NSData *Attachment::enclosingImageNSData() const
+{
+    if (!m_hasEnclosingImage)
+        return nil;
+
+    auto fileWrapper = this->fileWrapper();
+
+    if (![fileWrapper isRegularFile])
+        return nil;
+
+    return fileWrapper.regularFileContents;
+}
+
 bool Attachment::isEmpty() const
 {

trunk/Source/WebKit/UIProcess/mac/WKSharingServicePickerDelegate.mm

@@ -150 +150 @@
         ALLOW_DEPRECATED_DECLARATIONS_BEGIN
         WeakPtr weakPage = _menuProxy->page();
-        [itemProvider loadDataRepresentationForTypeIdentifier:(NSString *)kUTTypeData completionHandler:[weakPage, attachmentID = _attachmentID](NSData *data, NSError *error) {
+        NSString *itemUTI = itemProvider.registeredTypeIdentifiers.firstObject;
+        [itemProvider loadDataRepresentationForTypeIdentifier:itemUTI completionHandler:[weakPage, attachmentID = _attachmentID, itemUTI](NSData *data, NSError *error) {
             RefPtr webPage = weakPage.get();
             
@@ -164 +165 @@
             
             auto attachment = wrapper(apiAttachment);
-            [attachment setData:data newContentType:String(NSPasteboardTypeTIFF)];
+            [attachment setData:data newContentType:itemUTI];
             webPage->didInvalidateDataForAttachment(*apiAttachment.get());
         }];

trunk/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm

@@ -213 +213 @@
     NSArray *items = nil;
     if (hasControlledImage) {
-        RefPtr<ShareableBitmap> image = m_context.controlledImage();
-        if (!image)
-            return;
-
-        auto cgImage = image->makeCGImage();
-        auto nsImage = adoptNS([[NSImage alloc] initWithCGImage:cgImage.get() size:image->size()]);
-
-ALLOW_DEPRECATED_DECLARATIONS_BEGIN
-        auto itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:[nsImage TIFFRepresentation] typeIdentifier:(__bridge NSString *)kUTTypeTIFF]);
-ALLOW_DEPRECATED_DECLARATIONS_END
+        auto attachment = page()->attachmentForIdentifier(m_context.controlledImageAttachmentID());
+        RetainPtr<NSItemProvider> itemProvider;
+        if (attachment)
+            itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:attachment->enclosingImageNSData() typeIdentifier:attachment->utiType()]);
+        else {
+            RefPtr<ShareableBitmap> image = m_context.controlledImage();
+            if (!image)
+                return;
+            auto cgImage = image->makeCGImage();
+            auto nsImage = adoptNS([[NSImage alloc] initWithCGImage:cgImage.get() size:image->size()]);
+
+            ALLOW_DEPRECATED_DECLARATIONS_BEGIN
+            itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:[nsImage TIFFRepresentation] typeIdentifier:(__bridge NSString *)kUTTypeTIFF]);
+            ALLOW_DEPRECATED_DECLARATIONS_END
+        }
         items = @[ itemProvider.get() ];
+        
     } else if (!m_context.controlledSelectionData().isEmpty()) {
         auto selectionData = adoptNS([[NSData alloc] initWithBytes:static_cast<const void*>(m_context.controlledSelectionData().data()) length:m_context.controlledSelectionData().size()]);