Title: [223135] trunk
Revision
223135
Author
cdu...@apple.com
Date
2017-10-10 11:42:34 -0700 (Tue, 10 Oct 2017)

Log Message

Entries API should recognize path starting with 2 slashes as valid absolute path
https://bugs.webkit.org/show_bug.cgi?id=178135

Reviewed by Ryosuke Niwa.

Source/WebCore:

Entries API should recognize paths starting with 2 slashes as valid absolute paths to match Chrome's behavior.
See https://github.com/WICG/entries-api/commit/990454758005a6039655835503d551015e346d9d

This was causing us to fail some manual web-platform-tests.

No new tests, updated existing tests.

* Modules/entriesapi/DOMFileSystem.cpp:
(WebCore::isValidPathSegment):
(WebCore::isZeroOrMorePathSegmentsSeparatedBySlashes):
(WebCore::isValidRelativeVirtualPath):
(WebCore::isValidVirtualPath):

LayoutTests:

Add layout test coverage.

* editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory-expected.txt:
* editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory.html:
* editing/pasteboard/entries-api/datatransfer-items-drop-getFile-expected.txt:
* editing/pasteboard/entries-api/datatransfer-items-drop-getFile.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (223134 => 223135)


--- trunk/LayoutTests/ChangeLog	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/LayoutTests/ChangeLog	2017-10-10 18:42:34 UTC (rev 223135)
@@ -1,3 +1,17 @@
+2017-10-10  Chris Dumez  <cdu...@apple.com>
+
+        Entries API should recognize path starting with 2 slashes as valid absolute path
+        https://bugs.webkit.org/show_bug.cgi?id=178135
+
+        Reviewed by Ryosuke Niwa.
+
+        Add layout test coverage.
+
+        * editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory-expected.txt:
+        * editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory.html:
+        * editing/pasteboard/entries-api/datatransfer-items-drop-getFile-expected.txt:
+        * editing/pasteboard/entries-api/datatransfer-items-drop-getFile.html:
+
 2017-10-10  Matt Lewis  <jlew...@apple.com>
 
         Unreviewed, rolling out r223110.

Modified: trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory-expected.txt (223134 => 223135)


--- trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory-expected.txt	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory-expected.txt	2017-10-10 18:42:34 UTC (rev 223135)
@@ -62,6 +62,16 @@
 PASS testFilesEntry.isDirectory is true
 PASS testFilesEntry.name is "testFiles"
 PASS testFilesEntry.fullPath is "/testFiles"
+* Edge case: calling getDirectory() with path starting with 2 slashes
+PASS testFilesEntry.isFile is false
+PASS testFilesEntry.isDirectory is true
+PASS testFilesEntry.name is "subfolder1"
+PASS testFilesEntry.fullPath is "/testFiles/subfolder1"
+* Edge case: calling getDirectory() with path starting with 2 slashes and containing 2 following slashes
+PASS testFilesEntry.isFile is false
+PASS testFilesEntry.isDirectory is true
+PASS testFilesEntry.name is "subfolder1"
+PASS testFilesEntry.fullPath is "/testFiles/subfolder1"
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory.html (223134 => 223135)


--- trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory.html	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getDirectory.html	2017-10-10 18:42:34 UTC (rev 223135)
@@ -203,6 +203,32 @@
     });
 }
 
+function test17() {
+    debug("* Edge case: calling getDirectory() with path starting with 2 slashes");
+    return getDirectoryAsPromise(directoryEntry, '//testFiles/subfolder1', {}).then(entry => {
+        testFilesEntry = entry;
+        shouldBeFalse("testFilesEntry.isFile");
+        shouldBeTrue("testFilesEntry.isDirectory");
+        shouldBeEqualToString("testFilesEntry.name", "subfolder1");
+        shouldBeEqualToString("testFilesEntry.fullPath", "/testFiles/subfolder1");
+    }, e => {
+        testFailed("getDirectory('//testFiles/subfolder1') should succeed");
+    });
+}
+
+function test18() {
+    debug("* Edge case: calling getDirectory() with path starting with 2 slashes and containing 2 following slashes");
+    return getDirectoryAsPromise(directoryEntry, '//testFiles//subfolder1', {}).then(entry => {
+        testFilesEntry = entry;
+        shouldBeFalse("testFilesEntry.isFile");
+        shouldBeTrue("testFilesEntry.isDirectory");
+        shouldBeEqualToString("testFilesEntry.name", "subfolder1");
+        shouldBeEqualToString("testFilesEntry.fullPath", "/testFiles/subfolder1");
+    }, e => {
+        testFailed("getDirectory('//testFiles//subfolder1') should succeed");
+    });
+}
+
 var dropzone = document.getElementById('dropzone');
 dropzone._ondrop_ = function(e) {
     e.preventDefault();
@@ -227,6 +253,8 @@
     .then(test14)
     .then(test15)
     .then(test16)
+    .then(test17)
+    .then(test18)
     .then(finishJSTest);
 };
 

Modified: trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getFile-expected.txt (223134 => 223135)


--- trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getFile-expected.txt	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getFile-expected.txt	2017-10-10 18:42:34 UTC (rev 223135)
@@ -42,6 +42,14 @@
 PASS file1Entry.isFile is true
 * Error case: calling getFile() with empty path
 PASS ex.name is "TypeMismatchError"
+* Edge case: calling getFile() with absolute path starting with 2 slashes
+PASS file1Entry.name is "file1.txt"
+PASS file1Entry.fullPath is "/testFiles/file1.txt"
+PASS file1Entry.isFile is true
+* Edge case: calling getFile() with absolute path starting with 2 slashes and containing 2 following slashes
+PASS file1Entry.name is "file1.txt"
+PASS file1Entry.fullPath is "/testFiles/file1.txt"
+PASS file1Entry.isFile is true
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getFile.html (223134 => 223135)


--- trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getFile.html	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/LayoutTests/editing/pasteboard/entries-api/datatransfer-items-drop-getFile.html	2017-10-10 18:42:34 UTC (rev 223135)
@@ -159,6 +159,30 @@
     });
 }
 
+function test14() {
+    debug("* Edge case: calling getFile() with absolute path starting with 2 slashes");
+    return getFileAsPromise(directoryEntry, "//testFiles/file1.txt", {}).then(entry => {
+        file1Entry = entry;
+        shouldBeEqualToString("file1Entry.name", "file1.txt");
+        shouldBeEqualToString("file1Entry.fullPath", "/testFiles/file1.txt");
+        shouldBeTrue("file1Entry.isFile");
+    }, e => {
+        testFailed("getFile('//testFiles/file1.txt') should succeed");
+    });
+}
+
+function test15() {
+    debug("* Edge case: calling getFile() with absolute path starting with 2 slashes and containing 2 following slashes");
+    return getFileAsPromise(directoryEntry, "//testFiles//file1.txt", {}).then(entry => {
+        file1Entry = entry;
+        shouldBeEqualToString("file1Entry.name", "file1.txt");
+        shouldBeEqualToString("file1Entry.fullPath", "/testFiles/file1.txt");
+        shouldBeTrue("file1Entry.isFile");
+    }, e => {
+        testFailed("getFile('//testFiles//file1.txt') should succeed");
+    });
+}
+
 var dropzone = document.getElementById('dropzone');
 dropzone._ondrop_ = function(e) {
     e.preventDefault();
@@ -180,6 +204,8 @@
     .then(test11)
     .then(test12)
     .then(test13)
+    .then(test14)
+    .then(test15)
     .then(finishJSTest);
 };
 

Modified: trunk/Source/WebCore/ChangeLog (223134 => 223135)


--- trunk/Source/WebCore/ChangeLog	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/Source/WebCore/ChangeLog	2017-10-10 18:42:34 UTC (rev 223135)
@@ -1,3 +1,23 @@
+2017-10-10  Chris Dumez  <cdu...@apple.com>
+
+        Entries API should recognize path starting with 2 slashes as valid absolute path
+        https://bugs.webkit.org/show_bug.cgi?id=178135
+
+        Reviewed by Ryosuke Niwa.
+
+        Entries API should recognize paths starting with 2 slashes as valid absolute paths to match Chrome's behavior.
+        See https://github.com/WICG/entries-api/commit/990454758005a6039655835503d551015e346d9d
+
+        This was causing us to fail some manual web-platform-tests.
+
+        No new tests, updated existing tests.
+
+        * Modules/entriesapi/DOMFileSystem.cpp:
+        (WebCore::isValidPathSegment):
+        (WebCore::isZeroOrMorePathSegmentsSeparatedBySlashes):
+        (WebCore::isValidRelativeVirtualPath):
+        (WebCore::isValidVirtualPath):
+
 2017-10-10  Matt Lewis  <jlew...@apple.com>
 
         Unreviewed, rolling out r223110.

Modified: trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp (223134 => 223135)


--- trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp	2017-10-10 17:58:53 UTC (rev 223134)
+++ trunk/Source/WebCore/Modules/entriesapi/DOMFileSystem.cpp	2017-10-10 18:42:34 UTC (rev 223135)
@@ -96,8 +96,7 @@
 // https://wicg.github.io/entries-api/#path-segment
 static bool isValidPathSegment(StringView segment)
 {
-    ASSERT(!segment.isEmpty());
-    if (segment == "." || segment == "..")
+    if (segment.isEmpty() || segment == "." || segment == "..")
         return true;
 
     for (unsigned i = 0; i < segment.length(); ++i) {
@@ -107,6 +106,16 @@
     return true;
 }
 
+static bool isZeroOrMorePathSegmentsSeparatedBySlashes(StringView string)
+{
+    auto segments = string.split('/');
+    for (auto segment : segments) {
+        if (!isValidPathSegment(segment))
+            return false;
+    }
+    return true;
+}
+
 // https://wicg.github.io/entries-api/#relative-path
 static bool isValidRelativeVirtualPath(StringView virtualPath)
 {
@@ -116,12 +125,7 @@
     if (virtualPath[0] == '/')
         return false;
 
-    auto segments = virtualPath.split('/');
-    for (auto segment : segments) {
-        if (!isValidPathSegment(segment))
-            return false;
-    }
-    return true;
+    return isZeroOrMorePathSegmentsSeparatedBySlashes(virtualPath);
 }
 
 // https://wicg.github.io/entries-api/#valid-path
@@ -129,8 +133,10 @@
 {
     if (virtualPath.isEmpty())
         return true;
-    if (virtualPath[0] == '/')
-        return virtualPath.length() == 1 || isValidRelativeVirtualPath(virtualPath.substring(1));
+    if (virtualPath[0] == '/') {
+        // An absolute path is a string consisting of '/' (U+002F SOLIDUS) followed by one or more path segments joined by '/' (U+002F SOLIDUS).
+        return isZeroOrMorePathSegmentsSeparatedBySlashes(virtualPath.substring(1));
+    }
     return isValidRelativeVirtualPath(virtualPath);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to