2011-01-14 Dimitri Glazkov <dglazkov@chromium.org>
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jan 2011 20:17:17 +0000 (20:17 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jan 2011 20:17:17 +0000 (20:17 +0000)
        Reviewed by Darin Adler.

        Remove event forwarding logic from input[type=range], simplify event flow and thumb positioning logic.
        https://bugs.webkit.org/show_bug.cgi?id=52464

        This change has two parts:

        1) Utilize shadow DOM event retargeting to get rid of forwarding events
           via render tree;
        2) Move thumb positioning logic from RenderSlider to SliderThumbElement.

        These two are highly co-dependent. It looked ugly when I tried to
        separate them.

        No change in behavior, covered by existing tests.

        * html/HTMLInputElement.cpp:
        (WebCore::HTMLInputElement::defaultEventHandler): Added invocation of
            InputType::handleMouseDownEvent.
        * html/InputType.cpp:
        (WebCore::InputType::handleMouseDownEvent): Added empty decl.
        * html/InputType.h: Added def.
        * html/RangeInputType.cpp:
        (WebCore::RangeInputType::handleMouseDownEvent): Added to handle the case
            when the user clicks on the track of the slider. Also removed the
            forwardEvent method.
        * html/RangeInputType.h: Added/removed defs.
        * html/shadow/SliderThumbElement.cpp:
        (WebCore::SliderThumbElement::dragFrom): Added a helper method to start
            dragging from any position.
        (WebCore::SliderThumbElement::dragTo): Added a helper method to drag to
            specified position.
        (WebCore::SliderThumbElement::setPosition): Collapsed most of the positioning
            logic in RenderSlider into this method, which is now a simple calculation
            and adjusting of thumb position based on supplied coordinates.
        (WebCore::SliderThumbElement::startDragging): Added.
        (WebCore::SliderThumbElement::stopDragging): Added.
        (WebCore::SliderThumbElement::defaultEventHandler): Removed most of the
            old position-sniffing logic and replaced with simple calls to start,
            stop, and drag the thumb.
        * html/shadow/SliderThumbElement.h: Added defs.
        * rendering/RenderSlider.cpp: Removed a bunch of code that is no longer
            necessary.
        * rendering/RenderSlider.h: Removed defs, removed now-unnecessary friendliness.

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

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/InputType.cpp
Source/WebCore/html/InputType.h
Source/WebCore/html/RangeInputType.cpp
Source/WebCore/html/RangeInputType.h
Source/WebCore/html/shadow/SliderThumbElement.cpp
Source/WebCore/html/shadow/SliderThumbElement.h
Source/WebCore/rendering/RenderSlider.cpp
Source/WebCore/rendering/RenderSlider.h

index 787ac96717aa1b01979f77e9f997352025bd46b1..f61ef796f80931babe5a9b849f33ef25e928d95a 100644 (file)
@@ -1,3 +1,50 @@
+2011-01-14  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        Remove event forwarding logic from input[type=range], simplify event flow and thumb positioning logic.
+        https://bugs.webkit.org/show_bug.cgi?id=52464
+
+        This change has two parts:
+
+        1) Utilize shadow DOM event retargeting to get rid of forwarding events
+           via render tree;
+        2) Move thumb positioning logic from RenderSlider to SliderThumbElement.
+
+        These two are highly co-dependent. It looked ugly when I tried to
+        separate them.
+
+        No change in behavior, covered by existing tests.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::defaultEventHandler): Added invocation of
+            InputType::handleMouseDownEvent.
+        * html/InputType.cpp:
+        (WebCore::InputType::handleMouseDownEvent): Added empty decl.
+        * html/InputType.h: Added def.
+        * html/RangeInputType.cpp:
+        (WebCore::RangeInputType::handleMouseDownEvent): Added to handle the case
+            when the user clicks on the track of the slider. Also removed the
+            forwardEvent method.
+        * html/RangeInputType.h: Added/removed defs.
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::SliderThumbElement::dragFrom): Added a helper method to start
+            dragging from any position.
+        (WebCore::SliderThumbElement::dragTo): Added a helper method to drag to
+            specified position.
+        (WebCore::SliderThumbElement::setPosition): Collapsed most of the positioning
+            logic in RenderSlider into this method, which is now a simple calculation
+            and adjusting of thumb position based on supplied coordinates.
+        (WebCore::SliderThumbElement::startDragging): Added.
+        (WebCore::SliderThumbElement::stopDragging): Added.
+        (WebCore::SliderThumbElement::defaultEventHandler): Removed most of the
+            old position-sniffing logic and replaced with simple calls to start,
+            stop, and drag the thumb.
+        * html/shadow/SliderThumbElement.h: Added defs.
+        * rendering/RenderSlider.cpp: Removed a bunch of code that is no longer
+            necessary.
+        * rendering/RenderSlider.h: Removed defs, removed now-unnecessary friendliness.
+
 2011-01-19  Shane Stephens  <shanestephens@google.com>
 
         Reviewed by Chris Marrin.
index 3c11bfa6aa1e22b5b67b3b7797e66b931abf240d..673add560af3d776c5d2f08571021428cdcc7bd8 100644 (file)
@@ -1036,6 +1036,12 @@ void HTMLInputElement::defaultEventHandler(Event* evt)
             return;
     }
 
+    if (evt->isMouseEvent() && evt->type() == eventNames().mousedownEvent) {
+        m_inputType->handleMouseDownEvent(static_cast<MouseEvent*>(evt));
+        if (evt->defaultHandled())
+            return;
+    }
+
     m_inputType->forwardEvent(evt);
 
     if (!callBaseClassEarly && !evt->defaultHandled())
index 729f204eb1a1ed0bfbb5756c3af4d1f0c1833e9a..2f8d414d52f12debe995c4352b820260b1bc3b29 100644 (file)
@@ -305,6 +305,10 @@ void InputType::handleClickEvent(MouseEvent*)
 {
 }
 
+void InputType::handleMouseDownEvent(MouseEvent*)
+{
+}
+
 void InputType::handleDOMActivateEvent(Event*)
 {
 }
index 0d1f6b89fc761ed9d75717e0063eb34c188c1122..ea4541f7f86c1884101cad20cf047b3f37d0fc36 100644 (file)
@@ -153,6 +153,7 @@ public:
     // Event handlers
 
     virtual void handleClickEvent(MouseEvent*);
+    virtual void handleMouseDownEvent(MouseEvent*);
     virtual PassOwnPtr<ClickHandlingState> willDispatchClick();
     virtual void didDispatchClick(Event*, const ClickHandlingState&);
     virtual void handleDOMActivateEvent(Event*);
index 3fcb6415d30b487bc3fecfe421c36c30735a8316..a492ce6ef4a9c9695c4938d58660ec133fb43557 100644 (file)
@@ -36,6 +36,8 @@
 #include "HTMLNames.h"
 #include "HTMLParserIdioms.h"
 #include "KeyboardEvent.h"
+#include "MouseEvent.h"
+#include "PlatformMouseEvent.h"
 #include "RenderSlider.h"
 #include "SliderThumbElement.h"
 #include "StepRange.h"
@@ -141,6 +143,15 @@ double RangeInputType::stepScaleFactor() const
     return rangeStepScaleFactor;
 }
 
+void RangeInputType::handleMouseDownEvent(MouseEvent* event)
+{
+    if (event->button() != LeftButton || event->target() != element())
+        return;
+
+    if (SliderThumbElement* thumb = toSliderThumbElement(element()->shadowRoot()))
+        thumb->dragFrom(event->absoluteLocation());
+}
+
 void RangeInputType::handleKeydownEvent(KeyboardEvent* event)
 {
     const String& key = event->keyIdentifier();
@@ -181,12 +192,6 @@ void RangeInputType::handleKeydownEvent(KeyboardEvent* event)
     event->setDefaultHandled();
 }
 
-void RangeInputType::forwardEvent(Event* event)
-{
-    if (element()->renderer() && (event->isMouseEvent() || event->isDragEvent() || event->isWheelEvent()))
-        toRenderSlider(element()->renderer())->forwardEvent(event);
-}
-
 void RangeInputType::createShadowSubtree()
 {
     element()->setShadowRoot(SliderThumbElement::create(element()->document()));
index 57eb2999cda59e5d24570ccc480c0f67ffed9116..15d224ddc77d9cc96dd714691bd3c857c79f702c 100644 (file)
@@ -55,8 +55,8 @@ private:
     virtual double stepBase() const;
     virtual double defaultStep() const;
     virtual double stepScaleFactor() const;
+    virtual void handleMouseDownEvent(MouseEvent*);
     virtual void handleKeydownEvent(KeyboardEvent*);
-    virtual void forwardEvent(Event*);
     virtual RenderObject* createRenderer(RenderArena*, RenderStyle*) const;
     virtual void createShadowSubtree();
     virtual double parseToDouble(const String&, double) const;
index 79a009b7dffacbd0eaaf72b2504923dea660dcbb..762edda449b57ae76f610ec64cba7dcb15ee3af7 100644 (file)
 
 #include "Event.h"
 #include "Frame.h"
+#include "HTMLInputElement.h"
+#include "HTMLParserIdioms.h"
 #include "MouseEvent.h"
 #include "RenderSlider.h"
 #include "RenderTheme.h"
+#include "StepRange.h"
+#include <wtf/MathExtras.h>
+
+using namespace std;
 
 namespace WebCore {
 
@@ -48,7 +54,6 @@ public:
     virtual void layout();
 };
 
-
 RenderSliderThumb::RenderSliderThumb(Node* node)
     : RenderBlock(node)
 {
@@ -80,6 +85,70 @@ RenderObject* SliderThumbElement::createRenderer(RenderArena* arena, RenderStyle
     return new (arena) RenderSliderThumb(this);
 }
 
+void SliderThumbElement::dragFrom(const IntPoint& point)
+{
+    setPosition(point);
+    startDragging();
+}
+
+void SliderThumbElement::setPosition(const IntPoint& point)
+{
+    HTMLInputElement* input = static_cast<HTMLInputElement*>(shadowHost());
+    ASSERT(input);
+
+    if (!input->renderer() || !renderer())
+        return;
+
+    IntPoint offset = roundedIntPoint(input->renderer()->absoluteToLocal(point, false, true));
+    RenderStyle* sliderStyle = input->renderer()->style();
+    bool isVertical = sliderStyle->appearance() == SliderVerticalPart || sliderStyle->appearance() == MediaVolumeSliderPart;
+
+    int trackSize;
+    int position;
+    int currentPosition;
+    if (isVertical) {
+        trackSize = input->renderBox()->contentHeight() - renderBox()->height();
+        position = offset.y() - renderBox()->height() / 2;
+        currentPosition = renderBox()->y() - input->renderBox()->contentBoxRect().y();
+    } else {
+        trackSize = input->renderBox()->contentWidth() - renderBox()->width();
+        position = offset.x() - renderBox()->width() / 2;
+        currentPosition = renderBox()->x() - input->renderBox()->contentBoxRect().x();
+    }
+    position = max(0, min(position, trackSize));
+    if (position == currentPosition)
+        return;
+
+    StepRange range(input);
+    double fraction = static_cast<double>(position) / trackSize;
+    if (isVertical)
+        fraction = 1 - fraction;
+    double value = range.clampValue(range.valueFromProportion(fraction));
+
+    // FIXME: This is no longer being set from renderer. Consider updating the method name.
+    input->setValueFromRenderer(serializeForNumberType(value));
+    renderer()->setNeedsLayout(true);
+    input->dispatchFormControlChangeEvent();
+}
+
+void SliderThumbElement::startDragging()
+{
+    if (Frame* frame = document()->frame()) {
+        frame->eventHandler()->setCapturingMouseEventsNode(this);
+        m_inDragMode = true;
+    }
+}
+
+void SliderThumbElement::stopDragging()
+{
+    if (!m_inDragMode)
+        return;
+
+    if (Frame* frame = document()->frame())
+        frame->eventHandler()->setCapturingMouseEventsNode(0);
+    m_inDragMode = false;
+}
+
 void SliderThumbElement::defaultEventHandler(Event* event)
 {
     if (!event->isMouseEvent()) {
@@ -92,45 +161,15 @@ void SliderThumbElement::defaultEventHandler(Event* event)
     const AtomicString& eventType = event->type();
 
     if (eventType == eventNames().mousedownEvent && isLeftButton) {
-        if (document()->frame() && renderer()) {
-            RenderSlider* slider = toRenderSlider(renderer()->parent());
-            if (slider) {
-                if (slider->mouseEventIsInThumb(mouseEvent)) {
-                    // We selected the thumb, we want the cursor to always stay at
-                    // the same position relative to the thumb.
-                    m_offsetToThumb = slider->mouseEventOffsetToThumb(mouseEvent);
-                } else {
-                    // We are outside the thumb, move the thumb to the point were
-                    // we clicked. We'll be exactly at the center of the thumb.
-                    m_offsetToThumb.setX(0);
-                    m_offsetToThumb.setY(0);
-                }
-
-                m_inDragMode = true;
-                document()->frame()->eventHandler()->setCapturingMouseEventsNode(shadowHost());
-                event->setDefaultHandled();
-                return;
-            }
-        }
+        startDragging();
+        return;
     } else if (eventType == eventNames().mouseupEvent && isLeftButton) {
-        if (m_inDragMode) {
-            if (Frame* frame = document()->frame())
-                frame->eventHandler()->setCapturingMouseEventsNode(0);      
-            m_inDragMode = false;
-            event->setDefaultHandled();
-            return;
-        }
+        stopDragging();
+        return;
     } else if (eventType == eventNames().mousemoveEvent) {
-        if (m_inDragMode && renderer() && renderer()->parent()) {
-            RenderSlider* slider = toRenderSlider(renderer()->parent());
-            if (slider) {
-                FloatPoint curPoint = slider->absoluteToLocal(mouseEvent->absoluteLocation(), false, true);
-                IntPoint eventOffset(curPoint.x() + m_offsetToThumb.x(), curPoint.y() + m_offsetToThumb.y());
-                slider->setValueForPosition(slider->positionForOffset(eventOffset));
-                event->setDefaultHandled();
-                return;
-            }
-        }
+        if (m_inDragMode)
+            setPosition(mouseEvent->absoluteLocation());
+        return;
     }
 
     HTMLDivElement::defaultEventHandler(event);
index 7219186243fd8dbe94732f8444e2e8102ba4aa62..1f1c869e51de85830e56330095841e6300d71591 100644 (file)
@@ -50,6 +50,7 @@ public:
 
     bool inDragMode() const { return m_inDragMode; }
 
+    void dragFrom(const IntPoint&);
     virtual void defaultEventHandler(Event*);
     virtual void detach();
     virtual AtomicString shadowPseudoId() const;
@@ -57,6 +58,9 @@ public:
 private:
     SliderThumbElement(Document*);
     virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
+    void startDragging();
+    void stopDragging();
+    void setPosition(const IntPoint&);
 
     FloatPoint m_offsetToThumb;
     bool m_inDragMode;
index b0dc0d9f98eb363e037961aed68e98f76e4a68d6..49da3967231c0141f0a326fed0da78781ff43f5f 100644 (file)
@@ -181,116 +181,6 @@ SliderThumbElement* RenderSlider::sliderThumbElement() const
     return toSliderThumbElement(static_cast<Element*>(node())->shadowRoot());
 }
 
-bool RenderSlider::mouseEventIsInThumb(MouseEvent* evt)
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    if (!thumbElement || !thumbElement->renderer())
-        return false;
-
-#if ENABLE(VIDEO)
-    if (style()->appearance() == MediaSliderPart || style()->appearance() == MediaVolumeSliderPart) {
-        MediaControlInputElement* sliderThumb = static_cast<MediaControlInputElement*>(thumbElement->renderer()->node());
-        return sliderThumb->hitTest(evt->absoluteLocation());
-    }
-#endif
-
-    FloatPoint localPoint = thumbElement->renderBox()->absoluteToLocal(evt->absoluteLocation(), false, true);
-    IntRect thumbBounds = thumbElement->renderBox()->borderBoxRect();
-    return thumbBounds.contains(roundedIntPoint(localPoint));
-}
-
-FloatPoint RenderSlider::mouseEventOffsetToThumb(MouseEvent* evt)
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    ASSERT(thumbElement && thumbElement->renderer());
-    FloatPoint localPoint = thumbElement->renderBox()->absoluteToLocal(evt->absoluteLocation(), false, true);
-    IntRect thumbBounds = thumbElement->renderBox()->borderBoxRect();
-    FloatPoint offset;
-    offset.setX(thumbBounds.x() + thumbBounds.width() / 2 - localPoint.x());
-    offset.setY(thumbBounds.y() + thumbBounds.height() / 2 - localPoint.y());
-    return offset;
-}
-
-void RenderSlider::setValueForPosition(int position)
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    if (!thumbElement || !thumbElement->renderer())
-        return;
-
-    HTMLInputElement* element = static_cast<HTMLInputElement*>(node());
-
-    // Calculate the new value based on the position, and send it to the element.
-    StepRange range(element);
-    double fraction = static_cast<double>(position) / trackSize();
-    if (style()->appearance() == SliderVerticalPart || style()->appearance() == MediaVolumeSliderPart)
-        fraction = 1 - fraction;
-    double value = range.clampValue(range.valueFromProportion(fraction));
-    element->setValueFromRenderer(serializeForNumberType(value));
-
-    // Also update the position if appropriate.
-    if (position != currentPosition()) {
-        setNeedsLayout(true);
-
-        // FIXME: It seems like this could send extra change events if the same value is set
-        // multiple times with no layout in between.
-        element->dispatchFormControlChangeEvent();
-    }
-}
-
-int RenderSlider::positionForOffset(const IntPoint& p)
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    if (!thumbElement || !thumbElement->renderer())
-        return 0;
-
-    int position;
-    if (style()->appearance() == SliderVerticalPart || style()->appearance() == MediaVolumeSliderPart)
-        position = p.y() - thumbElement->renderBox()->height() / 2;
-    else
-        position = p.x() - thumbElement->renderBox()->width() / 2;
-    
-    return max(0, min(position, trackSize()));
-}
-
-int RenderSlider::currentPosition()
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    ASSERT(thumbElement && thumbElement->renderer());
-
-    if (style()->appearance() == SliderVerticalPart || style()->appearance() == MediaVolumeSliderPart)
-        return toRenderBox(thumbElement->renderer())->y() - contentBoxRect().y();
-    return toRenderBox(thumbElement->renderer())->x() - contentBoxRect().x();
-}
-
-int RenderSlider::trackSize()
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    ASSERT(thumbElement && thumbElement->renderer());
-
-    if (style()->appearance() == SliderVerticalPart || style()->appearance() == MediaVolumeSliderPart)
-        return contentHeight() - thumbElement->renderBox()->height();
-    return contentWidth() - thumbElement->renderBox()->width();
-}
-
-void RenderSlider::forwardEvent(Event* event)
-{
-    SliderThumbElement* thumbElement = sliderThumbElement();
-    if (!thumbElement)
-        return;
-
-    if (event->isMouseEvent()) {
-        MouseEvent* mouseEvent = static_cast<MouseEvent*>(event);
-        if (event->type() == eventNames().mousedownEvent && mouseEvent->button() == LeftButton) {
-            if (!mouseEventIsInThumb(mouseEvent)) {
-                IntPoint eventOffset = roundedIntPoint(absoluteToLocal(mouseEvent->absoluteLocation(), false, true));
-                setValueForPosition(positionForOffset(eventOffset));
-            }
-        }
-    }
-
-    thumbElement->defaultEventHandler(event);
-}
-
 bool RenderSlider::inDragMode() const
 {
     SliderThumbElement* thumbElement = sliderThumbElement();
index 5fb592191000b3c3d54dab7dc80fb506b8da0f6f..0162b7107bad5ee6c532d559bce554410c90e117 100644 (file)
@@ -34,7 +34,6 @@ namespace WebCore {
         RenderSlider(HTMLInputElement*);
         virtual ~RenderSlider();
 
-        void forwardEvent(Event*);
         bool inDragMode() const;
         IntRect thumbRect();
 
@@ -49,20 +48,8 @@ namespace WebCore {
         // FIXME: Eventually, the logic of manipulating slider thumb should move to
         // SliderThumbElement and accessing sliderThumbElement should not be necessary in this class.
         SliderThumbElement* sliderThumbElement() const;
-        bool mouseEventIsInThumb(MouseEvent*);
-        FloatPoint mouseEventOffsetToThumb(MouseEvent*);
-
-        void setValueForPosition(int position);
-        void setPositionFromValue();
-        int positionForOffset(const IntPoint&);
-
-        int currentPosition();
 
         virtual bool requiresForcedStyleRecalcPropagation() const { return true; }
-
-        int trackSize();
-
-        friend class SliderThumbElement;
     };
 
     inline RenderSlider* toRenderSlider(RenderObject* object)