From 4ab1f339cf4357094d313426bbe0d6760b74f408 Mon Sep 17 00:00:00 2001 From: "jianli@chromium.org" Date: Tue, 8 Sep 2009 18:02:11 +0000 Subject: [PATCH] WebCore: 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. 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 --- LayoutTests/ChangeLog | 12 ++++++++ .../clipboard/clipboard-file-access-expected.txt | 6 ++-- .../clipboard/resources/clipboard-file-access.js | 33 ++++++++++++++++------ WebCore/ChangeLog | 15 ++++++++++ WebCore/platform/mac/ClipboardMac.mm | 18 ++++++++---- 5 files changed, 66 insertions(+), 18 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index fc4ef58..4d60435 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,15 @@ +2009-09-08 Jian Li + + 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 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 --- 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: -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.types contains Files. +PASS event.dataTransfer.types does not contain Files. 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: 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 --- 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); +var filesToDrag; 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); @@ -19,21 +21,24 @@ dragTarget.addEventListener("dragenter", function() { 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:") - 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:") - eventShouldContainTransferType(event, "Files"); + var shouldContainType = (filesToDrag.length > 0); + checkForEventTransferType(event, "Files", shouldContainType); fileListShouldBe("event.dataTransfer.files", expectedFilesOnDrop); event.preventDefault(); }, false); @@ -51,6 +56,7 @@ function moveMouseToOutsideOfElement(element) { } function dragFilesOntoDragTarget(files, leave) { + filesToDrag = files; eventSender.beginDragWithFiles(files); moveMouseToCenterOfElement(dragTarget); if (leave && leave === true) @@ -58,12 +64,21 @@ function dragFilesOntoDragTarget(files, leave) { 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) diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 0b480bb..49648ac 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,18 @@ +2009-09-08 Jian Li + + 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 Reviewed by Eric Seidel. diff --git a/WebCore/platform/mac/ClipboardMac.mm b/WebCore/platform/mac/ClipboardMac.mm index dd0ebb7..0b461fd 100644 --- a/WebCore/platform/mac/ClipboardMac.mm +++ b/WebCore/platform/mac/ClipboardMac.mm @@ -104,7 +104,7 @@ static String utiTypeFromCocoaType(NSString *type) return String(); } -static void addHTMLClipboardTypesForCocoaType(HashSet& resultTypes, NSString *cocoaType) +static void addHTMLClipboardTypesForCocoaType(HashSet& resultTypes, NSString *cocoaType, NSPasteboard *pasteboard) { // 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& resultTypes, NSSt 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 { @@ -276,7 +282,7 @@ HashSet ClipboardMac::types() const 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; -- 1.8.3.1