Mouseup event does not fire on Scroll Bar
authormkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Feb 2013 04:21:32 +0000 (04:21 +0000)
committermkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Feb 2013 04:21:32 +0000 (04:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=25811

Reviewed by Tony Chang.

Source/WebCore:

Currently, clicking on a scrollbar fires a mousedown event, but not a
mouseup event. This causes problems for code like jQuery UI's
draggable[1], as the drag starts, but is never cancelled. Other use
cases are noted in the slightly old Chromium bug[2].

If a mouseup event is received after a mousedown event on a scrollbar,
this patch dispatches a mouseup event on the same node the mousedown
event dispatched on. This matches Gecko's behavior.

[1]: http://bugs.jqueryui.com/ticket/6925
[2]: http://crbug.com/14204

Tests: fast/scrolling/scrollbar-mousedown-mouseup.html
       fast/scrolling/scrollbar-mousedown-move-mouseup.html

* page/EventHandler.cpp:
(WebCore::EventHandler::handleMouseReleaseEvent):
    If a mouseup event follow a mousedown event on a scrollbar,
    dispatch an event on the same node from which the mousedown event
    was triggered.

LayoutTests:

* fast/scrolling/scrollbar-mousedown-mouseup-expected.txt: Added.
* fast/scrolling/scrollbar-mousedown-mouseup.html: Added.
* fast/scrolling/scrollbar-mousedown-move-mouseup-expected.txt: Added.
* fast/scrolling/scrollbar-mousedown-move-mouseup.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/scrolling/scrollbar-mousedown-mouseup-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/scrollbar-mousedown-mouseup.html [new file with mode: 0644]
LayoutTests/fast/scrolling/scrollbar-mousedown-move-mouseup-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/scrollbar-mousedown-move-mouseup.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/EventHandler.cpp

index e1f4128..61c494d 100644 (file)
@@ -1,3 +1,15 @@
+2013-02-20  Mike West  <mkwst@chromium.org>
+
+        Mouseup event does not fire on Scroll Bar
+        https://bugs.webkit.org/show_bug.cgi?id=25811
+
+        Reviewed by Tony Chang.
+
+        * fast/scrolling/scrollbar-mousedown-mouseup-expected.txt: Added.
+        * fast/scrolling/scrollbar-mousedown-mouseup.html: Added.
+        * fast/scrolling/scrollbar-mousedown-move-mouseup-expected.txt: Added.
+        * fast/scrolling/scrollbar-mousedown-move-mouseup.html: Added.
+
 2013-02-20  Takashi Toyoshima  <toyoshim@chromium.org>
 
         Unreviewed gardening, update TestExpectations.
diff --git a/LayoutTests/fast/scrolling/scrollbar-mousedown-mouseup-expected.txt b/LayoutTests/fast/scrolling/scrollbar-mousedown-mouseup-expected.txt
new file mode 100644 (file)
index 0000000..944f225
--- /dev/null
@@ -0,0 +1,19 @@
+This is a scrollable div.
+
+PASS events.length is 4
+PASS events[0].type is "mousedown"
+PASS events[0].target.id is "scrollme"
+PASS events[0].which is 1
+PASS events[1].type is "mouseup"
+PASS events[1].target.id is "scrollme"
+PASS events[1].which is 1
+PASS events[2].type is "mousedown"
+PASS events[2].target.id is "scrollme"
+PASS events[2].which is 2
+PASS events[3].type is "mouseup"
+PASS events[3].target.id is "scrollme"
+PASS events[3].which is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/scrollbar-mousedown-mouseup.html b/LayoutTests/fast/scrolling/scrollbar-mousedown-mouseup.html
new file mode 100644 (file)
index 0000000..5414a1d
--- /dev/null
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="../js/resources/js-test-pre.js"></script>
+    <script>
+        window.jsTestIsAsync = true;
+
+        function initEventHandlers(element) {
+            element.addEventListener('mousedown', handleEvent);
+            element.addEventListener('mouseup', handleEvent);
+            element.addEventListener('click', handleEvent);
+        }
+
+        window.events = [];
+        function handleEvent(e) {
+            window.events.push(e);
+        }
+
+        function finish() {
+            shouldBe("events.length", "4");
+            shouldBeEqualToString("events[0].type", "mousedown");
+            shouldBeEqualToString("events[0].target.id", "scrollme");
+            shouldBe("events[0].which", "1");
+            shouldBeEqualToString("events[1].type", "mouseup");
+            shouldBeEqualToString("events[1].target.id", "scrollme");
+            shouldBe("events[1].which", "1");
+            shouldBeEqualToString("events[2].type", "mousedown");
+            shouldBeEqualToString("events[2].target.id", "scrollme");
+            shouldBe("events[2].which", "2");
+            shouldBeEqualToString("events[3].type", "mouseup");
+            shouldBeEqualToString("events[3].target.id", "scrollme");
+            shouldBe("events[3].which", "2");
+            finishJSTest();
+        }
+
+        window.onload = function () {
+            var d = document.querySelector('#scrollme');
+            initEventHandlers(d);
+
+            if (window.eventSender) {
+                eventSender.mouseMoveTo(d.offsetLeft + d.offsetWidth - 4, d.offsetTop + 4);
+                eventSender.mouseDown();
+                eventSender.mouseUp();
+                eventSender.mouseDown(1);
+                eventSender.mouseUp(1);
+                finish();
+            } else
+                debug('This test requires eventSender. Click the scrollbar to play manually.');
+        };
+    </script>
+    <script src="../js/resources/js-test-post.js"></script>
+    <style>
+        #scrollme {
+            width: 100px;
+            height: 100px;
+            overflow: auto;
+        }
+        #scrollme p {
+            height: 1000px;
+        }
+    </style>
+</head>
+<body>
+    <div id="scrollme"><p>This is a scrollable div.</p></div>
+    <pre id="console"></pre>
+</body>
+</html>
diff --git a/LayoutTests/fast/scrolling/scrollbar-mousedown-move-mouseup-expected.txt b/LayoutTests/fast/scrolling/scrollbar-mousedown-move-mouseup-expected.txt
new file mode 100644 (file)
index 0000000..355659c
--- /dev/null
@@ -0,0 +1,16 @@
+This is a scrollable div.
+
+PASS events['scrollme'].length is 4
+PASS events['notscrollme'].length is 0
+PASS events['scrollme'][0].type is "mousedown"
+PASS events['scrollme'][0].which is 1
+PASS events['scrollme'][1].type is "mouseup"
+PASS events['scrollme'][1].which is 1
+PASS events['scrollme'][2].type is "mousedown"
+PASS events['scrollme'][2].which is 2
+PASS events['scrollme'][3].type is "mouseup"
+PASS events['scrollme'][3].which is 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/scrolling/scrollbar-mousedown-move-mouseup.html b/LayoutTests/fast/scrolling/scrollbar-mousedown-move-mouseup.html
new file mode 100644 (file)
index 0000000..397929c
--- /dev/null
@@ -0,0 +1,72 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="../js/resources/js-test-pre.js"></script>
+    <script>
+        window.jsTestIsAsync = true;
+
+        function initEventHandlers(element) {
+            element.addEventListener('mousedown', handleEvent);
+            element.addEventListener('mouseup', handleEvent);
+        }
+
+        window.events = {
+            'scrollme': [],
+            'notscrollme': []
+        };
+        function handleEvent(e) {
+            window.events[e.target.id].push(e);
+        }
+
+        function finish() {
+            shouldBe("events['scrollme'].length", "4");
+            shouldBe("events['notscrollme'].length", "0");
+            shouldBeEqualToString("events['scrollme'][0].type", "mousedown");
+            shouldBe("events['scrollme'][0].which", "1");
+            shouldBeEqualToString("events['scrollme'][1].type", "mouseup");
+            shouldBe("events['scrollme'][1].which", "1");
+            shouldBeEqualToString("events['scrollme'][2].type", "mousedown");
+            shouldBe("events['scrollme'][2].which", "2");
+            shouldBeEqualToString("events['scrollme'][3].type", "mouseup");
+            shouldBe("events['scrollme'][3].which", "2");
+            finishJSTest();
+        }
+
+        window.onload = function () {
+            var d1 = document.querySelector('#scrollme');
+            var d2 = document.querySelector('#notscrollme');
+            initEventHandlers(d1);
+            initEventHandlers(d2);
+
+            if (window.eventSender) {
+                eventSender.mouseMoveTo(d1.offsetLeft + d1.offsetWidth - 4, d1.offsetTop + 4);
+                eventSender.mouseDown();
+                eventSender.mouseMoveTo(d2.offsetLeft + d2.offsetWidth - 4, d2.offsetTop + 4);
+                eventSender.mouseUp();
+                eventSender.mouseMoveTo(d1.offsetLeft + d1.offsetWidth - 4, d1.offsetTop + 4);
+                eventSender.mouseDown(1);
+                eventSender.mouseMoveTo(d2.offsetLeft + d2.offsetWidth - 4, d2.offsetTop + 4);
+                eventSender.mouseUp(1);
+                finish();
+            } else
+                debug('This test requires eventSender. Click the scrollbar to play manually.');
+        };
+    </script>
+    <script src="../js/resources/js-test-post.js"></script>
+    <style>
+        #scrollme, #notscrollme {
+            width: 100px;
+            height: 100px;
+            overflow: auto;
+        }
+        #scrollme p {
+            height: 1000px;
+        }
+    </style>
+</head>
+<body>
+    <div id="scrollme"><p>This is a scrollable div.</p></div>
+    <div id="notscrollme"></div>
+    <pre id="console"></pre>
+</body>
+</html>
index c8e116d..01fb625 100644 (file)
@@ -1,3 +1,31 @@
+2013-02-20  Mike West  <mkwst@chromium.org>
+
+        Mouseup event does not fire on Scroll Bar
+        https://bugs.webkit.org/show_bug.cgi?id=25811
+
+        Reviewed by Tony Chang.
+
+        Currently, clicking on a scrollbar fires a mousedown event, but not a
+        mouseup event. This causes problems for code like jQuery UI's
+        draggable[1], as the drag starts, but is never cancelled. Other use
+        cases are noted in the slightly old Chromium bug[2].
+
+        If a mouseup event is received after a mousedown event on a scrollbar,
+        this patch dispatches a mouseup event on the same node the mousedown
+        event dispatched on. This matches Gecko's behavior.
+
+        [1]: http://bugs.jqueryui.com/ticket/6925
+        [2]: http://crbug.com/14204
+
+        Tests: fast/scrolling/scrollbar-mousedown-mouseup.html
+               fast/scrolling/scrollbar-mousedown-move-mouseup.html
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleMouseReleaseEvent):
+            If a mouseup event follow a mousedown event on a scrollbar,
+            dispatch an event on the same node from which the mousedown event
+            was triggered.
+
 2013-02-20  Takashi Sakamoto  <tasak@google.com>
 
         [Refactoring] Make m_state an on-stack object
index fdbb520..b127ae7 100644 (file)
@@ -1829,7 +1829,10 @@ bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)
 
     if (m_lastScrollbarUnderMouse) {
         invalidateClick();
-        return m_lastScrollbarUnderMouse->mouseUp(mouseEvent);
+        m_lastScrollbarUnderMouse->mouseUp(mouseEvent);
+        bool cancelable = true;
+        bool setUnder = false;
+        return !dispatchMouseEvent(eventNames().mouseupEvent, m_lastNodeUnderMouse.get(), cancelable, m_clickCount, mouseEvent, setUnder);
     }
 
     HitTestRequest request(HitTestRequest::Release);