[DnD] effectAllowed and dropEffect can be set to bogus values.
authorbweinstein@apple.com <bweinstein@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jan 2010 18:23:18 +0000 (18:23 +0000)
committerbweinstein@apple.com <bweinstein@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jan 2010 18:23:18 +0000 (18:23 +0000)
Fixes <https://bugs.webkit.org/show_bug.cgi?id=33635>.

Reviewed by Oliver Hunt.

WebCore:

Test to make aure dropEffect and effectAllowed are being set to valid values
when they are being set (list of valid values given by HTML5 specification).

Also, drive by change to initialize dropEffect to none (as described in spec).

Test: fast/events/bogus-dropEffect-effectAllowed.html

* dom/Clipboard.cpp:
(WebCore::Clipboard::Clipboard): Initialize m_dropEffect to "none".
(WebCore::Clipboard::setDropEffect): Check if dropEffect is being set to a valid value.
(WebCore::Clipboard::setEffectAllowed): Check if effectAllowed is being set to a valid value.

LayoutTests:

Added a test to check the handling of setting effectAllowed and
dropEffect to bogus values (that it gets ignored), and updated results
to drag-and-drop because it uses a dummy value.

* fast/events/bogus-dropEffect-effectAllowed-expected.txt: Added.
* fast/events/bogus-dropEffect-effectAllowed.html: Added.
* fast/events/drag-and-drop-expected.txt:
* fast/events/drag-and-drop.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/events/bogus-dropEffect-effectAllowed-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/bogus-dropEffect-effectAllowed.html [new file with mode: 0644]
LayoutTests/fast/events/drag-and-drop-expected.txt
LayoutTests/fast/events/drag-and-drop.html
WebCore/ChangeLog
WebCore/dom/Clipboard.cpp

index 4550263..19f78d9 100644 (file)
@@ -1,3 +1,19 @@
+2010-01-14  Brian Weinstein  <bweinstein@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        [DnD] effectAllowed and dropEffect can be set to bogus values.
+        Fixes <https://bugs.webkit.org/show_bug.cgi?id=33635>.
+        
+        Added a test to check the handling of setting effectAllowed and
+        dropEffect to bogus values (that it gets ignored), and updated results
+        to drag-and-drop because it uses a dummy value.
+
+        * fast/events/bogus-dropEffect-effectAllowed-expected.txt: Added.
+        * fast/events/bogus-dropEffect-effectAllowed.html: Added.
+        * fast/events/drag-and-drop-expected.txt:
+        * fast/events/drag-and-drop.html:
+
 2010-01-14  Adam Roben  <aroben@apple.com>
 
         Fix a typo in editing/selection/inactive-selection.html
diff --git a/LayoutTests/fast/events/bogus-dropEffect-effectAllowed-expected.txt b/LayoutTests/fast/events/bogus-dropEffect-effectAllowed-expected.txt
new file mode 100644 (file)
index 0000000..e7c555f
--- /dev/null
@@ -0,0 +1,56 @@
+This test checks that effectAllowed and dropEffect cannot be set to values that aren't defined in the spec
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS event.dataTransfer.effectAllowed is "all"
+PASS event.dataTransfer.effectAllowed is "copy"
+PASS event.dataTransfer.effectAllowed is "copy"
+PASS event.dataTransfer.effectAllowed is "copyLink"
+PASS event.dataTransfer.effectAllowed is "copyLink"
+PASS event.dataTransfer.effectAllowed is "copyMove"
+PASS event.dataTransfer.effectAllowed is "copyMove"
+PASS event.dataTransfer.effectAllowed is "link"
+PASS event.dataTransfer.effectAllowed is "linkMove"
+PASS event.dataTransfer.effectAllowed is "move"
+PASS event.dataTransfer.effectAllowed is "none"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "link"
+PASS event.dataTransfer.dropEffect is "link"
+PASS event.dataTransfer.dropEffect is "move"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.dropEffect is "link"
+PASS event.dataTransfer.dropEffect is "link"
+PASS event.dataTransfer.dropEffect is "move"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.dropEffect is "none"
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/bogus-dropEffect-effectAllowed.html b/LayoutTests/fast/events/bogus-dropEffect-effectAllowed.html
new file mode 100644 (file)
index 0000000..92e49ac
--- /dev/null
@@ -0,0 +1,146 @@
+<html>
+<head>
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+<style>
+#dropTarget, #dragMe { text-align: center; display: table-cell; vertical-align: middle }
+#dropTarget {width: 256px; height: 256px; border: 1px dashed}
+#dragMe {-webkit-user-drag: element; -webkit-user-select: none; background: #ff0000; width: 64px; height: 64px; color: white}
+</style>
+<script>
+    var dragMe;
+    var dropTarget;
+    var effectAllowedElem;
+    var dropEffectElem;
+    var consoleElm;
+    var event;
+    
+    window.onload = function()
+    {
+        dragMe = document.getElementById("dragMe");
+        dropTarget = document.getElementById("dropTarget");
+        consoleElm = document.getElementById("console");
+        
+        if (!dragMe || !dropTarget || !consoleElm)
+            return;
+        
+        dragMe.ondragstart = dragStart;
+        dragMe.ondragend = dragEnd;
+        
+        dropTarget.ondragenter = dragEntered;
+        dropTarget.ondragover = dragOver;
+        dropTarget.ondrop = drop;
+        
+        runTest();
+    }
+    
+    function dragStart(e)
+    {
+        var validEffectAllowedList = ["all", "copy", "copyLink", "copyMove", "link", "linkMove", "move", "none", "uninitialized"];
+        var effectAllowedListToTest = ["all", "copy", "bogus", "copyLink", "wrong", "copyMove", "linkCopyMove", "link",
+            "linkMove", "move", "none", "uninitialized", "dummy", "bogus", "fake", "illegal"];
+        
+        event = e;
+        
+        for (var i = 0; i < effectAllowedListToTest.length; i++) {
+            var effectAllowedBefore = e.dataTransfer.effectAllowed;
+            e.dataTransfer.effectAllowed = effectAllowedListToTest[i];
+            if (validEffectAllowedList.indexOf(effectAllowedListToTest[i]) != -1)
+                shouldBeEqualToString("event.dataTransfer.effectAllowed", effectAllowedListToTest[i]);
+            else
+                shouldBeEqualToString("event.dataTransfer.effectAllowed", effectAllowedBefore);
+        }
+        
+        e.dataTransfer.setData('Text', e.target.textContent);
+    }
+    
+    function dragEnd(e)
+    {
+        return;
+    }
+    
+    function dragEntered(e)
+    {
+        dragEnteredAndUpdated(e);
+    }
+    
+    function dragOver(e)
+    {
+        dragEnteredAndUpdated(e);
+    }
+    
+    function dragEnteredAndUpdated(e)
+    {
+        var validDropEffectList = ["none", "copy", "link", "move", "link"];
+        var dropEffectListToTest = ["all", "copy", "bogus", "copyLink", "wrong", "copyMove", "linkCopyMove", "link",
+            "linkMove", "move", "none", "uninitialized", "dummy", "bogus", "fake", "illegal"];
+        
+        event = e;
+        
+        for (var i = 0; i < dropEffectListToTest.length; i++) {
+            var dropEffectBefore = e.dataTransfer.dropEffect;
+            e.dataTransfer.dropEffect = dropEffectListToTest[i];
+
+            if (validDropEffectList.indexOf(dropEffectListToTest[i]) != -1)
+                shouldBeEqualToString("event.dataTransfer.dropEffect", dropEffectListToTest[i]);
+            else
+                shouldBeEqualToString("event.dataTransfer.dropEffect", dropEffectBefore);
+
+        }
+
+        cancelDrag(e);
+    }
+    
+    function drop(e)
+    {
+        cancelDrag(e);
+    }
+    
+    function cancelDrag(e)
+    {
+        e.preventDefault();
+    }
+
+    function runTest()
+    {
+        if (!window.eventSender)
+            return;
+            
+        if (window.layoutTestController)
+            layoutTestController.dumpAsText();
+            
+        var startX = dragMe.offsetLeft + 10;
+        var startY = dragMe.offsetTop + dragMe.offsetHeight / 2;
+        var endX = dropTarget.offsetLeft + 10;
+        var endY = dropTarget.offsetTop + dropTarget.offsetHeight / 2;
+        
+        eventSender.mouseMoveTo(startX, startY);
+        eventSender.mouseDown();
+        eventSender.leapForward(100);
+        eventSender.mouseMoveTo(endX, endY);
+        eventSender.mouseUp();
+
+        var testContainer = document.getElementById("test-container");
+        if (testContainer)
+            document.body.removeChild(testContainer);
+        debug('<br /><span class="pass">TEST COMPLETE</span>');
+    }
+</script>
+</head>
+<body>
+    <p id="description"></p>
+    <div id="test-container">
+        <br/><br/>
+        <div id="dropTarget">Drop the red square onto me.<br/><br/></div>
+        <hr/>
+        <p>Items that can be dragged to the drop target:</p>
+        <div id="dragMe" draggable="true">Square</div>
+        <hr/>
+    </div>
+    <div id="console"></div>
+    <script>
+        description("This test checks that effectAllowed and dropEffect cannot be set to values that aren't defined in the spec");
+        var successfullyParsed = true;
+    </script>
+</body>
+</html>
index 6ed7061..7c2de07 100644 (file)
@@ -92,10 +92,15 @@ PASS event.dataTransfer.dropEffect is "none"
 
 When effectAllowed == "dummy"
 
+PASS event.dataTransfer.effectAllowed is "uninitialized"
 PASS event.dataTransfer.dropEffect is "none"
-PASS event.dataTransfer.dropEffect is "none"
-PASS event.dataTransfer.dropEffect is "none"
-PASS event.dataTransfer.dropEffect is "none"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.dropEffect is "copy"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.dropEffect is "move"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
+PASS event.dataTransfer.dropEffect is "link"
+PASS event.dataTransfer.effectAllowed is "uninitialized"
 PASS event.dataTransfer.dropEffect is "none"
 
 TEST COMPLETE
index 842b607..92cbcd3 100644 (file)
             chosenEffectAllowed = "uninitialized";
         }
         
+        if (chosenEffectAllowed === "dummy") {
+            // If a bad effectAllowed is attempted to be set, it should never be set, and the
+            // effectAllowed should be uninitialized.
+            shouldBeEqualToString("event.dataTransfer.effectAllowed", "uninitialized");
+            
+            // Then set the chosenEffectAllowed so isDropEffectAllowed matches the HTML5 spec, and
+            // doesn't need special cases for undefined.
+            chosenEffectAllowed = "uninitialized";
+        }
+        
         if (isDropEffectAllowed(chosenDropEffect, chosenEffectAllowed))
             shouldBeEqualToString('event.dataTransfer.dropEffect', dropEffectElem.options[dropEffectElem.selectedIndex].value);
         else
index 67cac5d..0152ac7 100644 (file)
@@ -1,3 +1,22 @@
+2010-01-14  Brian Weinstein  <bweinstein@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        [DnD] effectAllowed and dropEffect can be set to bogus values.
+        Fixes <https://bugs.webkit.org/show_bug.cgi?id=33635>.
+        
+        Test to make aure dropEffect and effectAllowed are being set to valid values
+        when they are being set (list of valid values given by HTML5 specification).
+        
+        Also, drive by change to initialize dropEffect to none (as described in spec).
+
+        Test: fast/events/bogus-dropEffect-effectAllowed.html
+
+        * dom/Clipboard.cpp:
+        (WebCore::Clipboard::Clipboard): Initialize m_dropEffect to "none".
+        (WebCore::Clipboard::setDropEffect): Check if dropEffect is being set to a valid value.
+        (WebCore::Clipboard::setEffectAllowed): Check if effectAllowed is being set to a valid value.
+
 2010-01-14  Martin Robinson  <martin.james.robinson@gmail.com>
 
         Reviewed by Xan Lopez.
index b272f5d..2a0386f 100644 (file)
@@ -35,7 +35,8 @@
 namespace WebCore {
 
 Clipboard::Clipboard(ClipboardAccessPolicy policy, bool isForDragging) 
-    : m_policy(policy) 
+    : m_policy(policy)
+    , m_dropEffect("none")
     , m_effectAllowed("uninitialized")
     , m_dragStarted(false)
     , m_forDragging(isForDragging)
@@ -130,6 +131,10 @@ void Clipboard::setDropEffect(const String &effect)
     if (!m_forDragging)
         return;
 
+    // The attribute must ignore any attempts to set it to a value other than none, copy, link, and move. 
+    if (effect != "none" && effect != "copy"  && effect != "link" && effect != "move")
+        return;
+
     if (m_policy == ClipboardReadable || m_policy == ClipboardTypesReadable)
         m_dropEffect = effect;
 }
@@ -139,6 +144,17 @@ void Clipboard::setEffectAllowed(const String &effect)
     if (!m_forDragging)
         return;
 
+    if (dragOpFromIEOp(effect) == DragOperationPrivate) {
+        // This means that there was no conversion, and the effectAllowed that
+        // we are passed isn't a valid effectAllowed, so we should ignore it,
+        // and not set m_effectAllowed.
+
+        // The attribute must ignore any attempts to set it to a value other than 
+        // none, copy, copyLink, copyMove, link, linkMove, move, all, and uninitialized.
+        return;
+    }
+
+
     if (m_policy == ClipboardWritable)
         m_effectAllowed = effect;
 }