- Revision
- 228240
- Author
- wenson_hs...@apple.com
- Date
- 2018-02-07 13:31:15 -0800 (Wed, 07 Feb 2018)
Log Message
REGRESSION(r226396): File paths are inserted when dropping image files
https://bugs.webkit.org/show_bug.cgi?id=182557
<rdar://problem/37294120>
Reviewed by Ryosuke Niwa.
Source/WebCore:
Reverts unintended changes in <http://trac.webkit.org/r226396>. Before r226396, WebContentReader::readFilenames
(a helper function in macOS-specific code) contained logic to create and insert attachment elements if
ENABLE(ATTACHMENT_ELEMENT); otherwise, it would fall back to inserting the visible URL as a text node. Since we
enable the attachment element on all Cocoa platforms via xcconfig files, this was effectively dead code.
However, when r226396 (which moved this out from macOS to Cocoa platform code) refactored this helper function,
it also moved this chunk of code out of the !ENABLE(ATTACHMENT) conditional and into a PLATFORM(MAC) guard,
which means that we now fall back to inserting file paths as text when attachment elements are disabled. To fix
this, we simply remove the (previously) dead code.
A more subtle difference is that we no longer always return true from WebContentReader::readFilePaths. This
means that when we drop files, we no longer skip over the early return in documentFragmentFromDragData when
we've made a fragment, so we read the file path as a URL. To address this, we just restore the pre-macOS 10.13.4
behavior of initializing the document fragment.
Test: modified editing/pasteboard/drag-files-to-editable-element-as-URLs.html.
* editing/WebContentReader.cpp:
(WebCore::WebContentReader::ensureFragment): Deleted.
Remove this helper, as it was only used in WebContentReader::readFilePaths.
* editing/WebContentReader.h:
* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::WebContentReader::readFilePaths):
Tools:
Tweak some image pasting API tests to ensure that file paths are not inserted when pasting images backed by
file paths on disk.
* TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm:
(TEST):
LayoutTests:
Tweak an existing layout test that drops a file into a contenteditable, to check that no text is inserted into
the editable element after dropping.
* editing/pasteboard/drag-files-to-editable-element-as-URLs-expected.txt:
* editing/pasteboard/drag-files-to-editable-element-as-URLs.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (228239 => 228240)
--- trunk/LayoutTests/ChangeLog 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/LayoutTests/ChangeLog 2018-02-07 21:31:15 UTC (rev 228240)
@@ -1,3 +1,17 @@
+2018-02-07 Wenson Hsieh <wenson_hs...@apple.com>
+
+ REGRESSION(r226396): File paths are inserted when dropping image files
+ https://bugs.webkit.org/show_bug.cgi?id=182557
+ <rdar://problem/37294120>
+
+ Reviewed by Ryosuke Niwa.
+
+ Tweak an existing layout test that drops a file into a contenteditable, to check that no text is inserted into
+ the editable element after dropping.
+
+ * editing/pasteboard/drag-files-to-editable-element-as-URLs-expected.txt:
+ * editing/pasteboard/drag-files-to-editable-element-as-URLs.html:
+
2018-02-07 John Wilander <wilan...@apple.com>
Restrict Referer to just the origin for third parties in private mode and third parties ITP blocks cookies for in regular mode
Modified: trunk/LayoutTests/editing/pasteboard/drag-files-to-editable-element-as-URLs-expected.txt (228239 => 228240)
--- trunk/LayoutTests/editing/pasteboard/drag-files-to-editable-element-as-URLs-expected.txt 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/LayoutTests/editing/pasteboard/drag-files-to-editable-element-as-URLs-expected.txt 2018-02-07 21:31:15 UTC (rev 228240)
@@ -1,4 +1,4 @@
-If we drag files onto an editable area, then attachments should be inserted into the editable area.
+If we drag files onto an editable area, then attachments should not be inserted into the editable area since attachment elements are disabled.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
@@ -6,6 +6,7 @@
PASS window.HTMLAttachmentElement is undefined.
PASS document.createElement("attachment") instanceof HTMLUnknownElement is true
PASS editable.querySelector("attachment") is null
+PASS editable.textContent is ""
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/editing/pasteboard/drag-files-to-editable-element-as-URLs.html (228239 => 228240)
--- trunk/LayoutTests/editing/pasteboard/drag-files-to-editable-element-as-URLs.html 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/LayoutTests/editing/pasteboard/drag-files-to-editable-element-as-URLs.html 2018-02-07 21:31:15 UTC (rev 228240)
@@ -6,7 +6,7 @@
<div id="editable" contentEditable=true style="width:200px; height:200px"></div>
<script src=""
<script>
-description('If we drag files onto an editable area, then attachments should be inserted into the editable area.');
+description('If we drag files onto an editable area, then attachments should not be inserted into the editable area since attachment elements are disabled.');
var editable = document.getElementById("editable");
if (window.eventSender) {
@@ -14,6 +14,7 @@
shouldBeUndefined('window.HTMLAttachmentElement');
shouldBeTrue('document.createElement("attachment") instanceof HTMLUnknownElement');
shouldBe('editable.querySelector("attachment")', 'null');
+ shouldBe('editable.textContent', '""');
editable.innerHTML = '';
}
Modified: trunk/Source/WebCore/ChangeLog (228239 => 228240)
--- trunk/Source/WebCore/ChangeLog 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/Source/WebCore/ChangeLog 2018-02-07 21:31:15 UTC (rev 228240)
@@ -1,3 +1,37 @@
+2018-02-07 Wenson Hsieh <wenson_hs...@apple.com>
+
+ REGRESSION(r226396): File paths are inserted when dropping image files
+ https://bugs.webkit.org/show_bug.cgi?id=182557
+ <rdar://problem/37294120>
+
+ Reviewed by Ryosuke Niwa.
+
+ Reverts unintended changes in <http://trac.webkit.org/r226396>. Before r226396, WebContentReader::readFilenames
+ (a helper function in macOS-specific code) contained logic to create and insert attachment elements if
+ ENABLE(ATTACHMENT_ELEMENT); otherwise, it would fall back to inserting the visible URL as a text node. Since we
+ enable the attachment element on all Cocoa platforms via xcconfig files, this was effectively dead code.
+
+ However, when r226396 (which moved this out from macOS to Cocoa platform code) refactored this helper function,
+ it also moved this chunk of code out of the !ENABLE(ATTACHMENT) conditional and into a PLATFORM(MAC) guard,
+ which means that we now fall back to inserting file paths as text when attachment elements are disabled. To fix
+ this, we simply remove the (previously) dead code.
+
+ A more subtle difference is that we no longer always return true from WebContentReader::readFilePaths. This
+ means that when we drop files, we no longer skip over the early return in documentFragmentFromDragData when
+ we've made a fragment, so we read the file path as a URL. To address this, we just restore the pre-macOS 10.13.4
+ behavior of initializing the document fragment.
+
+ Test: modified editing/pasteboard/drag-files-to-editable-element-as-URLs.html.
+
+ * editing/WebContentReader.cpp:
+ (WebCore::WebContentReader::ensureFragment): Deleted.
+
+ Remove this helper, as it was only used in WebContentReader::readFilePaths.
+
+ * editing/WebContentReader.h:
+ * editing/cocoa/WebContentReaderCocoa.mm:
+ (WebCore::WebContentReader::readFilePaths):
+
2018-02-07 John Wilander <wilan...@apple.com>
Restrict Referer to just the origin for third parties in private mode and third parties ITP blocks cookies for in regular mode
Modified: trunk/Source/WebCore/editing/WebContentReader.cpp (228239 => 228240)
--- trunk/Source/WebCore/editing/WebContentReader.cpp 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/Source/WebCore/editing/WebContentReader.cpp 2018-02-07 21:31:15 UTC (rev 228240)
@@ -31,14 +31,6 @@
namespace WebCore {
-DocumentFragment& WebContentReader::ensureFragment()
-{
- ASSERT(frame.document());
- if (!fragment)
- fragment = frame.document()->createDocumentFragment();
- return *fragment;
-}
-
void WebContentReader::addFragment(Ref<DocumentFragment>&& newFragment)
{
if (!fragment)
Modified: trunk/Source/WebCore/editing/WebContentReader.h (228239 => 228240)
--- trunk/Source/WebCore/editing/WebContentReader.h 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/Source/WebCore/editing/WebContentReader.h 2018-02-07 21:31:15 UTC (rev 228240)
@@ -63,7 +63,6 @@
{
}
- DocumentFragment& ensureFragment();
void addFragment(Ref<DocumentFragment>&&);
private:
Modified: trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm (228239 => 228240)
--- trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm 2018-02-07 21:31:15 UTC (rev 228240)
@@ -649,27 +649,20 @@
return false;
auto& document = *frame.document();
- bool readAnyFilePath = false;
- for (auto& path : paths) {
+ if (!fragment)
+ fragment = document.createDocumentFragment();
+
#if ENABLE(ATTACHMENT_ELEMENT)
- if (RuntimeEnabledFeatures::sharedFeatures().attachmentElementEnabled()) {
+ if (RuntimeEnabledFeatures::sharedFeatures().attachmentElementEnabled()) {
+ for (auto& path : paths) {
auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document);
attachment->setFile(File::create(path), HTMLAttachmentElement::UpdateDisplayAttributes::Yes);
- ensureFragment().appendChild(attachment);
- readAnyFilePath = true;
- continue;
+ fragment->appendChild(attachment);
}
+ }
#endif
-#if PLATFORM(MAC)
- // FIXME: Does (and should) any macOS client depend on inserting file paths as plain text in web content?
- // If not, we should just remove this.
- auto paragraph = createDefaultParagraphElement(document);
- paragraph->appendChild(document.createTextNode(userVisibleString([NSURL fileURLWithPath:path])));
- ensureFragment().appendChild(paragraph);
- readAnyFilePath = true;
-#endif
- }
- return readAnyFilePath;
+
+ return true;
}
}
Modified: trunk/Tools/ChangeLog (228239 => 228240)
--- trunk/Tools/ChangeLog 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/Tools/ChangeLog 2018-02-07 21:31:15 UTC (rev 228240)
@@ -1,3 +1,17 @@
+2018-02-07 Wenson Hsieh <wenson_hs...@apple.com>
+
+ REGRESSION(r226396): File paths are inserted when dropping image files
+ https://bugs.webkit.org/show_bug.cgi?id=182557
+ <rdar://problem/37294120>
+
+ Reviewed by Ryosuke Niwa.
+
+ Tweak some image pasting API tests to ensure that file paths are not inserted when pasting images backed by
+ file paths on disk.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm:
+ (TEST):
+
2018-02-07 Ms2ger <ms2...@igalia.com>
[GTK] Enable WebKit.GeolocationTransitionTo{High,Low}Accuracy tests
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm (228239 => 228240)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm 2018-02-07 20:09:51 UTC (rev 228239)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm 2018-02-07 21:31:15 UTC (rev 228240)
@@ -140,6 +140,7 @@
EXPECT_WK_STREQ("1", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
EXPECT_WK_STREQ("image/gif", [webView stringByEvaluatingJavaScript:@"file = dataTransfer.files[0]; file.type"]);
EXPECT_WK_STREQ("sunset-in-cupertino-400px.gif", [webView stringByEvaluatingJavaScript:@"file.name"]);
+ EXPECT_WK_STREQ("", [webView stringByEvaluatingJavaScript:@"editor.textContent"]);
[webView stringByEvaluatingJavaScript:@"insertFileAsImage(file)"];
[webView waitForMessage:@"loaded"];
@@ -161,6 +162,7 @@
EXPECT_WK_STREQ("1", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
EXPECT_WK_STREQ("image/jpeg", [webView stringByEvaluatingJavaScript:@"file = dataTransfer.files[0]; file.type"]);
EXPECT_WK_STREQ("sunset-in-cupertino-600px.jpg", [webView stringByEvaluatingJavaScript:@"file.name"]);
+ EXPECT_WK_STREQ("", [webView stringByEvaluatingJavaScript:@"editor.textContent"]);
[webView stringByEvaluatingJavaScript:@"insertFileAsImage(file)"];
[webView waitForMessage:@"loaded"];
@@ -182,6 +184,7 @@
EXPECT_WK_STREQ("1", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
EXPECT_WK_STREQ("image/png", [webView stringByEvaluatingJavaScript:@"file = dataTransfer.files[0]; file.type"]);
EXPECT_WK_STREQ("sunset-in-cupertino-200px.png", [webView stringByEvaluatingJavaScript:@"file.name"]);
+ EXPECT_WK_STREQ("", [webView stringByEvaluatingJavaScript:@"editor.textContent"]);
[webView stringByEvaluatingJavaScript:@"insertFileAsImage(file)"];
[webView waitForMessage:@"loaded"];
@@ -203,6 +206,7 @@
EXPECT_WK_STREQ("1", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
EXPECT_WK_STREQ("image/tiff", [webView stringByEvaluatingJavaScript:@"file = dataTransfer.files[0]; file.type"]);
EXPECT_WK_STREQ("sunset-in-cupertino-100px.tiff", [webView stringByEvaluatingJavaScript:@"file.name"]);
+ EXPECT_WK_STREQ("", [webView stringByEvaluatingJavaScript:@"editor.textContent"]);
[webView stringByEvaluatingJavaScript:@"insertFileAsImage(file)"];
[webView waitForMessage:@"loaded"];
