Changeset 154832 in webkit

Timestamp:

Aug 29, 2013, 12:15:33 PM (13 years ago)

Author:

Simon Fraser

Message:

Fix slider thumb event handling to use local, not absolute coordinates
​https://bugs.webkit.org/show_bug.cgi?id=120480

Reviewed by Darin Adler.

SliderThumbElement::setPositionFromPoint() did all of its coordinate
math by mapping renderer rects into absolute coordinates, which was
unnecessary and expensive.

Fix by doing all the math in the coordinate space of the input's
renderer. This simplified the code. Also, currentPosition
was computed but unused, so was removed.

No behavior change. Tested by fast/forms/range/slider-transformed.html

• html/shadow/SliderThumbElement.cpp:

(WebCore::SliderThumbElement::setPositionFromPoint):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/shadow/SliderThumbElement.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-08-29  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix slider thumb event handling to use local, not absolute coordinates
+        https://bugs.webkit.org/show_bug.cgi?id=120480
+
+        Reviewed by Darin Adler.
+        
+        SliderThumbElement::setPositionFromPoint() did all of its coordinate
+        math by mapping renderer rects into absolute coordinates, which was
+        unnecessary and expensive.
+        
+        Fix by doing all the math in the coordinate space of the input's
+        renderer. This simplified the code. Also, currentPosition
+        was computed but unused, so was removed.
+
+        No behavior change. Tested by fast/forms/range/slider-transformed.html
+
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::SliderThumbElement::setPositionFromPoint):
+
 2013-08-29  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/html/shadow/SliderThumbElement.cpp

@@ -257 +257 @@
 }
 
-void SliderThumbElement::setPositionFromPoint(const LayoutPoint& point)
+void SliderThumbElement::setPositionFromPoint(const LayoutPoint& absolutePoint)
 {
     RefPtr<HTMLInputElement> input(hostInput());
@@ -266 +266 @@
 
     input->setTextAsOfLastFormControlChangeEvent(input->value());
-    LayoutPoint offset = roundedLayoutPoint(input->renderer()->absoluteToLocal(point, UseTransforms));
+
+    // Do all the tracking math relative to the input's renderer's box.
+    RenderBox& inputRenderer = *toRenderBox(input->renderer());
+    RenderBox& trackRenderer = *trackElement->renderBox();
+
     bool isVertical = hasVerticalAppearance(input.get());
     bool isLeftToRightDirection = renderBox()->style()->isLeftToRightDirection();
-    LayoutUnit trackSize;
+    
+    LayoutPoint offset = roundedLayoutPoint(inputRenderer.absoluteToLocal(absolutePoint, UseTransforms));
+    FloatRect trackBoundingBox = trackRenderer.localToContainerQuad(FloatRect(0, 0, trackRenderer.width(), trackRenderer.height()), &inputRenderer).enclosingBoundingBox();
+
+    LayoutUnit trackLength;
     LayoutUnit position;
-    LayoutUnit currentPosition;
-    // We need to calculate currentPosition from absolute points becaue the
-    // renderer for this node is usually on a layer and renderBox()->x() and
-    // y() are unusable.
-    // FIXME: This should probably respect transforms.
-    LayoutPoint absoluteThumbOrigin = renderBox()->absoluteBoundingBoxRectIgnoringTransforms().location();
-    LayoutPoint absoluteSliderContentOrigin = roundedLayoutPoint(input->renderer()->localToAbsolute());
-    IntRect trackBoundingBox = trackElement->renderer()->absoluteBoundingBoxRectIgnoringTransforms();
-    IntRect inputBoundingBox = input->renderer()->absoluteBoundingBoxRectIgnoringTransforms();
     if (isVertical) {
-        trackSize = trackElement->renderBox()->contentHeight() - renderBox()->height();
-        position = offset.y() - renderBox()->height() / 2 - trackBoundingBox.y() + inputBoundingBox.y() - renderBox()->marginBottom();
-        currentPosition = absoluteThumbOrigin.y() - absoluteSliderContentOrigin.y();
+        trackLength = trackRenderer.contentHeight() - renderBox()->height();
+        position = offset.y() - renderBox()->height() / 2 - trackBoundingBox.y() - renderBox()->marginBottom();
     } else {
-        trackSize = trackElement->renderBox()->contentWidth() - renderBox()->width();
-        position = offset.x() - renderBox()->width() / 2 - trackBoundingBox.x() + inputBoundingBox.x();
+        trackLength = trackRenderer.contentWidth() - renderBox()->width();
+        position = offset.x() - renderBox()->width() / 2 - trackBoundingBox.x();
         position -= isLeftToRightDirection ? renderBox()->marginLeft() : renderBox()->marginRight();
-        currentPosition = absoluteThumbOrigin.x() - absoluteSliderContentOrigin.x();
-    }
-    position = max<LayoutUnit>(0, min(position, trackSize));
-    const Decimal ratio = Decimal::fromDouble(static_cast<double>(position) / trackSize);
+    }
+
+    position = max<LayoutUnit>(0, min(position, trackLength));
+    const Decimal ratio = Decimal::fromDouble(static_cast<double>(position) / trackLength);
     const Decimal fraction = isVertical || !isLeftToRightDirection ? Decimal(1) - ratio : ratio;
     StepRange stepRange(input->createStepRange(RejectAny));
@@ -303 +301 @@
             double closestFraction = stepRange.proportionFromValue(closest).toDouble();
             double closestRatio = isVertical || !isLeftToRightDirection ? 1.0 - closestFraction : closestFraction;
-            LayoutUnit closestPosition = trackSize * closestRatio;
+            LayoutUnit closestPosition = trackLength * closestRatio;
             if ((closestPosition - position).abs() <= snappingThreshold)
                 value = closest;