- Revision
- 287494
- Author
- [email protected]
- Date
- 2021-12-31 11:46:47 -0800 (Fri, 31 Dec 2021)
Log Message
Updating the file name of attachment-backed images should automatically set the `alt` attribute
https://bugs.webkit.org/show_bug.cgi?id=234747
rdar://85899879
Reviewed by Darin Adler.
Source/WebCore:
Make a small adjustment when updating attachment attributes for attachment-backed images (i.e. when using
`-[_WKAttachmentInfo setFileWrapper:contentType:completion:]`), such that the `alt` attribute of the image is
set to the attachment element's `title`. While convenient for all internal clients of the `_WKAttachment` SPI
(i.e. Mail, Notes), this has the added benefit of preserving the attachment's file name when copying and pasting
across different web views (where a new `API::Attachment` is generated on paste), instead of defaulting to the
last path component of the blob URL corresponding to the image data in the web archive.
Test: WKAttachmentTests.CopyAndPasteImageBetweenWebViews
* editing/Editor.cpp:
(WebCore::Editor::notifyClientOfAttachmentUpdates):
* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::enclosingImageElement const):
(WebCore::HTMLAttachmentElement::updateAttributes):
(WebCore::HTMLAttachmentElement::updateEnclosingImageWithData):
(WebCore::HTMLAttachmentElement::hasEnclosingImage const): Deleted.
Replace `hasEnclosingImage()` with `enclosingImageElement()`, a helper that returns the enclosing image element
(or null if there is none). We use the latter in a couple of places above, where we modify some attributes on
the enclosing image in response to attribute and data updates.
* html/HTMLAttachmentElement.h:
Tools:
Add a new API test to exercise this scenario -- i.e. copying an attachment-backed image in one web view, pasting
it into another web view, and confirming that the pasted attachment preserves the _WKAttachment file name.
* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(-[TestWKWebView ensureAttachmentForImageElement]):
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (287493 => 287494)
--- trunk/Source/WebCore/ChangeLog 2021-12-31 14:21:02 UTC (rev 287493)
+++ trunk/Source/WebCore/ChangeLog 2021-12-31 19:46:47 UTC (rev 287494)
@@ -1,3 +1,34 @@
+2021-12-31 Wenson Hsieh <[email protected]>
+
+ Updating the file name of attachment-backed images should automatically set the `alt` attribute
+ https://bugs.webkit.org/show_bug.cgi?id=234747
+ rdar://85899879
+
+ Reviewed by Darin Adler.
+
+ Make a small adjustment when updating attachment attributes for attachment-backed images (i.e. when using
+ `-[_WKAttachmentInfo setFileWrapper:contentType:completion:]`), such that the `alt` attribute of the image is
+ set to the attachment element's `title`. While convenient for all internal clients of the `_WKAttachment` SPI
+ (i.e. Mail, Notes), this has the added benefit of preserving the attachment's file name when copying and pasting
+ across different web views (where a new `API::Attachment` is generated on paste), instead of defaulting to the
+ last path component of the blob URL corresponding to the image data in the web archive.
+
+ Test: WKAttachmentTests.CopyAndPasteImageBetweenWebViews
+
+ * editing/Editor.cpp:
+ (WebCore::Editor::notifyClientOfAttachmentUpdates):
+ * html/HTMLAttachmentElement.cpp:
+ (WebCore::HTMLAttachmentElement::enclosingImageElement const):
+ (WebCore::HTMLAttachmentElement::updateAttributes):
+ (WebCore::HTMLAttachmentElement::updateEnclosingImageWithData):
+ (WebCore::HTMLAttachmentElement::hasEnclosingImage const): Deleted.
+
+ Replace `hasEnclosingImage()` with `enclosingImageElement()`, a helper that returns the enclosing image element
+ (or null if there is none). We use the latter in a couple of places above, where we modify some attributes on
+ the enclosing image in response to attribute and data updates.
+
+ * html/HTMLAttachmentElement.h:
+
2021-12-31 Alan Bujtas <[email protected]>
[LFC][IFC] Take grapheme clusters into account when keeping the first "character" on the line
Modified: trunk/Source/WebCore/editing/Editor.cpp (287493 => 287494)
--- trunk/Source/WebCore/editing/Editor.cpp 2021-12-31 14:21:02 UTC (rev 287493)
+++ trunk/Source/WebCore/editing/Editor.cpp 2021-12-31 19:46:47 UTC (rev 287494)
@@ -4231,7 +4231,7 @@
for (auto& identifier : insertedAttachmentIdentifiers) {
if (auto attachment = m_document.attachmentForIdentifier(identifier))
- client()->didInsertAttachmentWithIdentifier(identifier, attachment->attributeWithoutSynchronization(HTMLNames::srcAttr), attachment->hasEnclosingImage());
+ client()->didInsertAttachmentWithIdentifier(identifier, attachment->attributeWithoutSynchronization(HTMLNames::srcAttr), attachment->enclosingImageElement());
else
ASSERT_NOT_REACHED();
}
Modified: trunk/Source/WebCore/html/HTMLAttachmentElement.cpp (287493 => 287494)
--- trunk/Source/WebCore/html/HTMLAttachmentElement.cpp 2021-12-31 14:21:02 UTC (rev 287493)
+++ trunk/Source/WebCore/html/HTMLAttachmentElement.cpp 2021-12-31 19:46:47 UTC (rev 287494)
@@ -152,9 +152,12 @@
return m_uniqueIdentifier;
}
-bool HTMLAttachmentElement::hasEnclosingImage() const
+RefPtr<HTMLImageElement> HTMLAttachmentElement::enclosingImageElement() const
{
- return is<HTMLImageElement>(shadowHost());
+ if (auto hostElement = shadowHost(); is<HTMLImageElement>(hostElement))
+ return downcast<HTMLImageElement>(hostElement);
+
+ return { };
}
void HTMLAttachmentElement::parseAttribute(const QualifiedName& name, const AtomString& value)
@@ -203,10 +206,15 @@
void HTMLAttachmentElement::updateAttributes(std::optional<uint64_t>&& newFileSize, const String& newContentType, const String& newFilename)
{
- if (!newFilename.isNull())
+ if (!newFilename.isNull()) {
+ if (RefPtr enclosingImage = enclosingImageElement())
+ enclosingImage->setAttributeWithoutSynchronization(HTMLNames::altAttr, newFilename);
setAttributeWithoutSynchronization(HTMLNames::titleAttr, newFilename);
- else
+ } else {
+ if (RefPtr enclosingImage = enclosingImageElement())
+ enclosingImage->removeAttribute(HTMLNames::altAttr);
removeAttribute(HTMLNames::titleAttr);
+ }
if (!newContentType.isNull())
setAttributeWithoutSynchronization(HTMLNames::typeAttr, newContentType);
@@ -229,10 +237,13 @@
void HTMLAttachmentElement::updateEnclosingImageWithData(const String& contentType, Ref<FragmentedSharedBuffer>&& buffer)
{
- auto* hostElement = shadowHost();
- if (!is<HTMLImageElement>(hostElement) || !buffer->size())
+ if (buffer->isEmpty())
return;
+ RefPtr enclosingImage = enclosingImageElement();
+ if (!enclosingImage)
+ return;
+
String mimeType = contentType;
#if PLATFORM(COCOA)
if (isDeclaredUTI(contentType))
@@ -242,7 +253,7 @@
if (!mimeTypeIsSuitableForInlineImageAttachment(mimeType))
return;
- hostElement->setAttributeWithoutSynchronization(HTMLNames::srcAttr, DOMURL::createObjectURL(document(), Blob::create(&document(), buffer->extractData(), mimeType)));
+ enclosingImage->setAttributeWithoutSynchronization(HTMLNames::srcAttr, DOMURL::createObjectURL(document(), Blob::create(&document(), buffer->extractData(), mimeType)));
}
void HTMLAttachmentElement::updateThumbnail(const RefPtr<Image>& thumbnail)
Modified: trunk/Source/WebCore/html/HTMLAttachmentElement.h (287493 => 287494)
--- trunk/Source/WebCore/html/HTMLAttachmentElement.h 2021-12-31 14:21:02 UTC (rev 287493)
+++ trunk/Source/WebCore/html/HTMLAttachmentElement.h 2021-12-31 19:46:47 UTC (rev 287494)
@@ -64,7 +64,7 @@
void removedFromAncestor(RemovalType, ContainerNode&) final;
const String& ensureUniqueIdentifier();
- bool hasEnclosingImage() const;
+ RefPtr<HTMLImageElement> enclosingImageElement() const;
WEBCORE_EXPORT String attachmentTitle() const;
String attachmentTitleForDisplay() const;
Modified: trunk/Tools/ChangeLog (287493 => 287494)
--- trunk/Tools/ChangeLog 2021-12-31 14:21:02 UTC (rev 287493)
+++ trunk/Tools/ChangeLog 2021-12-31 19:46:47 UTC (rev 287494)
@@ -1,3 +1,18 @@
+2021-12-31 Wenson Hsieh <[email protected]>
+
+ Updating the file name of attachment-backed images should automatically set the `alt` attribute
+ https://bugs.webkit.org/show_bug.cgi?id=234747
+ rdar://85899879
+
+ Reviewed by Darin Adler.
+
+ Add a new API test to exercise this scenario -- i.e. copying an attachment-backed image in one web view, pasting
+ it into another web view, and confirming that the pasted attachment preserves the _WKAttachment file name.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+ (-[TestWKWebView ensureAttachmentForImageElement]):
+ (TestWebKitAPI::TEST):
+
2021-12-30 Jean-Yves Avenard <[email protected]>
SharedBuffer::takeData() is still dangerous
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm (287493 => 287494)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm 2021-12-31 14:21:02 UTC (rev 287493)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm 2021-12-31 19:46:47 UTC (rev 287494)
@@ -365,6 +365,24 @@
observer.expectAttachmentUpdates(removed, inserted);
}
+- (NSString *)ensureAttachmentForImageElement
+{
+ __block bool doneWaitingForAttachmentInsertion = false;
+ [self performAfterReceivingMessage:@"inserted" action:^{
+ doneWaitingForAttachmentInsertion = true;
+ }];
+
+ const char *scriptForEnsuringAttachmentIdentifier = \
+ "const identifier = HTMLAttachmentElement.getAttachmentIdentifier(document.querySelector('img'));"
+ "setTimeout(() => webkit.messageHandlers.testHandler.postMessage('inserted'), 0);"
+ "identifier";
+
+ RetainPtr attachmentIdentifier = [self stringByEvaluatingJavaScript:@(scriptForEnsuringAttachmentIdentifier)];
+ TestWebKitAPI::Util::run(&doneWaitingForAttachmentInsertion);
+
+ return attachmentIdentifier.autorelease();
+}
+
@end
@implementation NSData (AttachmentTesting)
@@ -1407,22 +1425,11 @@
auto webView = webViewForTestingAttachments();
[webView _synchronouslyExecuteEditCommand:@"InsertHTML" argument:@"<img></img>"];
- __block bool doneWaitingForAttachmentInsertion = false;
- [webView performAfterReceivingMessage:@"inserted" action:^{
- doneWaitingForAttachmentInsertion = true;
- }];
-
- const char *scriptForEnsuringAttachmentIdentifier = \
- "const identifier = HTMLAttachmentElement.getAttachmentIdentifier(document.querySelector('img'));"
- "setTimeout(() => webkit.messageHandlers.testHandler.postMessage('inserted'), 0);"
- "identifier";
-
ObserveAttachmentUpdatesForScope observer(webView.get());
- NSString *attachmentIdentifier = [webView stringByEvaluatingJavaScript:@(scriptForEnsuringAttachmentIdentifier)];
+ NSString *attachmentIdentifier = [webView ensureAttachmentForImageElement];
auto attachment = retainPtr([webView _attachmentForIdentifier:attachmentIdentifier]);
EXPECT_WK_STREQ(attachmentIdentifier, [attachment uniqueIdentifier]);
EXPECT_WK_STREQ(attachmentIdentifier, [webView stringByEvaluatingJavaScript:@"document.querySelector('img').attachmentIdentifier"]);
- Util::run(&doneWaitingForAttachmentInsertion);
observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
auto firstImage = adoptNS([[NSFileWrapper alloc] initWithURL:testImageFileURL() options:0 error:nil]);
@@ -1533,6 +1540,37 @@
EXPECT_WK_STREQ("application/zip", pastedArchiveInfo.contentType);
}
+TEST(WKAttachmentTests, CopyAndPasteImageBetweenWebViews)
+{
+ auto image = adoptNS([[NSFileWrapper alloc] initWithURL:testImageFileURL() options:0 error:nil]);
+ [image setPreferredFilename:@"icon.png"];
+ RetainPtr originalData = [image regularFileContents];
+
+ @autoreleasepool {
+ auto sourceView = webViewForTestingAttachments();
+ [sourceView _synchronouslyExecuteEditCommand:@"InsertHTML" argument:@"<img></img>"];
+
+ auto attachment = [sourceView _attachmentForIdentifier:[sourceView ensureAttachmentForImageElement]];
+ [attachment synchronouslySetFileWrapper:image.get() newContentType:@"image/png" error:nil];
+ [sourceView waitForImageElementSizeToBecome:CGSizeMake(215, 174)];
+
+ EXPECT_WK_STREQ("icon.png", [sourceView valueOfAttribute:@"alt" forQuerySelector:@"img"]);
+ [sourceView selectAll:nil];
+ [sourceView _synchronouslyExecuteEditCommand:@"Copy" argument:nil];
+ [sourceView waitForNextPresentationUpdate];
+ }
+
+ auto destinationView = webViewForTestingAttachments();
+ ObserveAttachmentUpdatesForScope observer(destinationView.get());
+ [destinationView _synchronouslyExecuteEditCommand:@"Paste" argument:nil];
+ EXPECT_EQ(1U, observer.observer().inserted.count);
+
+ auto pastedAttachmentInfo = [observer.observer().inserted.firstObject info];
+ EXPECT_WK_STREQ("image/png", pastedAttachmentInfo.contentType);
+ EXPECT_WK_STREQ("icon.png", pastedAttachmentInfo.name);
+ EXPECT_TRUE([originalData isEqualToData:pastedAttachmentInfo.data]);
+}
+
TEST(WKAttachmentTests, AttachmentIdentifierOfClonedAttachment)
{
auto webView = webViewForTestingAttachments();
