REGRESSION(r105396): drag state is not cleared after each drag
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jan 2012 00:47:28 +0000 (00:47 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jan 2012 00:47:28 +0000 (00:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=76878

Reviewed by Alexey Proskuryakov.

Source/WebCore:

Revert a part of r105396 that made performDragAndDrop not call clearDragState
when the default action was not prevented since it caused a regression.

I'm pretty certain always calling clearDragState in performDragAndDrop is wrong
but I can't think of a test case where this becomes a problem at the moment.
Since this area is not well tested, revert the change instead of making further
changes to the code base.

Tests: fast/events/clear-drag-state.html
       fast/events/clear-edit-drag-state.html

* page/EventHandler.cpp:
(WebCore::EventHandler::performDragAndDrop):

LayoutTests:

Add a regression test to ensure we don't fire extra dragenter event
on the second drag at an element that contains the dragged content.

* fast/events/clear-drag-state-expected.txt: Added.
* fast/events/clear-drag-state.html: Added.
* fast/events/clear-edit-drag-state-expected.txt: Added.
* fast/events/clear-edit-drag-state.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/clear-drag-state-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/clear-drag-state.html [new file with mode: 0644]
LayoutTests/fast/events/clear-edit-drag-state-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/clear-edit-drag-state.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp

index 60ab781..bf6a169 100644 (file)
@@ -1,3 +1,18 @@
+2012-01-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r105396): drag state is not cleared after each drag
+        https://bugs.webkit.org/show_bug.cgi?id=76878
+
+        Reviewed by Alexey Proskuryakov.
+
+        Add a regression test to ensure we don't fire extra dragenter event
+        on the second drag at an element that contains the dragged content.
+
+        * fast/events/clear-drag-state-expected.txt: Added.
+        * fast/events/clear-drag-state.html: Added.
+        * fast/events/clear-edit-drag-state-expected.txt: Added.
+        * fast/events/clear-edit-drag-state.html: Added.
+
 2012-01-23  Dale Curtis  <dalecurtis@chromium.org>
 
         Reduce throttling on video-buffering-repaints-controls test to prevent timeout.
diff --git a/LayoutTests/fast/events/clear-drag-state-expected.txt b/LayoutTests/fast/events/clear-drag-state-expected.txt
new file mode 100644 (file)
index 0000000..e7f9264
--- /dev/null
@@ -0,0 +1,4 @@
+This test ensures we clear the drag state at the end of each drag. To test manually, drag the green box below twice. You should observe the same list of events in the same order twice.
+
+Drag me down twice!
+PASS
diff --git a/LayoutTests/fast/events/clear-drag-state.html b/LayoutTests/fast/events/clear-drag-state.html
new file mode 100644 (file)
index 0000000..5140a60
--- /dev/null
@@ -0,0 +1,70 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+span, div { display: block; }
+span {
+  -webkit-user-drag: element;
+  -webkit-user-select: none;
+  background: green;
+  padding: 10px;
+}
+div {
+  background: red;
+  padding: 50px;
+}
+</style>
+</head>
+<body>
+<p>This test ensures we clear the drag state at the end of each drag. To test manually, drag the green box below twice.
+You should observe the same list of events in the same order twice.</p>
+<div>
+  <span>Drag me down twice!</span>
+</div>
+<pre id="log">
+</pre>
+<script>
+
+function log(event) {
+    document.getElementById('log').textContent += event.target.localName + ':' + this.localName + ':' + event.type + '\n';
+}
+
+function setLog(log) {
+    var value = document.getElementById('log').textContent;
+    document.getElementById('log').textContent = log;
+    return value;
+}
+
+var div = document.querySelector('div');
+div.addEventListener('dragover', function (event) { event.preventDefault(); });
+div.addEventListener('dragenter', log);
+div.addEventListener('dragleave', log);
+
+var span = document.querySelector('span');
+span.addEventListener('dragenter', log);
+span.addEventListener('dragleave', log);
+
+function dragSpan() {
+    var x = span.offsetLeft + span.offsetWidth / 2;
+    eventSender.mouseMoveTo(x, span.offsetTop + span.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.mouseMoveTo(x, span.offsetTop + 2 * span.offsetHeight / 3);
+    eventSender.leapForward(200);
+    eventSender.mouseMoveTo(x, span.offsetTop + 3 * span.offsetHeight / 2);
+    eventSender.leapForward(200);
+    eventSender.mouseUp();
+}
+
+if (window.eventSender) {
+    layoutTestController.dumpAsText();
+    dragSpan();
+    var firstLog = setLog('');
+    dragSpan();
+    var secondLog = setLog('PASS');
+    if (firstLog != secondLog)
+        setLog('FAIL:\nFirst drag:\n' + firstLog + '\nSecond drag:\n' + secondLog);
+}
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/events/clear-edit-drag-state-expected.txt b/LayoutTests/fast/events/clear-edit-drag-state-expected.txt
new file mode 100644 (file)
index 0000000..e7f9264
--- /dev/null
@@ -0,0 +1,4 @@
+This test ensures we clear the drag state at the end of each drag. To test manually, drag the green box below twice. You should observe the same list of events in the same order twice.
+
+Drag me down twice!
+PASS
diff --git a/LayoutTests/fast/events/clear-edit-drag-state.html b/LayoutTests/fast/events/clear-edit-drag-state.html
new file mode 100644 (file)
index 0000000..88551d2
--- /dev/null
@@ -0,0 +1,69 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+span, div { display: block; }
+span {
+  background: green;
+  padding: 10px;
+}
+div {
+  background: red;
+  padding: 50px;
+}
+</style>
+</head>
+<body>
+<p>This test ensures we clear the drag state at the end of each drag. To test manually, drag the green box below twice.
+You should observe the same list of events in the same order twice.</p>
+<div contenteditable>
+  <span>Drag me down twice!</span>
+</div>
+<pre id="log">
+</pre>
+<script>
+
+function log(event) {
+    document.getElementById('log').textContent += event.target.localName + ':' + this.localName + ':' + event.type + '\n';
+}
+
+function setLog(log) {
+    var value = document.getElementById('log').textContent;
+    document.getElementById('log').textContent = log;
+    return value;
+}
+
+var div = document.querySelector('div');
+div.addEventListener('dragover', function (event) { event.preventDefault(); });
+div.addEventListener('dragenter', log);
+div.addEventListener('dragleave', log);
+
+var span = document.querySelector('span');
+span.addEventListener('dragenter', log);
+span.addEventListener('dragleave', log);
+
+function dragSpan() {
+    var x = span.offsetLeft + span.offsetWidth / 2;
+    eventSender.mouseMoveTo(x, span.offsetTop + span.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.mouseMoveTo(x, span.offsetTop + 2 * span.offsetHeight / 3);
+    eventSender.leapForward(200);
+    eventSender.mouseMoveTo(x, span.offsetTop + 3 * span.offsetHeight / 2);
+    eventSender.leapForward(200);
+    eventSender.mouseUp();
+}
+
+if (window.eventSender) {
+    layoutTestController.dumpAsText();
+    getSelection().selectAllChildren(span);
+    dragSpan();
+    var firstLog = setLog('');
+    dragSpan();
+    var secondLog = setLog('PASS');
+    if (firstLog != secondLog)
+        setLog('FAIL:\nFirst drag:\n' + firstLog + '\nSecond drag:\n' + secondLog);
+}
+
+</script>
+</body>
+</html>
index d404b2d..dc0cffa 100644 (file)
@@ -1,3 +1,24 @@
+2012-01-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION(r105396): drag state is not cleared after each drag
+        https://bugs.webkit.org/show_bug.cgi?id=76878
+
+        Reviewed by Alexey Proskuryakov.
+
+        Revert a part of r105396 that made performDragAndDrop not call clearDragState
+        when the default action was not prevented since it caused a regression.
+
+        I'm pretty certain always calling clearDragState in performDragAndDrop is wrong
+        but I can't think of a test case where this becomes a problem at the moment.
+        Since this area is not well tested, revert the change instead of making further
+        changes to the code base.
+
+        Tests: fast/events/clear-drag-state.html
+               fast/events/clear-edit-drag-state.html
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::performDragAndDrop):
+
 2012-01-23  Thiago Marcos P. Santos  <tmpsantos@gmail.com>
 
         Fixed typo in exception messages
index 1e85da0..507a914 100644 (file)
@@ -1920,8 +1920,7 @@ bool EventHandler::performDragAndDrop(const PlatformMouseEvent& event, Clipboard
             preventedDefault = targetFrame->eventHandler()->performDragAndDrop(event, clipboard);
     } else if (m_dragTarget.get())
         preventedDefault = dispatchDragEvent(eventNames().dropEvent, m_dragTarget.get(), event, clipboard);
-    if (preventedDefault)
-        clearDragState();
+    clearDragState();
     return preventedDefault;
 }