Having a drop handler prevents navigation on drop even if event is not cancelled
authordcheng@chromium.org <dcheng@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Apr 2012 21:21:46 +0000 (21:21 +0000)
committerdcheng@chromium.org <dcheng@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Apr 2012 21:21:46 +0000 (21:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79172

Reviewed by Ryosuke Niwa.

Source/WebCore:

Only early return if the drop handler prevents the default action.
http://trac.webkit.org/changeset/105396 introduced this issue when fixing some other aspects
of DnD handling.

Test: fast/events/drop-handler-should-not-stop-navigate.html

* page/DragController.cpp:
(WebCore::DragController::performDrag):
(WebCore::DragController::concludeEditDrag): Remove the assert. By definition, we want to
    allow default actions to run now if they weren't explicitly canceled.

LayoutTests:

* fast/events/drag-dataTransferItemList.html: Fix drop handler to prevent default.
* fast/events/drop-handler-should-not-stop-navigate-expected.txt: Added.
* fast/events/drop-handler-should-not-stop-navigate.html: Added.
* http/tests/security/clipboard/clipboard-file-access.html: Change dragover to drop handler
    to prevent bubbled events from causing navigation.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/drag-dataTransferItemList.html
LayoutTests/fast/events/drop-handler-should-not-stop-navigate-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/drop-handler-should-not-stop-navigate.html [new file with mode: 0644]
LayoutTests/http/tests/security/clipboard/clipboard-file-access.html
Source/WebCore/ChangeLog
Source/WebCore/page/DragController.cpp

index 5b02890..eeb7667 100644 (file)
@@ -1,3 +1,16 @@
+2012-04-03  Daniel Cheng  <dcheng@chromium.org>
+
+        Having a drop handler prevents navigation on drop even if event is not cancelled
+        https://bugs.webkit.org/show_bug.cgi?id=79172
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/events/drag-dataTransferItemList.html: Fix drop handler to prevent default.
+        * fast/events/drop-handler-should-not-stop-navigate-expected.txt: Added.
+        * fast/events/drop-handler-should-not-stop-navigate.html: Added.
+        * http/tests/security/clipboard/clipboard-file-access.html: Change dragover to drop handler
+            to prevent bubbled events from causing navigation.
+
 2012-04-03  Erik Arvidsson  <arv@chromium.org>
 
         [v8] Fix memory leak in V8LazyEventListener
index bc332e9..17f0855 100644 (file)
@@ -131,6 +131,7 @@ function drop(event)
         legacyDrop(event.dataTransfer);
     else if (dropMethod.selectedIndex == 1)
         itemListDrop(event.dataTransfer);
+    event.preventDefault();
 }
 
 function runTest(dragMethodIndex, dropMethodIndex)
diff --git a/LayoutTests/fast/events/drop-handler-should-not-stop-navigate-expected.txt b/LayoutTests/fast/events/drop-handler-should-not-stop-navigate-expected.txt
new file mode 100644 (file)
index 0000000..e8a5ebc
--- /dev/null
@@ -0,0 +1,5 @@
+This tests that a drop handler's default action must be prevented in order to stop navigation. Otherwise, if event.preventDefault() is not called, navigation should occur. To test manually, simply drag and drop another link or HTML file on this page. If navigation occurs, then the test passed.
+
+Starting test
+Not preventing default event on drop.
+PASS
diff --git a/LayoutTests/fast/events/drop-handler-should-not-stop-navigate.html b/LayoutTests/fast/events/drop-handler-should-not-stop-navigate.html
new file mode 100644 (file)
index 0000000..97d5ace
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div>This tests that a drop handler's default action must be prevented in order to stop navigation.
+Otherwise, if event.preventDefault() is not called, navigation should occur. To test manually,
+simply drag and drop another link or HTML file on this page. If navigation occurs, then the test
+passed.</div>
+<script>
+function log(text)
+{
+    document.body.appendChild(document.createElement('br'));
+    document.body.appendChild(document.createElement('div').appendChild(document.createTextNode(text)));
+}
+window.addEventListener('beforeunload', function ()
+{
+    log('PASS');
+    layoutTestController.notifyDone();
+});
+document.body.addEventListener('dragenter', function (event)
+{
+    event.preventDefault();
+});
+document.body.addEventListener('dragover', function (event)
+{
+    event.preventDefault();
+});
+document.body.addEventListener('drop', function (event)
+{
+    log('Not preventing default event on drop.');
+});
+(function ()
+{
+    if (!window.layoutTestController)
+        return;
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+    log('Starting test');
+    eventSender.beginDragWithFiles(['DRTFakeFile']);
+    eventSender.mouseMoveTo(document.body.offsetLeft + 10, document.body.offsetTop + 10);
+    eventSender.mouseUp();
+    window.setTimeout(function ()
+    {
+        // Deadman's switch so we don't need to wait for the test to timeout to fail.
+        log('FAIL');
+        layoutTestController.notifyDone();
+    }, 0);
+})();
+</script>
+</body>
+</html>
index 9801223..1d23d59 100644 (file)
@@ -55,7 +55,7 @@ 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() {
+document.body.addEventListener("drop", function() {
     event.preventDefault();
 });
 
index 9345e7b..3430c21 100644 (file)
@@ -1,3 +1,21 @@
+2012-04-03  Daniel Cheng  <dcheng@chromium.org>
+
+        Having a drop handler prevents navigation on drop even if event is not cancelled
+        https://bugs.webkit.org/show_bug.cgi?id=79172
+
+        Reviewed by Ryosuke Niwa.
+
+        Only early return if the drop handler prevents the default action.
+        http://trac.webkit.org/changeset/105396 introduced this issue when fixing some other aspects
+        of DnD handling.
+
+        Test: fast/events/drop-handler-should-not-stop-navigate.html
+
+        * page/DragController.cpp:
+        (WebCore::DragController::performDrag):
+        (WebCore::DragController::concludeEditDrag): Remove the assert. By definition, we want to
+            allow default actions to run now if they weren't explicitly canceled.
+
 2012-04-03  Erik Arvidsson  <arv@chromium.org>
 
         [v8] Fix memory leak in V8LazyEventListener
index dfeb529..cfdc9c5 100644 (file)
@@ -212,7 +212,7 @@ bool DragController::performDrag(DragData* dragData)
             preventedDefault = mainFrame->eventHandler()->performDragAndDrop(createMouseEvent(dragData), clipboard.get());
             clipboard->setAccessPolicy(ClipboardNumb); // Invalidate clipboard here for security
         }
-        if (m_isHandlingDrag || preventedDefault) {
+        if (preventedDefault) {
             m_documentUnderMouse = 0;
             return true;
         }
@@ -422,7 +422,6 @@ bool DragController::dispatchTextInputEventFor(Frame* innerFrame, DragData* drag
 bool DragController::concludeEditDrag(DragData* dragData)
 {
     ASSERT(dragData);
-    ASSERT(!m_isHandlingDrag);
 
     RefPtr<HTMLInputElement> fileInput = m_fileInputElementUnderMouse;
     if (m_fileInputElementUnderMouse) {