dragover's default action should prevent drop for file drags
authordcheng@chromium.org <dcheng@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Oct 2012 01:31:08 +0000 (01:31 +0000)
committerdcheng@chromium.org <dcheng@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Oct 2012 01:31:08 +0000 (01:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79173

Reviewed by Tony Chang.

Source/WebCore:

During a file drag, we need to keep track of whether or not the document has cancelled the
dragover action. We should only send a drop event if the dragover event was cancelled; this
matches the behavior of the spec, as well as IE, Gecko, and Opera. The relevant sections
from the spec are the sections pertaining to dragover and drop events at:
http://www.whatwg.org/specs/web-apps/current-work/#drag-and-drop-processing-model

Test: fast/events/only-valid-drop-targets-receive-file-drop.html

* page/DragController.cpp:
(WebCore::DragController::performDrag):
(WebCore::DragController::dragEnteredOrUpdated):
(WebCore::DragController::tryDocumentDrag):
* page/DragController.h:
(DragController): Cleanup to repurpose a variable that doesn't need to be a member anymore
                  and remove the corresponding getter/setter.

LayoutTests:

* fast/dom/shadow/drop-event-in-shadow.html:
    Added dragover handler as required by the HTML specification.
* fast/events/input-element-display-none-in-dragleave-crash.html:
    Added dragover handler as required by the HTML specification.
* fast/events/only-valid-drop-targets-receive-file-drop-expected.txt: Added.
* fast/events/only-valid-drop-targets-receive-file-drop.html: Added.
* http/tests/security/clipboard/clipboard-file-access.html:
    Added dragover handler as required by the HTML specification.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/shadow/drop-event-in-shadow.html
LayoutTests/fast/events/input-element-display-none-in-dragleave-crash.html
LayoutTests/fast/events/only-valid-drop-targets-receive-file-drop-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/only-valid-drop-targets-receive-file-drop.html [new file with mode: 0644]
LayoutTests/http/tests/security/clipboard/clipboard-file-access.html
Source/WebCore/ChangeLog
Source/WebCore/page/DragController.cpp
Source/WebCore/page/DragController.h

index 279d82c..49a1312 100644 (file)
@@ -1,3 +1,19 @@
+2012-10-26  Daniel Cheng  <dcheng@chromium.org>
+
+        dragover's default action should prevent drop for file drags
+        https://bugs.webkit.org/show_bug.cgi?id=79173
+
+        Reviewed by Tony Chang.
+
+        * fast/dom/shadow/drop-event-in-shadow.html:
+            Added dragover handler as required by the HTML specification.
+        * fast/events/input-element-display-none-in-dragleave-crash.html:
+            Added dragover handler as required by the HTML specification.
+        * fast/events/only-valid-drop-targets-receive-file-drop-expected.txt: Added.
+        * fast/events/only-valid-drop-targets-receive-file-drop.html: Added.
+        * http/tests/security/clipboard/clipboard-file-access.html:
+            Added dragover handler as required by the HTML specification.
+
 2012-10-26  Anders Carlsson  <andersca@apple.com>
 
         Crash when making NPRuntime calls with a null NPP pointer
index 1d52346..fa928af 100644 (file)
@@ -10,6 +10,9 @@ function createBox(name) {
     div.style.width = '100px';
     div.style.height = '100px';
 
+    div.addEventListener('dragover', function(e) {
+        e.preventDefault();
+    });
     div.addEventListener('drop', function(e) {
         debug('PASS: drop event is fired.');
     });
index 4ff96e9..bdd9a6c 100644 (file)
                 // The test harness wants us to call eventSender.mouseUp() before finishing,
                 // but we need to not navigate when that happens, so add a drop handler that
                 // prevents navigation.
+                window.addEventListener("dragover", function() {
+                    event.preventDefault();
+                }, false);
                 window.addEventListener("drop", function() {
-                  event.preventDefault();
+                    event.preventDefault();
                 }, false);
                 eventSender.mouseUp();
             }
-            
+
             if (window.testRunner)
                 testRunner.notifyDone();
         }
@@ -39,4 +42,4 @@
     <p>This test passes if there is no crash when dragging a file over and then away from the file input element below.</p>
     <input type="file" id="drop-target">
 </body>
-</html>
\ No newline at end of file
+</html>
diff --git a/LayoutTests/fast/events/only-valid-drop-targets-receive-file-drop-expected.txt b/LayoutTests/fast/events/only-valid-drop-targets-receive-file-drop-expected.txt
new file mode 100644 (file)
index 0000000..677555a
--- /dev/null
@@ -0,0 +1,9 @@
+To run this test manually, drag a file to one of the two boxes below.
+
+Dropping in drop target 1 should result in a drop event.
+Dropping in drop target 2 should NOT result in a drop event (page will navigate).
+Starting drag...
+Drop received in drop target 1
+Starting drag...
+Drop not received
+
diff --git a/LayoutTests/fast/events/only-valid-drop-targets-receive-file-drop.html b/LayoutTests/fast/events/only-valid-drop-targets-receive-file-drop.html
new file mode 100644 (file)
index 0000000..08cb537
--- /dev/null
@@ -0,0 +1,81 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+div, span {
+    border: 1px solid black;
+};
+</style>
+<script>
+var dropReceived;
+function log(str)
+{
+    var result = document.getElementById('result');
+    result.appendChild(document.createTextNode(str));
+    result.appendChild(document.createElement('br'));
+}
+function dragDrop(target)
+{
+    log('Starting drag...');
+    eventSender.beginDragWithFiles(['test']);
+    eventSender.leapForward(100);
+    eventSender.mouseMoveTo(target.offsetLeft + target.offsetWidth / 2,
+                            target.offsetTop + target.offsetHeight / 2);
+    eventSender.mouseUp();
+}
+window.onload = function()
+{
+    var drop1 = document.getElementById('drop1');
+    var drop2 = document.getElementById('drop2');
+
+    document.body.addEventListener('dragover', function () {
+        dropReceived = false;
+    }, false);
+
+    drop1.addEventListener('dragover', function(e) {
+        e.preventDefault();
+    }, false);
+    drop1.addEventListener('drop', function(e) {
+        e.preventDefault();
+        dropReceived = true;
+        log('Drop received in drop target 1');
+    }, false);
+
+    drop2.addEventListener('dragover', function() {
+    }, false);
+    drop2.addEventListener('drop', function(e) {
+        e.preventDefault();
+        dropReceived = true;
+        log('Drop received in drop target 2');
+    }, false);
+
+    if (!window.testRunner)
+        return;
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+
+    window.addEventListener('beforeunload', function () {
+        if (!dropReceived)
+            log('Drop not received');
+        testRunner.notifyDone();
+    });
+
+    dragDrop(drop1);
+    unloadExpected = true;
+    dragDrop(drop2);
+    window.setTimeout(function () {
+        // Deadman's switch to fail quickly.
+        log('FAIL: navigation expected');
+        testRunner.notifyDone();
+    }, 0);
+}
+</script>
+</head>
+<body>
+<p>To run this test manually, drag a file to one of the two boxes below.
+<div id="drop1">Dropping in drop target 1 should result in a drop event.</div>
+<div id="drop2">Dropping in drop target 2 should NOT result in a drop event (page will navigate).</div>
+<div id="result">
+</div>
+</body>
+</html>
index 1d23d59..282d931 100644 (file)
@@ -55,6 +55,9 @@ dragTarget.addEventListener("drop", function() {
 // Some tests don't end up dropping the draggee over the drag target. Catch any
 // leftover drop events bubbling up through the tree so they don't cause page
 // navigation.
+document.body.addEventListener("dragover", function() {
+    event.preventDefault();
+});
 document.body.addEventListener("drop", function() {
     event.preventDefault();
 });
index e16d540..3bd378a 100644 (file)
@@ -1,3 +1,26 @@
+2012-10-26  Daniel Cheng  <dcheng@chromium.org>
+
+        dragover's default action should prevent drop for file drags
+        https://bugs.webkit.org/show_bug.cgi?id=79173
+
+        Reviewed by Tony Chang.
+
+        During a file drag, we need to keep track of whether or not the document has cancelled the
+        dragover action. We should only send a drop event if the dragover event was cancelled; this
+        matches the behavior of the spec, as well as IE, Gecko, and Opera. The relevant sections
+        from the spec are the sections pertaining to dragover and drop events at:
+        http://www.whatwg.org/specs/web-apps/current-work/#drag-and-drop-processing-model
+
+        Test: fast/events/only-valid-drop-targets-receive-file-drop.html
+
+        * page/DragController.cpp:
+        (WebCore::DragController::performDrag):
+        (WebCore::DragController::dragEnteredOrUpdated):
+        (WebCore::DragController::tryDocumentDrag):
+        * page/DragController.h:
+        (DragController): Cleanup to repurpose a variable that doesn't need to be a member anymore
+                          and remove the corresponding getter/setter.
+
 2012-10-26  Nico Weber  <thakis@chromium.org>
 
         Fix a operator ordering bug in SVGSMILElement::calculateAnimationPercentAndRepeat
index 2e0aafb..81ea3f9 100644 (file)
@@ -96,10 +96,10 @@ DragController::DragController(Page* page, DragClient* client)
     , m_documentUnderMouse(0)
     , m_dragInitiator(0)
     , m_fileInputElementUnderMouse(0)
+    , m_documentIsHandlingDrag(false)
     , m_dragDestinationAction(DragDestinationActionNone)
     , m_dragSourceAction(DragSourceActionNone)
     , m_didInitiateDrag(false)
-    , m_isHandlingDrag(false)
     , m_sourceDragOperation(DragOperationNone)
 {
     ASSERT(m_client);
@@ -209,7 +209,7 @@ bool DragController::performDrag(DragData* dragData)
 {
     ASSERT(dragData);
     m_documentUnderMouse = m_page->mainFrame()->documentAtPoint(dragData->clientPosition());
-    if (m_dragDestinationAction & DragDestinationActionDHTML) {
+    if ((m_dragDestinationAction & DragDestinationActionDHTML) && m_documentIsHandlingDrag) {
         m_client->willPerformDragDestinationAction(DragDestinationActionDHTML, dragData);
         RefPtr<Frame> mainFrame = m_page->mainFrame();
         bool preventedDefault = false;
@@ -265,8 +265,8 @@ DragSession DragController::dragEnteredOrUpdated(DragData* dragData)
     }
 
     DragSession dragSession;
-    bool handledByDocument = tryDocumentDrag(dragData, m_dragDestinationAction, dragSession);
-    if (!handledByDocument && (m_dragDestinationAction & DragDestinationActionLoad))
+    m_documentIsHandlingDrag = tryDocumentDrag(dragData, m_dragDestinationAction, dragSession);
+    if (!m_documentIsHandlingDrag && (m_dragDestinationAction & DragDestinationActionLoad))
         dragSession.operation = operationForLoad(dragData);
     return dragSession;
 }
@@ -314,9 +314,9 @@ bool DragController::tryDocumentDrag(DragData* dragData, DragDestinationAction a
     if (m_dragInitiator && !m_documentUnderMouse->securityOrigin()->canReceiveDragData(m_dragInitiator->securityOrigin()))
         return false;
 
-    m_isHandlingDrag = false;
+    bool isHandlingDrag = false;
     if (actionMask & DragDestinationActionDHTML) {
-        m_isHandlingDrag = tryDHTMLDrag(dragData, dragSession.operation);
+        isHandlingDrag = tryDHTMLDrag(dragData, dragSession.operation);
         // Do not continue if m_documentUnderMouse has been reset by tryDHTMLDrag.
         // tryDHTMLDrag fires dragenter event. The event listener that listens
         // to this event may create a nested message loop (open a modal dialog),
@@ -332,10 +332,12 @@ bool DragController::tryDocumentDrag(DragData* dragData, DragDestinationAction a
     if (!frameView)
         return false;
 
-    if (m_isHandlingDrag) {
+    if (isHandlingDrag) {
         m_page->dragCaretController()->clear();
         return true;
-    } else if ((actionMask & DragDestinationActionEdit) && canProcessDrag(dragData)) {
+    }
+
+    if ((actionMask & DragDestinationActionEdit) && canProcessDrag(dragData)) {
         if (dragData->containsColor()) {
             dragSession.operation = DragOperationGeneric;
             return true;
index 74d6178..c7927cb 100644 (file)
@@ -68,8 +68,6 @@ namespace WebCore {
         // drag logic is in WebCore.
         void setDidInitiateDrag(bool initiated) { m_didInitiateDrag = initiated; } 
         bool didInitiateDrag() const { return m_didInitiateDrag; }
-        void setIsHandlingDrag(bool handling) { m_isHandlingDrag = handling; }
-        bool isHandlingDrag() const { return m_isHandlingDrag; }
         DragOperation sourceDragOperation() const { return m_sourceDragOperation; }
         const KURL& draggingImageURL() const { return m_draggingImageURL; }
         void setDragOffset(const IntPoint& offset) { m_dragOffset = offset; }
@@ -119,15 +117,15 @@ namespace WebCore {
 
         Page* m_page;
         DragClient* m_client;
-        
+
         RefPtr<Document> m_documentUnderMouse; // The document the mouse was last dragged over.
         RefPtr<Document> m_dragInitiator; // The Document (if any) that initiated the drag.
         RefPtr<HTMLInputElement> m_fileInputElementUnderMouse;
-        
+        bool m_documentIsHandlingDrag;
+
         DragDestinationAction m_dragDestinationAction;
         DragSourceAction m_dragSourceAction;
         bool m_didInitiateDrag;
-        bool m_isHandlingDrag;
         DragOperation m_sourceDragOperation; // Set in startDrag when a drag starts from a mouse down within WebKit
         IntPoint m_dragOffset;
         KURL m_draggingImageURL;