Attachment 44649 Details for Bug 32303 – [PATCH] Fix

[patch] [PATCH] Fix

pan_scroll_fire_prevent_default.patch (text/plain), 11.45 KB, created by Brian Weinstein on 2009-12-10 16:40:36 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Brian Weinstein

Created: 2009-12-10 16:40:36 PST

Size: 11.45 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 51972)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,28 @@
>+2009-12-10  Brian Weinstein  <bweinstein@apple.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixes <http://webkit.org/b/32303>.
>+        Middle-mouse button not firing mousedown DOM event when autoscroll happens.
>+        
>+        This fix moves the pan scrolling code into Node.cpp, as part of the default event handler.
>+        We get two wins out of this:
>+            1) Mousedown DOM events are now fired before pan scrolling.
>+            2) Calling event.preventDefault() on a middle mouse button will now prevent pan scrolling.
>+        
>+        Added tests to show that middle mousedown DOM events are fired when panscrolling happens, and
>+        that when event.preventDefault is called, pan scrolling doesn't happen.
>+
>+        Tests: platform/win/fast/events/panScroll-event-fired.html
>+               platform/win/fast/events/panScroll-preventDefault.html
>+
>+        * dom/Node.cpp:
>+        (WebCore::Node::defaultEventHandler): Moved pan scrolling starting code to here.
>+        * page/EventHandler.cpp:
>+        (WebCore::EventHandler::startPanScrolling): Function to initialize scrolling variables and start timer.
>+        (WebCore::EventHandler::handleMousePressEvent): From here.
>+        * page/EventHandler.h:
>+
> 2009-12-10  Kent Hansen  <kent.hansen@nokia.com>
> 
>         Reviewed by Geoffrey Garen.
>Index: WebCore/dom/Node.cpp
>===================================================================
>--- WebCore/dom/Node.cpp	(revision 51951)
>+++ WebCore/dom/Node.cpp	(working copy)
>@@ -2827,6 +2827,26 @@ void Node::defaultEventHandler(Event* ev
>         if (event->isTextEvent())
>             if (Frame* frame = document()->frame())
>                 frame->eventHandler()->defaultTextInputEventHandler(static_cast<TextEvent*>(event));
>+#if ENABLE(PAN_SCROLLING)
>+    } else if (eventType == eventNames().mousedownEvent) {
>+        MouseEvent* mouseEvent = static_cast<MouseEvent*>(event);
>+        if (mouseEvent->button() == MiddleButton && !this->isLink()) {
>+            RenderObject* renderer = this->renderer();
>+
>+            while (renderer && (!renderer->isBox() || !toRenderBox(renderer)->canBeScrolledAndHasScrollableArea())) {
>+                // FIXME: If we start in a frame that can't scroll, we don't want to jump out of it to start scrolling:
>+                // <https://bugs.webkit.org/show_bug.cgi?id=32399>.
>+                if (!renderer->parent() && renderer->node() == renderer->document() && renderer->document()->ownerElement())
>+                    renderer = renderer->document()->ownerElement()->renderer();
>+                else
>+                    renderer = renderer->parent();
>+            }
>+
>+            if (renderer)
>+                if (Frame* frame = renderer->document()->frame())
>+                    frame->eventHandler()->startPanScrolling(renderer);
>+        }
>+#endif
>     }
> }
> 
>Index: WebCore/page/EventHandler.cpp
>===================================================================
>--- WebCore/page/EventHandler.cpp	(revision 51951)
>+++ WebCore/page/EventHandler.cpp	(working copy)
>@@ -689,6 +689,14 @@ void EventHandler::autoscrollTimerFired(
> 
> #if ENABLE(PAN_SCROLLING)
> 
>+void EventHandler::startPanScrolling(RenderObject* renderer)
>+{
>+    m_panScrollInProgress = true;
>+    m_panScrollButtonPressed = true;
>+    handleAutoscroll(renderer);
>+    invalidateClick();
>+}
>+
> void EventHandler::updatePanScrollState()
> {
>     FrameView* view = m_frame->view();
>@@ -1188,25 +1196,6 @@ bool EventHandler::handleMousePressEvent
>         invalidateClick();
>         return true;
>     }
>-
>-    if (mouseEvent.button() == MiddleButton && !mev.isOverLink()) {
>-        RenderObject* renderer = mev.targetNode()->renderer();
>-
>-        while (renderer && (!renderer->isBox() || !toRenderBox(renderer)->canBeScrolledAndHasScrollableArea())) {
>-            if (!renderer->parent() && renderer->node() == renderer->document() && renderer->document()->ownerElement())
>-                renderer = renderer->document()->ownerElement()->renderer();
>-            else
>-                renderer = renderer->parent();
>-        }
>-
>-        if (renderer) {
>-            m_panScrollInProgress = true;
>-            m_panScrollButtonPressed = true;
>-            handleAutoscroll(renderer);
>-            invalidateClick();
>-            return true;
>-        }
>-    }
> #endif
> 
>     m_clickCount = mouseEvent.clickCount();
>Index: WebCore/page/EventHandler.h
>===================================================================
>--- WebCore/page/EventHandler.h	(revision 51951)
>+++ WebCore/page/EventHandler.h	(working copy)
>@@ -85,6 +85,7 @@ public:
>     Node* mousePressNode() const;
>     void setMousePressNode(PassRefPtr<Node>);
> 
>+    void startPanScrolling(RenderObject*);
>     bool panScrollInProgress() { return m_panScrollInProgress; }
>     void setPanScrollInProgress(bool inProgress) { m_panScrollInProgress = inProgress; }
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 51972)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,18 @@
>+2009-12-10  Brian Weinstein  <bweinstein@apple.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixes <http://webkit.org/b/32303>.
>+        Middle-mouse button not firing mousedown DOM event when autoscroll happens.
>+        
>+        Added tests to show that middle mousedown DOM events are fired when panscrolling happens, and
>+        that when event.preventDefault is called, pan scrolling doesn't happen.
>+
>+        * platform/win/fast/events/panScroll-event-fired-expected.txt: Added.
>+        * platform/win/fast/events/panScroll-event-fired.html: Added.
>+        * platform/win/fast/events/panScroll-preventDefault-expected.txt: Added.
>+        * platform/win/fast/events/panScroll-preventDefault.html: Added.
>+
> 2009-12-10  Kenneth Russell  <kbr@google.com>
> 
>         Reviewed by Oliver Hunt.
>Index: LayoutTests/platform/win/fast/events/panScroll-event-fired-expected.txt
>===================================================================
>--- LayoutTests/platform/win/fast/events/panScroll-event-fired-expected.txt	(revision 0)
>+++ LayoutTests/platform/win/fast/events/panScroll-event-fired-expected.txt	(revision 0)
>@@ -0,0 +1,4 @@
>+CONSOLE MESSAGE: line 14: Mousedown with 1
>+Test for bug 28023 This tests that pan scrolling works without event.preventDefault and the middle button mousedown event is fired. To test manually, try to pan scroll inside this div.
>+
>+Success! Div with overflow was scrolled
>Index: LayoutTests/platform/win/fast/events/panScroll-event-fired.html
>===================================================================
>--- LayoutTests/platform/win/fast/events/panScroll-event-fired.html	(revision 0)
>+++ LayoutTests/platform/win/fast/events/panScroll-event-fired.html	(revision 0)
>@@ -0,0 +1,46 @@
>+<html>
>+    <head>
>+        <title>Pan Scrolling Test</title>
>+    </head>
>+    <body>
>+        <script>
>+            if (window.layoutTestController) {
>+                layoutTestController.dumpAsText();
>+                layoutTestController.waitUntilDone();
>+            }
>+            
>+            addEventListener('mousedown', 
>+                function(event) { 
>+                    console.log("Mousedown with " + event.button);
>+                }
>+            , false);
>+        </script>
>+        <div id="overflow" style="width:500px; height:150px; overflow:auto; border:2px solid red; padding:10px">
>+            <h1>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=32303">bug 28023</a> This tests that pan
>+            scrolling works without event.preventDefault and the middle button mousedown event is fired.
>+            To test manually, try to pan scroll inside this div.</h1>
>+        </div>
>+        <p>
>+        <div id="console"></div>
>+        <script>
>+            if (window.eventSender)
>+            {
>+                eventSender.mouseMoveTo(50, 50);
>+                eventSender.mouseDown(1);
>+                eventSender.mouseUp(1);
>+                eventSender.mouseMoveTo(50, 200);
>+                setTimeout(finished, 500);
>+            }
>+            
>+            function finished()
>+            {
>+                if (document.getElementById('overflow').scrollTop)
>+                    document.getElementById('console').innerHTML = "Success! Div with overflow was scrolled";
>+                else
>+                    document.getElementById('console').innerHTML = "Fail! Div with overflow was not scrolled";
>+                
>+                window.layoutTestController.notifyDone();
>+            }
>+        </script>
>+    </body>
>+</html>
>Index: LayoutTests/platform/win/fast/events/panScroll-preventDefault-expected.txt
>===================================================================
>--- LayoutTests/platform/win/fast/events/panScroll-preventDefault-expected.txt	(revision 0)
>+++ LayoutTests/platform/win/fast/events/panScroll-preventDefault-expected.txt	(revision 0)
>@@ -0,0 +1,4 @@
>+CONSOLE MESSAGE: line 14: Mousedown with 1
>+Test for bug 28023 This tests that having a preventDefault on mousedown will prevent pan scrolling and the middle button mousedown event is fired. To test manually, try to pan scroll inside this div.
>+
>+Success! Div with overflow was not scrolled
>Index: LayoutTests/platform/win/fast/events/panScroll-preventDefault.html
>===================================================================
>--- LayoutTests/platform/win/fast/events/panScroll-preventDefault.html	(revision 0)
>+++ LayoutTests/platform/win/fast/events/panScroll-preventDefault.html	(revision 0)
>@@ -0,0 +1,47 @@
>+<html>
>+    <head>
>+        <title>Pan Scrolling Test</title>
>+    </head>
>+    <body>
>+        <script>
>+            if (window.layoutTestController) {
>+                layoutTestController.dumpAsText();
>+                layoutTestController.waitUntilDone();
>+            }
>+            
>+            addEventListener('mousedown', 
>+                function(event) { 
>+                    console.log("Mousedown with " + event.button);
>+                    event.preventDefault();
>+                }
>+            , false);
>+        </script>
>+        <div id="overflow" style="width:500px; height:150px; overflow:auto; border:2px solid red; padding:10px">
>+            <h1>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=32303">bug 28023</a> This tests that having a
>+            preventDefault on mousedown will prevent pan scrolling and the middle button mousedown event is fired.
>+            To test manually, try to pan scroll inside this div.</h1>
>+        </div>
>+        <p>
>+        <div id="console"></div>
>+        <script>
>+            if (window.eventSender)
>+            {
>+                eventSender.mouseMoveTo(50, 50);
>+                eventSender.mouseDown(1);
>+                eventSender.mouseUp(1);
>+                eventSender.mouseMoveTo(50, 200);
>+                setTimeout(finished, 500);
>+            }
>+            
>+            function finished()
>+            {
>+                if (document.getElementById('overflow').scrollTop)
>+                    document.getElementById('console').innerHTML = "Fail! Div with overflow was scrolled";
>+                else
>+                    document.getElementById('console').innerHTML = "Success! Div with overflow was not scrolled";
>+                
>+                window.layoutTestController.notifyDone();
>+            }
>+        </script>
>+    </body>
>+</html>

Flags:

aroben: review+
bweinstein: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 32303: 44643 | 44649 | 44656