Title: [287547] trunk
Revision
287547
Author
wenson_hs...@apple.com
Date
2022-01-03 10:27:07 -0800 (Mon, 03 Jan 2022)

Log Message

Undownloaded iCloud Photos are inserted as broken images when attachment element is enabled
https://bugs.webkit.org/show_bug.cgi?id=234803
rdar://82318259

Reviewed by Darin Adler.

Source/WebCore:

Currently, when inserting file paths via paste or drop that correspond to undownloaded files in iCloud Drive,
we show a generic empty file as the file preview, with "Zero Bytes" as the attachment subtitle when the
attachment element is enabled (i.e., Mail compose when using WebKit2). Even worse, for undownloaded image files,
we'll attempt to insert them as broken images.

Mail handles this scenario by detecting that we've dropped an undownloaded attachment, downloads the attachment,
and eventually updates the attachment element with the downloaded data. In the meantime, however, it's not ideal
for WebKit to represent the attachment that is still being downloaded as a broken image or empty file.

To address this, make a slight adjustment to detect that the file we're inserting is not locally available and
render it as an attachment element with progress="0". The client is then expected to update this progress as the
download is taking place, and update the attachment data once it's complete.

Test: WKAttachmentTestsMac.InsertNonExistentImageFileAsAttachment

* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::attachmentForFilePath):

Tools:

Add a new API test to exercise the change. See WebCore/ChangeLog for more information.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(nonexistentFilePath):
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287546 => 287547)


--- trunk/Source/WebCore/ChangeLog	2022-01-03 17:46:10 UTC (rev 287546)
+++ trunk/Source/WebCore/ChangeLog	2022-01-03 18:27:07 UTC (rev 287547)
@@ -1,3 +1,29 @@
+2022-01-03  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Undownloaded iCloud Photos are inserted as broken images when attachment element is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=234803
+        rdar://82318259
+
+        Reviewed by Darin Adler.
+
+        Currently, when inserting file paths via paste or drop that correspond to undownloaded files in iCloud Drive,
+        we show a generic empty file as the file preview, with "Zero Bytes" as the attachment subtitle when the
+        attachment element is enabled (i.e., Mail compose when using WebKit2). Even worse, for undownloaded image files,
+        we'll attempt to insert them as broken images.
+
+        Mail handles this scenario by detecting that we've dropped an undownloaded attachment, downloads the attachment,
+        and eventually updates the attachment element with the downloaded data. In the meantime, however, it's not ideal
+        for WebKit to represent the attachment that is still being downloaded as a broken image or empty file.
+
+        To address this, make a slight adjustment to detect that the file we're inserting is not locally available and
+        render it as an attachment element with progress="0". The client is then expected to update this progress as the
+        download is taking place, and update the attachment data once it's complete.
+
+        Test: WKAttachmentTestsMac.InsertNonExistentImageFileAsAttachment
+
+        * editing/cocoa/WebContentReaderCocoa.mm:
+        (WebCore::attachmentForFilePath):
+
 2022-01-03  Antti Koivisto  <an...@apple.com>
 
         PseudoClassChangeInvalidation should allow multiple pseudo classes as argument

Modified: trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm (287546 => 287547)


--- trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2022-01-03 17:46:10 UTC (rev 287546)
+++ trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2022-01-03 18:27:07 UTC (rev 287547)
@@ -711,7 +711,8 @@
         return attachment;
     }
 
-    bool isDirectory = FileSystem::fileTypeFollowingSymlinks(path) == FileSystem::FileType::Directory;
+    auto fileType = FileSystem::fileTypeFollowingSymlinks(path);
+    bool isDirectory = fileType == FileSystem::FileType::Directory;
     String contentType = typeForAttachmentElement(explicitContentType);
     if (contentType.isEmpty()) {
 ALLOW_DEPRECATED_DECLARATIONS_BEGIN
@@ -726,12 +727,14 @@
     }
 
     std::optional<uint64_t> fileSizeForDisplay;
-    if (!isDirectory)
+    if (fileType && !isDirectory)
         fileSizeForDisplay = FileSystem::fileSize(path).value_or(0);
 
     frame.editor().registerAttachmentIdentifier(attachment->ensureUniqueIdentifier(), contentType, path);
 
-    if (contentTypeIsSuitableForInlineImageRepresentation(contentType)) {
+    if (!fileType)
+        attachment->setAttributeWithoutSynchronization(HTMLNames::progressAttr, AtomString::number(0));
+    else if (contentTypeIsSuitableForInlineImageRepresentation(contentType)) {
         auto image = HTMLImageElement::create(document);
         image->setAttributeWithoutSynchronization(HTMLNames::srcAttr, DOMURL::createObjectURL(document, File::create(document.ptr(), path)));
         image->setAttachmentElement(WTFMove(attachment));

Modified: trunk/Tools/ChangeLog (287546 => 287547)


--- trunk/Tools/ChangeLog	2022-01-03 17:46:10 UTC (rev 287546)
+++ trunk/Tools/ChangeLog	2022-01-03 18:27:07 UTC (rev 287547)
@@ -1,3 +1,17 @@
+2022-01-03  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Undownloaded iCloud Photos are inserted as broken images when attachment element is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=234803
+        rdar://82318259
+
+        Reviewed by Darin Adler.
+
+        Add a new API test to exercise the change. See WebCore/ChangeLog for more information.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (nonexistentFilePath):
+        (TestWebKitAPI::TEST):
+
 2022-01-03  Martin Robinson  <mrobin...@webkit.org>
 
         A manual test was imported with the WPT css-transforms test suite

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm (287546 => 287547)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm	2022-01-03 17:46:10 UTC (rev 287546)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm	2022-01-03 18:27:07 UTC (rev 287547)
@@ -511,8 +511,14 @@
     return true;
 }
 
-#endif
+static NSString *nonexistentFilePath(NSString *extension)
+{
+    auto nonexistentFileName = [NSString stringWithFormat:@"%@.%@", NSUUID.UUID.UUIDString, extension];
+    return [NSTemporaryDirectory() stringByAppendingPathComponent:nonexistentFileName];
+}
 
+#endif // PLATFORM(MAC)
+
 #if PLATFORM(IOS_FAMILY)
 
 typedef void(^ItemProviderDataLoadHandler)(NSData *, NSError *);
@@ -1672,6 +1678,32 @@
     }
 }
 
+TEST(WKAttachmentTestsMac, InsertNonExistentImageFileAsAttachment)
+{
+    NSPasteboard *pasteboard = [NSPasteboard generalPasteboard];
+    [pasteboard clearContents];
+    [pasteboard declareTypes:@[NSFilenamesPboardType] owner:nil];
+
+    RetainPtr filePath = nonexistentFilePath(@"jpg");
+    [pasteboard setPropertyList:@[filePath.get()] forType:NSFilenamesPboardType];
+
+    RetainPtr<NSArray<_WKAttachment *>> insertedAttachments;
+    auto webView = webViewForTestingAttachments();
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [webView _synchronouslyExecuteEditCommand:@"Paste" argument:nil];
+        insertedAttachments = [observer.observer() inserted];
+        EXPECT_EQ(1U, [insertedAttachments count]);
+    }
+
+    [webView expectElementCount:1 querySelector:@"ATTACHMENT"];
+    [webView expectElementCount:0 querySelector:@"IMG"];
+    EXPECT_WK_STREQ("0", [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').getAttribute('progress')"]);
+    EXPECT_WK_STREQ([filePath lastPathComponent], [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').getAttribute('title')"]);
+    EXPECT_WK_STREQ("image/jpeg", [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').getAttribute('type')"]);
+    EXPECT_FALSE([[webView objectByEvaluatingJavaScript:@"document.querySelector('attachment').hasAttribute('subtitle')"] boolValue]);
+}
+
 TEST(WKAttachmentTestsMac, InsertDroppedFilePromisesAsAttachments)
 {
     auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to