WebCore: dataTransfer.types() should not return Files if file list is empty in the...

https://bugs.webkit.org/show_bug.cgi?id=28891

Reviewed by David Levin.

We change the behavior to handle the empty file list in order to match the spec.

Tested by clipboard-file-access.html.

* platform/mac/ClipboardMac.mm:
(WebCore::addHTMLClipboardTypesForCocoaType):
(WebCore::ClipboardMac::types):

LayoutTests: dataTransfer.types() should not return Files if file list is empty in the clipboard.
https://bugs.webkit.org/show_bug.cgi?id=28891

Reviewed by David Levin.

Update the test script and expected result to reflect the behavior change.

* http/tests/security/clipboard/clipboard-file-access-expected.txt:
* http/tests/security/clipboard/resources/clipboard-file-access.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@48169 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index fc4ef58..4d60435 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2009-09-08  Jian Li  <jianli@chromium.org>
+
+        Reviewed by David Levin.
+
+        dataTransfer.types() should not return Files if file list is empty in the clipboard.
+        https://bugs.webkit.org/show_bug.cgi?id=28891
+
+        Update the test script and expected result to reflect the behavior change.
+
+        * http/tests/security/clipboard/clipboard-file-access-expected.txt:
+        * http/tests/security/clipboard/resources/clipboard-file-access.js:
+

 2009-09-08  Steve VanDeBogart  <vandebo@chromium.org>
 
         Reviewed by Eric Seidel.
 2009-09-08  Steve VanDeBogart  <vandebo@chromium.org>
 
         Reviewed by Eric Seidel.

diff --git a/LayoutTests/http/tests/security/clipboard/clipboard-file-access-expected.txt b/LayoutTests/http/tests/security/clipboard/clipboard-file-access-expected.txt
index 524d340..3da883b 100644 (file)
--- a/LayoutTests/http/tests/security/clipboard/clipboard-file-access-expected.txt
+++ b/LayoutTests/http/tests/security/clipboard/clipboard-file-access-expected.txt
@@ -5,13 +5,13 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 Dragging no files should return an empty file list (arbitrary implementation detail):
 On dragenter:
 
 Dragging no files should return an empty file list (arbitrary implementation detail):
 On dragenter:

-PASS event.dataTransfer.types contains Files.
+PASS event.dataTransfer.types does not contain Files.

 PASS event.dataTransfer.files.length is 0
 On dragover:
 PASS event.dataTransfer.files.length is 0
 On dragover:

-PASS event.dataTransfer.types contains Files.
+PASS event.dataTransfer.types does not contain Files.

 PASS event.dataTransfer.files.length is 0
 On drop:
 PASS event.dataTransfer.files.length is 0
 On drop:

-PASS event.dataTransfer.types contains Files.
+PASS event.dataTransfer.types does not contain Files.

 PASS event.dataTransfer.files.length is 0
 Drag drop a single (non-existant) file onto an element:
 On dragenter:
 PASS event.dataTransfer.files.length is 0
 Drag drop a single (non-existant) file onto an element:
 On dragenter:

diff --git a/LayoutTests/http/tests/security/clipboard/resources/clipboard-file-access.js b/LayoutTests/http/tests/security/clipboard/resources/clipboard-file-access.js
index 1d7f1c3..6197b44 100644 (file)
--- a/LayoutTests/http/tests/security/clipboard/resources/clipboard-file-access.js
+++ b/LayoutTests/http/tests/security/clipboard/resources/clipboard-file-access.js
@@ -8,10 +8,12 @@ dragTarget.style.height = "100px";
 // Important that we put this at the top of the doc so that logging does not cause it to go out of view (and be undragable)
 document.body.insertBefore(dragTarget, document.body.firstChild);
 
 // Important that we put this at the top of the doc so that logging does not cause it to go out of view (and be undragable)
 document.body.insertBefore(dragTarget, document.body.firstChild);
 

+var filesToDrag;

 dragTarget.addEventListener("dragenter", function() {
     debug("On dragenter:")
     event.dataTransfer.dropEffect = "copy";
 dragTarget.addEventListener("dragenter", function() {
     debug("On dragenter:")
     event.dataTransfer.dropEffect = "copy";

-    eventShouldContainTransferType(event, "Files");
+    var shouldContainType = (filesToDrag.length > 0);
+    checkForEventTransferType(event, "Files", shouldContainType);

     fileListShouldBe("event.dataTransfer.files", []);
     event.preventDefault();
 }, false);
     fileListShouldBe("event.dataTransfer.files", []);
     event.preventDefault();
 }, false);


@@ -19,21 +21,24 @@ dragTarget.addEventListener("dragenter", function() {
 dragTarget.addEventListener("dragover", function() {
     debug("On dragover:")
     event.dataTransfer.dropEffect = "copy";
 dragTarget.addEventListener("dragover", function() {
     debug("On dragover:")
     event.dataTransfer.dropEffect = "copy";

-    eventShouldContainTransferType(event, "Files");
+    var shouldContainType = (filesToDrag.length > 0);
+    checkForEventTransferType(event, "Files", shouldContainType);

     fileListShouldBe("event.dataTransfer.files", []);
     event.preventDefault();
 }, false);
 
 dragTarget.addEventListener("dragleave", function() {
     debug("On dragleave:")
     fileListShouldBe("event.dataTransfer.files", []);
     event.preventDefault();
 }, false);
 
 dragTarget.addEventListener("dragleave", function() {
     debug("On dragleave:")

-    eventShouldContainTransferType(event, "Files");
+    var shouldContainType = (filesToDrag.length > 0);
+    checkForEventTransferType(event, "Files", shouldContainType);

     fileListShouldBe("event.dataTransfer.files", []);
 }, false);
 
 var expectedFilesOnDrop;
 dragTarget.addEventListener("drop", function() {
     debug("On drop:")
     fileListShouldBe("event.dataTransfer.files", []);
 }, false);
 
 var expectedFilesOnDrop;
 dragTarget.addEventListener("drop", function() {
     debug("On drop:")

-    eventShouldContainTransferType(event, "Files");
+    var shouldContainType = (filesToDrag.length > 0);
+    checkForEventTransferType(event, "Files", shouldContainType);

     fileListShouldBe("event.dataTransfer.files", expectedFilesOnDrop);
     event.preventDefault();
 }, false);
     fileListShouldBe("event.dataTransfer.files", expectedFilesOnDrop);
     event.preventDefault();
 }, false);


@@ -51,6 +56,7 @@ function moveMouseToOutsideOfElement(element) {
 }
 
 function dragFilesOntoDragTarget(files, leave) {
 }
 
 function dragFilesOntoDragTarget(files, leave) {

+    filesToDrag = files;

     eventSender.beginDragWithFiles(files);
     moveMouseToCenterOfElement(dragTarget);
     if (leave && leave === true)
     eventSender.beginDragWithFiles(files);
     moveMouseToCenterOfElement(dragTarget);
     if (leave && leave === true)


@@ -58,12 +64,21 @@ function dragFilesOntoDragTarget(files, leave) {
     eventSender.mouseUp();
 }
 
     eventSender.mouseUp();
 }
 

-function eventShouldContainTransferType(event, typeString)
+function checkForEventTransferType(event, typeString, shouldContainType)

 {
 {

-   if (event.dataTransfer.types.indexOf(typeString) == -1)
-       testFailed("event.dataTransfer.types " + typeString + " expected.");
-   else
-       testPassed("event.dataTransfer.types contains " + typeString + ".");
+    var passedCheck;
+    var message;
+    if (event.dataTransfer.types && event.dataTransfer.types.indexOf(typeString) != -1) {
+        passedCheck = shouldContainType;
+        message = "event.dataTransfer.types contains " + typeString + ".";
+    } else {
+        passedCheck = !shouldContainType;
+        message = "event.dataTransfer.types does not contain " + typeString + ".";
+    }
+    if (passedCheck)
+        testPassed(message);
+    else
+        testFailed(message);

 }
 
 function fileListShouldBe(fileListString, filesArray)
 }
 
 function fileListShouldBe(fileListString, filesArray)

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 0b480bb..49648ac 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2009-09-08  Jian Li  <jianli@chromium.org>
+
+        Reviewed by David Levin.
+
+        dataTransfer.types() should not return Files if file list is empty in the clipboard.
+        https://bugs.webkit.org/show_bug.cgi?id=28891
+
+        We change the behavior to handle the empty file list in order to match the spec.
+
+        Tested by clipboard-file-access.html.
+
+        * platform/mac/ClipboardMac.mm:
+        (WebCore::addHTMLClipboardTypesForCocoaType):
+        (WebCore::ClipboardMac::types):
+

 2009-09-08  Steve VanDeBogart  <vandebo@chromium.org>
 
         Reviewed by Eric Seidel.
 2009-09-08  Steve VanDeBogart  <vandebo@chromium.org>
 
         Reviewed by Eric Seidel.

diff --git a/WebCore/platform/mac/ClipboardMac.mm b/WebCore/platform/mac/ClipboardMac.mm
index dd0ebb7..0b461fd 100644 (file)
--- a/WebCore/platform/mac/ClipboardMac.mm
+++ b/WebCore/platform/mac/ClipboardMac.mm
@@ -104,7 +104,7 @@ static String utiTypeFromCocoaType(NSString *type)
     return String();
 }
 
     return String();
 }
 

-static void addHTMLClipboardTypesForCocoaType(HashSet<String>& resultTypes, NSString *cocoaType)
+static void addHTMLClipboardTypesForCocoaType(HashSet<String>& resultTypes, NSString *cocoaType, NSPasteboard *pasteboard)

 {
     // UTI may not do these right, so make sure we get the right, predictable result
     if ([cocoaType isEqualToString:NSStringPboardType])
 {
     // UTI may not do these right, so make sure we get the right, predictable result
     if ([cocoaType isEqualToString:NSStringPboardType])


@@ -112,10 +112,16 @@ static void addHTMLClipboardTypesForCocoaType(HashSet<String>& resultTypes, NSSt
     else if ([cocoaType isEqualToString:NSURLPboardType])
         resultTypes.add("text/uri-list");
     else if ([cocoaType isEqualToString:NSFilenamesPboardType]) {
     else if ([cocoaType isEqualToString:NSURLPboardType])
         resultTypes.add("text/uri-list");
     else if ([cocoaType isEqualToString:NSFilenamesPboardType]) {

-        // It is unknown if NSFilenamesPboardType always implies NSURLPboardType in Cocoa,
-        // but NSFilenamesPboardType should imply both 'text/uri-list' and 'Files'
-        resultTypes.add("text/uri-list");
-        resultTypes.add("Files");
+        // If file list is empty, add nothing.
+        // Note that there is a chance that the file list count could have changed since we grabbed the types array.
+        // However, this is not really an issue for us doing a sanity check here.
+        NSArray *fileList = [pasteboard propertyListForType:NSFilenamesPboardType];
+        if ([fileList count]) {
+            // It is unknown if NSFilenamesPboardType always implies NSURLPboardType in Cocoa,
+            // but NSFilenamesPboardType should imply both 'text/uri-list' and 'Files'
+            resultTypes.add("text/uri-list");
+            resultTypes.add("Files");
+        }

     } else if (String utiType = utiTypeFromCocoaType(cocoaType))
         resultTypes.add(utiType);
     else {
     } else if (String utiType = utiTypeFromCocoaType(cocoaType))
         resultTypes.add(utiType);
     else {


@@ -276,7 +282,7 @@ HashSet<String> ClipboardMac::types() const
         if ([pbType isEqualToString:@"NeXT plain ascii pasteboard type"])
             continue;   // skip this ancient type that gets auto-supplied by some system conversion
 
         if ([pbType isEqualToString:@"NeXT plain ascii pasteboard type"])
             continue;   // skip this ancient type that gets auto-supplied by some system conversion
 

-        addHTMLClipboardTypesForCocoaType(result, pbType);
+        addHTMLClipboardTypesForCocoaType(result, pbType, m_pasteboard.get());

     }
 
     return result;
     }
 
     return result;