- Revision
- 226097
- Author
- wenson_hs...@apple.com
- Date
- 2017-12-18 21:38:47 -0800 (Mon, 18 Dec 2017)
Log Message
[Attachment Support] The 'webkitattachmentbloburl' attribute should not persist after markup serialization
https://bugs.webkit.org/show_bug.cgi?id=180924
<rdar://problem/36099093>
Reviewed by Tim Horton.
Source/WebCore:
Work towards dragging Blob-backed attachment elements as files on iOS and Mac. It doesn't make sense for the
attachment blob URL to stick around on the element after markup serialization, so this patch removes logic that
eagerly sets the blob URL upon setting an attachment's File. Instead, we just append this attribute when
generating markup.
This patch also augments existing WKAttachmentTests to ensure that these attributes are not present.
* editing/markup.cpp:
(WebCore::StyledMarkupAccumulator::appendCustomAttributes):
(WebCore::createFragmentFromMarkup):
* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::setFile):
* rendering/HitTestResult.cpp:
Fixes a related issue where an attachment is backed by Blob data (and not a file path) would specify "file:///"
as its attachment file path in DragController when starting a drag. Instead, if there is no file path, fall back
to the blob URL.
This will be tested in a future patch once a WK2 dragging simulator for Mac is implemented, and support for
dragging out Blob-backed attachments as (platform) files is implemented.
(WebCore::HitTestResult::absoluteAttachmentURL const):
Tools:
Tweaks some existing tests to check that temporary attachment serialization attributes don't stick around on the
attachment elements.
* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(-[TestWKWebView hasAttribute:forQuerySelector:]):
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (226096 => 226097)
--- trunk/Source/WebCore/ChangeLog 2017-12-19 05:08:52 UTC (rev 226096)
+++ trunk/Source/WebCore/ChangeLog 2017-12-19 05:38:47 UTC (rev 226097)
@@ -1,3 +1,34 @@
+2017-12-18 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [Attachment Support] The 'webkitattachmentbloburl' attribute should not persist after markup serialization
+ https://bugs.webkit.org/show_bug.cgi?id=180924
+ <rdar://problem/36099093>
+
+ Reviewed by Tim Horton.
+
+ Work towards dragging Blob-backed attachment elements as files on iOS and Mac. It doesn't make sense for the
+ attachment blob URL to stick around on the element after markup serialization, so this patch removes logic that
+ eagerly sets the blob URL upon setting an attachment's File. Instead, we just append this attribute when
+ generating markup.
+
+ This patch also augments existing WKAttachmentTests to ensure that these attributes are not present.
+
+ * editing/markup.cpp:
+ (WebCore::StyledMarkupAccumulator::appendCustomAttributes):
+ (WebCore::createFragmentFromMarkup):
+ * html/HTMLAttachmentElement.cpp:
+ (WebCore::HTMLAttachmentElement::setFile):
+ * rendering/HitTestResult.cpp:
+
+ Fixes a related issue where an attachment is backed by Blob data (and not a file path) would specify "file:///"
+ as its attachment file path in DragController when starting a drag. Instead, if there is no file path, fall back
+ to the blob URL.
+
+ This will be tested in a future patch once a WK2 dragging simulator for Mac is implemented, and support for
+ dragging out Blob-backed attachments as (platform) files is implemented.
+
+ (WebCore::HitTestResult::absoluteAttachmentURL const):
+
2017-12-18 Chris Dumez <cdu...@apple.com>
Default scope used when registering a service worker is wrong
Modified: trunk/Source/WebCore/editing/markup.cpp (226096 => 226097)
--- trunk/Source/WebCore/editing/markup.cpp 2017-12-19 05:08:52 UTC (rev 226096)
+++ trunk/Source/WebCore/editing/markup.cpp 2017-12-19 05:38:47 UTC (rev 226097)
@@ -371,7 +371,7 @@
return nodeValue;
}
-void StyledMarkupAccumulator::appendCustomAttributes(StringBuilder& out, const Element&element, Namespaces* namespaces)
+void StyledMarkupAccumulator::appendCustomAttributes(StringBuilder& out, const Element& element, Namespaces* namespaces)
{
#if ENABLE(ATTACHMENT_ELEMENT)
if (!is<HTMLAttachmentElement>(element))
@@ -378,8 +378,12 @@
return;
const HTMLAttachmentElement& attachment = downcast<HTMLAttachmentElement>(element);
- if (attachment.file())
- appendAttribute(out, element, Attribute(webkitattachmentpathAttr, attachment.file()->path()), namespaces);
+ if (auto* file = attachment.file()) {
+ // These attributes are only intended for File deserialization, and are removed from the generated attachment
+ // element after we've deserialized and set its backing File.
+ appendAttribute(out, element, { webkitattachmentpathAttr, file->path() }, namespaces);
+ appendAttribute(out, element, { webkitattachmentbloburlAttr, { file->url() }}, namespaces);
+ }
#else
UNUSED_PARAM(out);
UNUSED_PARAM(element);
@@ -764,11 +768,13 @@
for (auto& attachment : attachments) {
auto attachmentPath = attachment->attachmentPath();
- if (attachmentPath.isEmpty())
- attachment->setFile(File::deserialize({ }, attachment->blobURL(), attachment->attachmentType(), attachment->attachmentTitle()));
- else
+ auto blobURL = attachment->blobURL();
+ if (!attachmentPath.isEmpty())
attachment->setFile(File::create(attachmentPath));
+ else if (!blobURL.isEmpty())
+ attachment->setFile(File::deserialize({ }, blobURL, attachment->attachmentType(), attachment->attachmentTitle()));
attachment->removeAttribute(webkitattachmentpathAttr);
+ attachment->removeAttribute(webkitattachmentbloburlAttr);
}
#endif
if (!baseURL.isEmpty() && baseURL != blankURL() && baseURL != document.baseURL())
Modified: trunk/Source/WebCore/html/HTMLAttachmentElement.cpp (226096 => 226097)
--- trunk/Source/WebCore/html/HTMLAttachmentElement.cpp 2017-12-19 05:08:52 UTC (rev 226096)
+++ trunk/Source/WebCore/html/HTMLAttachmentElement.cpp 2017-12-19 05:38:47 UTC (rev 226097)
@@ -120,8 +120,6 @@
{
m_file = WTFMove(file);
- setAttributeWithoutSynchronization(HTMLNames::webkitattachmentbloburlAttr, m_file ? m_file->url() : emptyString());
-
if (updateAttributes == UpdateDisplayAttributes::Yes) {
if (m_file) {
setAttributeWithoutSynchronization(HTMLNames::titleAttr, m_file->name());
Modified: trunk/Source/WebCore/rendering/HitTestResult.cpp (226096 => 226097)
--- trunk/Source/WebCore/rendering/HitTestResult.cpp 2017-12-19 05:08:52 UTC (rev 226096)
+++ trunk/Source/WebCore/rendering/HitTestResult.cpp 2017-12-19 05:38:47 UTC (rev 226097)
@@ -356,7 +356,12 @@
if (!attachmentFile)
return URL();
- return URL::fileURLWithFileSystemPath(attachmentFile->path());
+ auto filepath = attachmentFile->path();
+ if (!filepath.isEmpty())
+ return URL::fileURLWithFileSystemPath(filepath);
+
+ ASSERT(attachmentFile->url().protocolIsBlob());
+ return attachmentFile->url();
}
#endif
Modified: trunk/Tools/ChangeLog (226096 => 226097)
--- trunk/Tools/ChangeLog 2017-12-19 05:08:52 UTC (rev 226096)
+++ trunk/Tools/ChangeLog 2017-12-19 05:38:47 UTC (rev 226097)
@@ -1,5 +1,20 @@
2017-12-18 Wenson Hsieh <wenson_hs...@apple.com>
+ [Attachment Support] The 'webkitattachmentbloburl' attribute should not persist after markup serialization
+ https://bugs.webkit.org/show_bug.cgi?id=180924
+ <rdar://problem/36099093>
+
+ Reviewed by Tim Horton.
+
+ Tweaks some existing tests to check that temporary attachment serialization attributes don't stick around on the
+ attachment elements.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+ (-[TestWKWebView hasAttribute:forQuerySelector:]):
+ (TestWebKitAPI::TEST):
+
+2017-12-18 Wenson Hsieh <wenson_hs...@apple.com>
+
[Attachment Support] Support representing pasted or dropped content using attachment elements
https://bugs.webkit.org/show_bug.cgi?id=180892
<rdar://problem/36064210>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm (226096 => 226097)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm 2017-12-19 05:08:52 UTC (rev 226096)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm 2017-12-19 05:38:47 UTC (rev 226097)
@@ -224,6 +224,11 @@
}
}
+- (BOOL)hasAttribute:(NSString *)attributeName forQuerySelector:(NSString *)querySelector
+{
+ return [self stringByEvaluatingJavaScript:[NSString stringWithFormat:@"document.querySelector('%@').hasAttribute('%@')", querySelector, attributeName]].boolValue;
+}
+
- (NSString *)valueOfAttribute:(NSString *)attributeName forQuerySelector:(NSString *)querySelector
{
return [self stringByEvaluatingJavaScript:[NSString stringWithFormat:@"document.querySelector('%@').getAttribute('%@')", querySelector, attributeName]];
@@ -425,6 +430,8 @@
[firstAttachment expectRequestedDataToBe:nil];
[secondAttachment expectRequestedDataToBe:testImageData()];
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentpath" forQuerySelector:@"attachment"]);
}
TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingAndDeletingNewline)
@@ -490,6 +497,8 @@
[webView expectUpdatesAfterCommand:@"ToggleItalic" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
[webView expectUpdatesAfterCommand:@"ToggleUnderline" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
[attachment expectRequestedDataToBe:testHTMLData()];
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentpath" forQuerySelector:@"attachment"]);
// Inserting text should delete the current selection, removing the attachment in the process.
[webView expectUpdatesAfterCommand:@"InsertText" withArgument:@"foo" expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
@@ -511,6 +520,8 @@
[webView expectUpdatesAfterCommand:@"InsertUnorderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
[webView expectUpdatesAfterCommand:@"InsertUnorderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
[attachment expectRequestedDataToBe:testHTMLData()];
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentpath" forQuerySelector:@"attachment"]);
}
TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingRichMarkup)
@@ -523,6 +534,8 @@
attachment = observer.observer().inserted[0];
observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
}
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentpath" forQuerySelector:@"attachment"]);
{
ObserveAttachmentUpdatesForScope observer(webView.get());
[webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').remove()"];
@@ -555,6 +568,8 @@
observer.expectAttachmentUpdates(@[], @[attachment.get()]);
}
[attachment expectRequestedDataToBe:testHTMLData()];
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
+ EXPECT_FALSE([webView hasAttribute:@"webkitattachmentpath" forQuerySelector:@"attachment"]);
}
TEST(WKAttachmentTests, AttachmentDataForEmptyFile)
