Title: [197784] trunk
Revision
197784
Author
toniki...@webkit.org
Date
2016-03-08 10:57:54 -0800 (Tue, 08 Mar 2016)

Log Message

Scrolling does not work when the mouse down is handled by a node
https://bugs.webkit.org/show_bug.cgi?id=19033

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/events/prevent-default-prevents-interaction-with-scrollbars-.html

When a mouse press/down event happens on a scrollbar area, but event
is default prevented in the document level**, for example, event does not get
properly passed to scrollbars, although it should.

Problem started long ago with r17770, and was improved with r19596.
However, years later, the way Scrollbar* is obtained is still currently different
weither event is default prevented or not.

Patch uniforms the logic for both cases, and fixes the bug.

Note: code before used to look like

if (swallowEvent) {
    <code>
} else {
    <bleh>
    <foo>
}

.. and now looks like

if (!swallowEvent)
    <bleh>

<code>

if (!swallowEvent)
    <foo>

** e.g. document.addEventListener('mousedown', function (e) { e.preventDefault(); });

* page/EventHandler.cpp:
(WebCore::scrollbarForMouseEvent):
(WebCore::EventHandler::handleMousePressEvent):

LayoutTests:

* fast/events/prevent-default-prevents-interaction-with-scrollbars-expected.txt: Added.
* fast/events/prevent-default-prevents-interaction-with-scrollbars.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (197783 => 197784)


--- trunk/LayoutTests/ChangeLog	2016-03-08 18:54:05 UTC (rev 197783)
+++ trunk/LayoutTests/ChangeLog	2016-03-08 18:57:54 UTC (rev 197784)
@@ -1,3 +1,13 @@
+2016-03-08  Antonio Gomes  <toniki...@webkit.org>
+
+        Scrolling does not work when the mouse down is handled by a node
+        https://bugs.webkit.org/show_bug.cgi?id=19033
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/prevent-default-prevents-interaction-with-scrollbars-expected.txt: Added.
+        * fast/events/prevent-default-prevents-interaction-with-scrollbars.html: Added.
+
 2016-03-08  Michael Saboff  <msab...@apple.com>
 
         [ES6] Regular _expression_ canonicalization tables for Unicode need to be updated to use Unicode CaseFolding.txt

Added: trunk/LayoutTests/fast/events/prevent-default-prevents-interaction-with-scrollbars-expected.txt (0 => 197784)


--- trunk/LayoutTests/fast/events/prevent-default-prevents-interaction-with-scrollbars-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/prevent-default-prevents-interaction-with-scrollbars-expected.txt	2016-03-08 18:57:54 UTC (rev 197784)
@@ -0,0 +1,3 @@
+This is a test for https://bugs.webkit.org/show_bug.cgi?id=19033
+
+PASS: scrollbar click could scroll document.

Added: trunk/LayoutTests/fast/events/prevent-default-prevents-interaction-with-scrollbars.html (0 => 197784)


--- trunk/LayoutTests/fast/events/prevent-default-prevents-interaction-with-scrollbars.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/prevent-default-prevents-interaction-with-scrollbars.html	2016-03-08 18:57:54 UTC (rev 197784)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<head>
+    <style type="text/css" media="screen">
+        body {
+            height: 5000px;
+        }
+    </style>
+    <script>
+        function scrollTest()
+        {
+            eventSender.mouseMoveTo(window.innerWidth - 4, window.innerHeight - 20);
+            eventSender.mouseDown();
+            eventSender.mouseUp();
+
+            setTimeout('checkOffset();', 100);
+        }
+
+        function checkOffset()
+        {
+            var scroller = document.scrollingElement;
+            if (scroller.scrollTop != 0)
+                document.getElementById('result').textContent = "PASS: scrollbar click could scroll document.";
+            else
+                document.getElementById('result').textContent = "FAIL: scrollbar click could not scroll document.";
+            testRunner.notifyDone();
+        }
+
+        function startTest()
+        {
+            if (window.eventSender) {
+                testRunner.dumpAsText();
+                testRunner.waitUntilDone();
+
+                setTimeout(scrollTest, 0);
+            }
+        }
+
+        window.addEventListener('load', startTest, false);
+        document.addEventListener('mousedown', function (e) { e.preventDefault(); });
+    </script>
+</head>
+<body style="min-height: 5000px">
+    <p>This is a test for <a href=""
+    <div id="result"></div>
+</body>

Modified: trunk/Source/WebCore/ChangeLog (197783 => 197784)


--- trunk/Source/WebCore/ChangeLog	2016-03-08 18:54:05 UTC (rev 197783)
+++ trunk/Source/WebCore/ChangeLog	2016-03-08 18:57:54 UTC (rev 197784)
@@ -1,3 +1,47 @@
+2016-03-08  Antonio Gomes  <toniki...@webkit.org>
+
+        Scrolling does not work when the mouse down is handled by a node
+        https://bugs.webkit.org/show_bug.cgi?id=19033
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/events/prevent-default-prevents-interaction-with-scrollbars-.html
+
+        When a mouse press/down event happens on a scrollbar area, but event
+        is default prevented in the document level**, for example, event does not get
+        properly passed to scrollbars, although it should.
+
+        Problem started long ago with r17770, and was improved with r19596.
+        However, years later, the way Scrollbar* is obtained is still currently different
+        weither event is default prevented or not.
+
+        Patch uniforms the logic for both cases, and fixes the bug.
+
+        Note: code before used to look like
+
+        if (swallowEvent) {
+            <code>
+        } else {
+            <bleh>
+            <foo>
+        }
+
+        .. and now looks like
+
+        if (!swallowEvent)
+            <bleh>
+
+        <code>
+
+        if (!swallowEvent)
+            <foo>
+
+        ** e.g. document.addEventListener('mousedown', function (e) { e.preventDefault(); });
+
+        * page/EventHandler.cpp:
+        (WebCore::scrollbarForMouseEvent):
+        (WebCore::EventHandler::handleMousePressEvent):
+
 2016-03-08  Chris Dumez  <cdu...@apple.com>
 
         Unreviewed Windows build fix after r197728.

Modified: trunk/Source/WebCore/page/EventHandler.cpp (197783 => 197784)


--- trunk/Source/WebCore/page/EventHandler.cpp	2016-03-08 18:54:05 UTC (rev 197783)
+++ trunk/Source/WebCore/page/EventHandler.cpp	2016-03-08 18:57:54 UTC (rev 197784)
@@ -1577,6 +1577,16 @@
     return view ? view->windowToContents(windowPoint) : windowPoint;
 }
 
+static Scrollbar* scrollbarForMouseEvent(const MouseEventWithHitTestResults& mouseEvent, FrameView* view)
+{
+    if (view) {
+        if (auto* scrollbar = view->scrollbarAtPoint(mouseEvent.event().position()))
+            return scrollbar;
+    }
+    return mouseEvent.scrollbar();
+
+}
+
 bool EventHandler::handleMousePressEvent(const PlatformMouseEvent& platformMouseEvent)
 {
     RefPtr<FrameView> protector(m_frame.view());
@@ -1693,35 +1703,25 @@
             m_lastScrollbarUnderMouse = nullptr;
     }
 
-    if (swallowEvent) {
-        // scrollbars should get events anyway, even disabled controls might be scrollable
-        Scrollbar* scrollbar = mouseEvent.scrollbar();
-
-        updateLastScrollbarUnderMouse(scrollbar, true);
-
-        if (scrollbar)
-            passMousePressEventToScrollbar(mouseEvent, scrollbar);
-    } else {
+    if (!swallowEvent) {
         // Refetch the event target node if it currently is the shadow node inside an <input> element.
         // If a mouse event handler changes the input element type to one that has a widget associated,
         // we'd like to EventHandler::handleMousePressEvent to pass the event to the widget and thus the
         // event target node can't still be the shadow node.
         if (is<ShadowRoot>(*mouseEvent.targetNode()) && is<HTMLInputElement>(*downcast<ShadowRoot>(*mouseEvent.targetNode()).host()))
             mouseEvent = m_frame.document()->prepareMouseEvent(HitTestRequest(), documentPoint, platformMouseEvent);
+    }
 
-        FrameView* view = m_frame.view();
-        Scrollbar* scrollbar = view ? view->scrollbarAtPoint(platformMouseEvent.position()) : 0;
-        if (!scrollbar)
-            scrollbar = mouseEvent.scrollbar();
+    Scrollbar* scrollbar = scrollbarForMouseEvent(mouseEvent, m_frame.view());
+    updateLastScrollbarUnderMouse(scrollbar, true);
 
-        updateLastScrollbarUnderMouse(scrollbar, true);
-
-        if (scrollbar && passMousePressEventToScrollbar(mouseEvent, scrollbar))
+    bool passedToScrollbar = scrollbar && passMousePressEventToScrollbar(mouseEvent, scrollbar);
+    if (!swallowEvent) {
+        if (passedToScrollbar)
             swallowEvent = true;
         else
             swallowEvent = handleMousePressEvent(mouseEvent);
     }
-
     return swallowEvent;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to