WebCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Mar 2009 14:34:02 +0000 (14:34 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 25 Mar 2009 14:34:02 +0000 (14:34 +0000)
2009-03-25  Darin Adler  <darin@apple.com>

        Reviewed by David Hyatt.

        Bug 24740: crash in RenderSlider::setPositionFromValue when calling pause() after setting <video> to display: none
        https://bugs.webkit.org/show_bug.cgi?id=24740
        rdar://problem/6679873

        Bug 12104: Native Slider: When the thumb's height is specified as a percentage, it is not centered properly
        https://bugs.webkit.org/show_bug.cgi?id=12104

        Test: media/video-display-none-crash.html

        The problem here was that RenderSlider was trying to position its thumb in a way that
        requires it to call updateLayout inside rendering code. The right way to position a
        child renderer is to do layout, so I had to write a layout function. And then fix a few
        other small problems shown by the same test case.

        * rendering/RenderSlider.cpp: Made defaultTrackLength have internal linkage.
        Renamed HTMLSliderThumbElement to SliderThumbElement because we only use the HTML
        prefix for public DOM classes, not elements used as implementation details.
        Made SliderThumbElement function members private and got rid of unneeded default
        argument value for shadowParent.
        (WebCore::SliderRange::SliderRange): Added. Parses precision, max, and min attributes.
        (WebCore::SliderRange::clampValue): Added. Does standard clamping based on the above.
        (WebCore::SliderRange::valueFromElement): Added. Reads the value from the element in
        a way that clamps to the range.
        (WebCore::sliderPosition): Added. Computes the slider position: a double.
        (WebCore::SliderThumbElement::SliderThumbElement): Removed unneeded explicit
        initialization of m_initialClickPoint.
        (WebCore::SliderThumbElement::defaultEventHandler): Call setValueForPosition instead
        of calling setCurrentPosition and valueChanged.
        (WebCore::RenderSlider::RenderSlider): Remove unneeded explicit initialization of m_thumb.
        (WebCore::RenderSlider::styleDidChange): Remove unneeded second argument to createThumbStyle.
        (WebCore::RenderSlider::createThumbStyle): Remove unneeded second argument. Get rid of code
        setting the position to relative and setting the left and top. We now handle positioning
        in a custom layout function.
        (WebCore::RenderSlider::layout): Rewrote to handle positioning of the thumb as layout.
        (WebCore::RenderSlider::updateFromElement): Added code to immediately update the value
        in the element if it's out of range. This clamping used to be done as a side effect of
        setPositionFromValue. Also, this has nothing to do with the renderer, so at some point
        it could be moved into HTMLInputElement. Removed call to setPositionFromValue
        and instead just rely on the call to setNeedsLayout. Fix the setNeedsLayout call to be
        a normal setNeedsLayout(true), not a setNeedsLayout(true, false), because we do want
        this to be propagated to the parent -- it's not called during layout.
        (WebCore::RenderSlider::setValueForPosition): Refactor to use the new SliderRange
        class. Also don't call setCurrentPosition; instead just call setNeedsLayout.
        (WebCore::RenderSlider::currentPosition): Use the actual position of the renderer rather
        than the style to find the position; that means this needs to be done after layout is done.
        Also removed unneeded runtime checks and replaced them with assertions, after checking
        all callers to see they already guarantee this.
        (WebCore::RenderSlider::trackSize): Removed unneeded runtime checks and replaced them
        with assertions, after checking all callers to see they already guarantee this.
        (WebCore::RenderSlider::inDragMode): Added a null check for m_thumb so this won't
        crash if called early on a brand new RenderSlider.

        * rendering/RenderSlider.h: Made all functions private except for forwardEvent and inDragMode.
        Renamed HTMLSliderThumbElement to SliderThumbElement because we only use the HTML
        prefix for public DOM classes, not elements used as implementation details. Made the
        mouseEventIsInThumb function non-virtual. Removed the return value and argument from
        setPositionFromValue. Removed valueChanged and setCurrentPosition. Removed the oldStyle
        argument to createThumbStyle (see above). Made SliderThumbElement a friend so it can use some
        private member functions.

LayoutTests:

2009-03-25  Darin Adler  <darin@apple.com>

        Reviewed by David Hyatt.

        Bug 24740: crash in RenderSlider::setPositionFromValue when calling pause() after setting <video> to display: none
        https://bugs.webkit.org/show_bug.cgi?id=24740
        rdar://problem/6679873

        Bug 12104: Native Slider: When the thumb's height is specified as a percentage, it is not centered properly
        https://bugs.webkit.org/show_bug.cgi?id=12104

        * media/video-display-none-crash-expected.txt: Added.
        * media/video-display-none-crash.html: Added.

        * platform/mac/fast/forms/box-shadow-override-expected.txt: Updated since a slider's thumb is no longer
        relative-positioned and hence no longer gets its own layer.
        * platform/mac/fast/forms/input-appearance-height-expected.txt: Ditto.
        * platform/mac/fast/forms/slider-padding-expected.txt: Ditto.
        * platform/mac/fast/forms/slider-thumb-shared-style-expected.txt: Ditto.
        * platform/mac/fast/forms/slider-thumb-stylability-expected.txt: Ditto.
        * platform/mac/media/audio-controls-rendering-expected.txt: Ditto.
        * platform/mac/media/video-controls-rendering-expected.txt: Ditto.
        * platform/mac/media/video-display-toggle-expected.txt: Ditto.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/media/video-display-none-crash-expected.txt [new file with mode: 0644]
LayoutTests/media/video-display-none-crash.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/forms/box-shadow-override-expected.txt
LayoutTests/platform/mac/fast/forms/input-appearance-height-expected.txt
LayoutTests/platform/mac/fast/forms/slider-padding-expected.txt
LayoutTests/platform/mac/fast/forms/slider-thumb-shared-style-expected.txt
LayoutTests/platform/mac/fast/forms/slider-thumb-stylability-expected.txt
LayoutTests/platform/mac/media/audio-controls-rendering-expected.txt
LayoutTests/platform/mac/media/video-controls-rendering-expected.txt
LayoutTests/platform/mac/media/video-display-toggle-expected.txt
WebCore/ChangeLog
WebCore/rendering/RenderSlider.cpp
WebCore/rendering/RenderSlider.h

index c758596..47224b0 100644 (file)
@@ -1,3 +1,27 @@
+2009-03-25  Darin Adler  <darin@apple.com>
+
+        Reviewed by David Hyatt.
+
+        Bug 24740: crash in RenderSlider::setPositionFromValue when calling pause() after setting <video> to display: none
+        https://bugs.webkit.org/show_bug.cgi?id=24740
+        rdar://problem/6679873
+
+        Bug 12104: Native Slider: When the thumb's height is specified as a percentage, it is not centered properly
+        https://bugs.webkit.org/show_bug.cgi?id=12104
+
+        * media/video-display-none-crash-expected.txt: Added.
+        * media/video-display-none-crash.html: Added.
+
+        * platform/mac/fast/forms/box-shadow-override-expected.txt: Updated since a slider's thumb is no longer
+        relative-positioned and hence no longer gets its own layer.
+        * platform/mac/fast/forms/input-appearance-height-expected.txt: Ditto.
+        * platform/mac/fast/forms/slider-padding-expected.txt: Ditto.
+        * platform/mac/fast/forms/slider-thumb-shared-style-expected.txt: Ditto.
+        * platform/mac/fast/forms/slider-thumb-stylability-expected.txt: Ditto.
+        * platform/mac/media/audio-controls-rendering-expected.txt: Ditto.
+        * platform/mac/media/video-controls-rendering-expected.txt: Ditto.
+        * platform/mac/media/video-display-toggle-expected.txt: Ditto.
+
 2009-03-24  Mark Rowe  <mrowe@apple.com>
 
         Reviewed by Jon "The Most Boring Man in the World" Honeycutt.
diff --git a/LayoutTests/media/video-display-none-crash-expected.txt b/LayoutTests/media/video-display-none-crash-expected.txt
new file mode 100644 (file)
index 0000000..bf05d5c
--- /dev/null
@@ -0,0 +1,4 @@
+Test that pause() after changing display to "none" doesn't cause a crash.
+
+END OF TEST
+
diff --git a/LayoutTests/media/video-display-none-crash.html b/LayoutTests/media/video-display-none-crash.html
new file mode 100644 (file)
index 0000000..31a6323
--- /dev/null
@@ -0,0 +1,10 @@
+<video controls></video>
+<p>Test that pause() after changing display to "none" doesn't cause a crash.</p>
+<script src="video-test.js"></script>
+<script>
+    video.src = "content/test.mp4";
+    video.play();
+    video.style.display = "none";
+    video.pause();
+    endTest();
+</script>
index 74c1129..ed6ec35 100644 (file)
@@ -35,6 +35,7 @@ layer at (0,0) size 800x600
         RenderText {#text} at (40,9) size 4x18
           text run at (40,9) width 4: " "
         RenderSlider {INPUT} at (46,8) size 129x15 [bgcolor=#FFFFFF]
+          RenderBlock {DIV} at (57,0) size 15x15
         RenderText {#text} at (177,9) size 4x18
           text run at (177,9) width 4: " "
         RenderFileUploadControl {INPUT} at (183,10) size 237x18
@@ -77,5 +78,3 @@ layer at (30,104) size 112x13
   RenderBlock {DIV} at (17,0) size 112x13
 layer at (35,127) size 107x13
   RenderBlock {DIV} at (22,0) size 107x13
-layer at (111,153) size 15x15
-  RenderBlock (relative positioned) {DIV} at (0,0) size 15x15
index ef1a308..d82452a 100644 (file)
@@ -50,6 +50,7 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,127) size 39x18
           text run at (0,127) width 39: "range "
         RenderSlider {INPUT} at (41,126) size 129x15 [bgcolor=#FFFFFF]
+          RenderBlock {DIV} at (57,0) size 15x15
         RenderText {#text} at (172,127) size 4x18
           text run at (172,127) width 4: " "
         RenderBR {BR} at (176,141) size 0x0
@@ -100,5 +101,3 @@ layer at (77,243) size 142x13
   RenderBlock {DIV} at (3,3) size 142x13
 layer at (65,266) size 121x13
   RenderBlock {DIV} at (8,0) size 121x13
-layer at (106,152) size 15x15
-  RenderBlock (relative positioned) {DIV} at (0,0) size 15x15
index d6fd033..cf5ac6f 100644 (file)
@@ -9,7 +9,6 @@ layer at (0,0) size 800x600
         RenderBR {BR} at (0,0) size 0x0
       RenderBlock {DIV} at (0,18) size 784x39 [bgcolor=#ADD8E6]
         RenderSlider {INPUT} at (2,2) size 100x35 [bgcolor=#FFFFFF]
+          RenderBlock {DIV} at (10,10) size 15x15
         RenderText {#text} at (0,0) size 0x0
       RenderBlock {PRE} at (0,70) size 784x0
-layer at (20,38) size 15x15
-  RenderBlock (relative positioned) {DIV} at (10,10) size 15x15
index a3689bb..1beae67 100644 (file)
@@ -20,10 +20,8 @@ layer at (0,0) size 800x600
           text run at (0,0) width 282: "The first slider\x{2019}s thumb should be on the left."
       RenderBlock {DIV} at (0,68) size 784x38
         RenderSlider {INPUT} at (2,2) size 129x15 [bgcolor=#FFFFFF]
+          RenderBlock {DIV} at (0,0) size 15x15
         RenderBR {BR} at (133,17) size 0x0
         RenderSlider {INPUT} at (2,21) size 129x15 [bgcolor=#FFFFFF]
+          RenderBlock {DIV} at (114,0) size 15x15
         RenderText {#text} at (0,0) size 0x0
-layer at (10,78) size 15x15
-  RenderBlock (relative positioned) {DIV} at (0,0) size 15x15
-layer at (124,97) size 15x15
-  RenderBlock (relative positioned) {DIV} at (0,0) size 15x15
index cb1dda0..3873263 100644 (file)
@@ -11,8 +11,7 @@ layer at (0,0) size 800x600
           text run at (0,0) width 258: "You should see a green 20\x{D7}20px square."
       RenderBlock (anonymous) at (0,68) size 784x24
         RenderSlider {INPUT} at (2,2) size 129x20 [bgcolor=#FFFFFF]
+          RenderBlock {DIV} at (0,0) size 20x20 [bgcolor=#008000]
         RenderText {#text} at (0,0) size 0x0
         RenderText {#text} at (0,0) size 0x0
         RenderText {#text} at (0,0) size 0x0
-layer at (10,78) size 20x20
-  RenderBlock (relative positioned) {DIV} at (0,0) size 20x20 [bgcolor=#008000]
index 30fc5fc..706e438 100644 (file)
@@ -27,8 +27,7 @@ layer at (40,42) size 136x16
   RenderBlock (positioned) {DIV} at (32,0) size 136x16
 layer at (40,42) size 136x16
   RenderSlider {INPUT} at (0,0) size 136x16
-layer at (42,43) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (175,42) size 17x16
   RenderButton {INPUT} at (167,0) size 17x16
 layer at (191,42) size 17x16
@@ -46,8 +45,7 @@ layer at (40,76) size 256x16
   RenderBlock (positioned) {DIV} at (32,0) size 256x16
 layer at (40,76) size 256x16
   RenderSlider {INPUT} at (0,0) size 256x16
-layer at (42,77) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (295,76) size 17x16
   RenderButton {INPUT} at (287,0) size 17x16
 layer at (311,76) size 17x16
@@ -67,8 +65,7 @@ layer at (40,194) size 256x16
   RenderBlock (positioned) {DIV} at (32,84) size 256x16
 layer at (40,194) size 256x16
   RenderSlider {INPUT} at (0,0) size 256x16
-layer at (42,195) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (295,194) size 17x16
   RenderButton {INPUT} at (287,84) size 17x16
 layer at (311,194) size 17x16
index ec3f0c9..b0b947c 100644 (file)
@@ -26,8 +26,7 @@ layer at (40,266) size 256x16
   RenderBlock (positioned) {DIV} at (32,224) size 256x16
 layer at (40,266) size 256x16
   RenderSlider {INPUT} at (0,0) size 256x16
-layer at (42,267) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (295,266) size 17x16
   RenderButton {INPUT} at (287,224) size 17x16
 layer at (311,266) size 17x16
@@ -45,8 +44,7 @@ layer at (40,506) size 256x16
   RenderBlock (positioned) {DIV} at (32,224) size 256x16
 layer at (40,506) size 256x16
   RenderSlider {INPUT} at (0,0) size 256x16
-layer at (42,507) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (295,506) size 17x16
   RenderButton {INPUT} at (287,224) size 17x16
 layer at (311,506) size 17x16
@@ -66,8 +64,7 @@ layer at (40,746) size 256x16
   RenderBlock (positioned) {DIV} at (32,224) size 256x16
 layer at (40,746) size 256x16
   RenderSlider {INPUT} at (0,0) size 256x16
-layer at (42,747) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (295,746) size 17x16
   RenderButton {INPUT} at (287,224) size 17x16
 layer at (311,746) size 17x16
index 3f97ef6..32de974 100644 (file)
@@ -23,8 +23,7 @@ layer at (40,250) size 256x16
   RenderBlock (positioned) {DIV} at (32,224) size 256x16
 layer at (40,250) size 256x16
   RenderSlider {INPUT} at (0,0) size 256x16
-layer at (42,251) size 13x14
-  RenderBlock (relative positioned) {DIV} at (2,0) size 13x14
+    RenderBlock {DIV} at (2,1) size 13x14
 layer at (295,250) size 17x16
   RenderButton {INPUT} at (287,224) size 17x16
 layer at (311,250) size 17x16
index 66d4085..d8171e2 100644 (file)
@@ -1,3 +1,67 @@
+2009-03-25  Darin Adler  <darin@apple.com>
+
+        Reviewed by David Hyatt.
+
+        Bug 24740: crash in RenderSlider::setPositionFromValue when calling pause() after setting <video> to display: none
+        https://bugs.webkit.org/show_bug.cgi?id=24740
+        rdar://problem/6679873
+
+        Bug 12104: Native Slider: When the thumb's height is specified as a percentage, it is not centered properly
+        https://bugs.webkit.org/show_bug.cgi?id=12104
+
+        Test: media/video-display-none-crash.html
+
+        The problem here was that RenderSlider was trying to position its thumb in a way that
+        requires it to call updateLayout inside rendering code. The right way to position a
+        child renderer is to do layout, so I had to write a layout function. And then fix a few
+        other small problems shown by the same test case.
+
+        * rendering/RenderSlider.cpp: Made defaultTrackLength have internal linkage.
+        Renamed HTMLSliderThumbElement to SliderThumbElement because we only use the HTML
+        prefix for public DOM classes, not elements used as implementation details.
+        Made SliderThumbElement function members private and got rid of unneeded default
+        argument value for shadowParent.
+        (WebCore::SliderRange::SliderRange): Added. Parses precision, max, and min attributes.
+        (WebCore::SliderRange::clampValue): Added. Does standard clamping based on the above.
+        (WebCore::SliderRange::valueFromElement): Added. Reads the value from the element in
+        a way that clamps to the range.
+        (WebCore::sliderPosition): Added. Computes the slider position: a double.
+        (WebCore::SliderThumbElement::SliderThumbElement): Removed unneeded explicit
+        initialization of m_initialClickPoint.
+        (WebCore::SliderThumbElement::defaultEventHandler): Call setValueForPosition instead
+        of calling setCurrentPosition and valueChanged.
+        (WebCore::RenderSlider::RenderSlider): Remove unneeded explicit initialization of m_thumb.
+        (WebCore::RenderSlider::styleDidChange): Remove unneeded second argument to createThumbStyle.
+        (WebCore::RenderSlider::createThumbStyle): Remove unneeded second argument. Get rid of code
+        setting the position to relative and setting the left and top. We now handle positioning
+        in a custom layout function.
+        (WebCore::RenderSlider::layout): Rewrote to handle positioning of the thumb as layout.
+        (WebCore::RenderSlider::updateFromElement): Added code to immediately update the value
+        in the element if it's out of range. This clamping used to be done as a side effect of
+        setPositionFromValue. Also, this has nothing to do with the renderer, so at some point
+        it could be moved into HTMLInputElement. Removed call to setPositionFromValue
+        and instead just rely on the call to setNeedsLayout. Fix the setNeedsLayout call to be
+        a normal setNeedsLayout(true), not a setNeedsLayout(true, false), because we do want
+        this to be propagated to the parent -- it's not called during layout.
+        (WebCore::RenderSlider::setValueForPosition): Refactor to use the new SliderRange
+        class. Also don't call setCurrentPosition; instead just call setNeedsLayout.
+        (WebCore::RenderSlider::currentPosition): Use the actual position of the renderer rather
+        than the style to find the position; that means this needs to be done after layout is done.
+        Also removed unneeded runtime checks and replaced them with assertions, after checking
+        all callers to see they already guarantee this.
+        (WebCore::RenderSlider::trackSize): Removed unneeded runtime checks and replaced them
+        with assertions, after checking all callers to see they already guarantee this.
+        (WebCore::RenderSlider::inDragMode): Added a null check for m_thumb so this won't
+        crash if called early on a brand new RenderSlider.
+
+        * rendering/RenderSlider.h: Made all functions private except for forwardEvent and inDragMode.
+        Renamed HTMLSliderThumbElement to SliderThumbElement because we only use the HTML
+        prefix for public DOM classes, not elements used as implementation details. Made the
+        mouseEventIsInThumb function non-virtual. Removed the return value and argument from
+        setPositionFromValue. Removed valueChanged and setCurrentPosition. Removed the oldStyle
+        argument to createThumbStyle (see above). Made SliderThumbElement a friend so it can use some
+        private member functions.
+
 2009-03-25  Eli Fidler  <eli.fidler@torchmobile.com>
 
         Reviewed by George Staikos.
index d8ad152..f54ccb8 100644 (file)
@@ -1,6 +1,5 @@
-/**
- *
- * Copyright (C) 2006, 2007, 2008 Apple Computer, Inc.
+/*
+ * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -35,6 +34,7 @@
 #include "MouseEvent.h"
 #include "RenderLayer.h"
 #include "RenderTheme.h"
+#include "RenderView.h"
 #include <wtf/MathExtras.h>
 
 using std::min;
@@ -43,40 +43,95 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-const int defaultTrackLength = 129;
+static const int defaultTrackLength = 129;
+
+// FIXME: The SliderRange class and functions are entirely based on the DOM,
+// and could be put with HTMLInputElement (possibly with a new name) instead of here.
+struct SliderRange {
+    bool isIntegral;
+    double minimum;
+    double maximum;
+
+    explicit SliderRange(HTMLInputElement*);
+    double clampValue(double value);
+    double valueFromElement(HTMLInputElement*, bool* wasClamped = 0);
+};
+
+SliderRange::SliderRange(HTMLInputElement* element)
+{
+    // FIXME: What's the right way to handle an integral range with non-integral minimum and maximum?
+    // Currently values are guaranteed to be integral but could be outside the range in that case.
+
+    isIntegral = !equalIgnoringCase(element->getAttribute(precisionAttr), "float");
+
+    // FIXME: This treats maximum strings that can't be parsed as 0, but perhaps 100 would be more appropriate.
+    const AtomicString& maxString = element->getAttribute(maxAttr);
+    maximum = maxString.isNull() ? 100.0 : maxString.toDouble();
+
+    // If the maximum is smaller, use it as the minimum.
+    minimum = min(element->getAttribute(minAttr).toDouble(), maximum);
+}
+
+double SliderRange::clampValue(double value)
+{
+    double clampedValue = max(minimum, min(value, maximum));
+    return isIntegral ? round(clampedValue) : clampedValue;
+}
 
-class HTMLSliderThumbElement : public HTMLDivElement {
+double SliderRange::valueFromElement(HTMLInputElement* element, bool* wasClamped)
+{
+    String valueString = element->value();
+    double oldValue = valueString.isNull() ? (minimum + maximum) / 2 : valueString.toDouble();
+    double newValue = clampValue(oldValue);
+
+    if (wasClamped)
+        *wasClamped = valueString.isNull() || newValue != oldValue;
+
+    return newValue;
+}
+
+// Returns a value between 0 and 1.
+// As with SliderRange, this could be on HTMLInputElement instead of here.
+static double sliderPosition(HTMLInputElement* element)
+{
+    SliderRange range(element);
+    double value = range.valueFromElement(element);
+    return (value - range.minimum) / (range.maximum - range.minimum);
+}
+
+class SliderThumbElement : public HTMLDivElement {
 public:
-    HTMLSliderThumbElement(Document*, Node* shadowParent = 0);
-        
+    SliderThumbElement(Document*, Node* shadowParent);
+    
+    bool inDragMode() const { return m_inDragMode; }
+
     virtual void defaultEventHandler(Event*);
+
+private:        
     virtual bool isShadowNode() const { return true; }
     virtual Node* shadowParentNode() { return m_shadowParent; }
-    
-    bool inDragMode() const { return m_inDragMode; }
-private:
+
     Node* m_shadowParent;
     FloatPoint m_initialClickPoint;       // initial click point in RenderSlider-local coordinates
     int m_initialPosition;
     bool m_inDragMode;
 };
 
-HTMLSliderThumbElement::HTMLSliderThumbElement(Document* doc, Node* shadowParent)
-    : HTMLDivElement(divTag, doc)
+SliderThumbElement::SliderThumbElement(Document* document, Node* shadowParent)
+    : HTMLDivElement(divTag, document)
     , m_shadowParent(shadowParent)
-    , m_initialClickPoint(IntPoint())
     , m_initialPosition(0)
     , m_inDragMode(false)
 {
 }
 
-void HTMLSliderThumbElement::defaultEventHandler(Event* event)
+void SliderThumbElement::defaultEventHandler(Event* event)
 {
     const AtomicString& eventType = event->type();
     if (eventType == eventNames().mousedownEvent && event->isMouseEvent() && static_cast<MouseEvent*>(event)->button() == LeftButton) {
         MouseEvent* mouseEvent = static_cast<MouseEvent*>(event);
         RenderSlider* slider;
-        if (document()->frame() && renderer() && renderer()->parent() &&
+        if (document()->frame() && renderer() &&
                 (slider = static_cast<RenderSlider*>(renderer()->parent())) &&
                 slider->mouseEventIsInThumb(mouseEvent)) {
             
@@ -106,15 +161,9 @@ void HTMLSliderThumbElement::defaultEventHandler(Event* event)
             RenderSlider* slider = static_cast<RenderSlider*>(renderer()->parent());
 
             FloatPoint curPoint = slider->absoluteToLocal(mouseEvent->absoluteLocation(), false, true);
-            int newPosition = slider->positionForOffset(
-                IntPoint(m_initialPosition + curPoint.x() - m_initialClickPoint.x()
-                        + (renderBox()->width() / 2), 
-                    m_initialPosition + curPoint.y() - m_initialClickPoint.y()
-                        + (renderBox()->height() / 2)));
-            if (slider->currentPosition() != newPosition) {
-                slider->setCurrentPosition(newPosition);
-                slider->valueChanged();
-            }
+            IntPoint eventOffset(m_initialPosition + curPoint.x() - m_initialClickPoint.x() + renderBox()->width() / 2, 
+                m_initialPosition + curPoint.y() - m_initialClickPoint.y() + renderBox()->height() / 2);
+            slider->setValueForPosition(slider->positionForOffset(eventOffset));
             event->setDefaultHandled();
             return;
         }
@@ -125,7 +174,6 @@ void HTMLSliderThumbElement::defaultEventHandler(Event* event)
 
 RenderSlider::RenderSlider(HTMLInputElement* element)
     : RenderBlock(element)
-    , m_thumb(0)
 {
 }
 
@@ -173,14 +221,14 @@ void RenderSlider::calcPrefWidths()
 void RenderSlider::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     RenderBlock::styleDidChange(diff, oldStyle);
-    
+
     if (m_thumb)
-        m_thumb->renderer()->setStyle(createThumbStyle(style(), m_thumb->renderer()->style()));
-        
+        m_thumb->renderer()->setStyle(createThumbStyle(style()));
+
     setReplaced(isInline());
 }
 
-PassRefPtr<RenderStyle> RenderSlider::createThumbStyle(const RenderStyle* parentStyle, const RenderStyle* oldStyle)
+PassRefPtr<RenderStyle> RenderSlider::createThumbStyle(const RenderStyle* parentStyle)
 {
     RefPtr<RenderStyle> style;
     RenderStyle* pseudoStyle = getCachedPseudoStyle(SLIDER_THUMB);
@@ -194,16 +242,11 @@ PassRefPtr<RenderStyle> RenderSlider::createThumbStyle(const RenderStyle* parent
         style->inheritFrom(parentStyle);
 
     style->setDisplay(BLOCK);
-    style->setPosition(RelativePosition);
-    if (oldStyle) {
-        style->setLeft(oldStyle->left());
-        style->setTop(oldStyle->top());
-    }
 
     if (parentStyle->appearance() == SliderVerticalPart)
-       style->setAppearance(SliderThumbVerticalPart);
+        style->setAppearance(SliderThumbVerticalPart);
     else if (parentStyle->appearance() == SliderHorizontalPart)
-       style->setAppearance(SliderThumbHorizontalPart);
+        style->setAppearance(SliderThumbHorizontalPart);
     else if (parentStyle->appearance() == MediaSliderPart)
         style->setAppearance(MediaSliderThumbPart);
 
@@ -211,42 +254,97 @@ PassRefPtr<RenderStyle> RenderSlider::createThumbStyle(const RenderStyle* parent
 }
 
 void RenderSlider::layout()
-{    
-    bool relayoutChildren = false;
-    
-    if (m_thumb && m_thumb->renderer()) {
-            
-        int oldWidth = width();
-        calcWidth();
-        int oldHeight = height();
-        calcHeight();
-        
-        if (oldWidth != width() || oldHeight != height())
-            relayoutChildren = true;  
-
-        // Allow the theme to set the size of the thumb
-        if (m_thumb->renderer()->style()->hasAppearance())
-            theme()->adjustSliderThumbSize(m_thumb->renderer());
+{
+    ASSERT(needsLayout());
+
+    RenderBox* thumb = m_thumb ? toRenderBox(m_thumb->renderer()) : 0;
+
+    IntSize baseSize(borderLeft() + paddingLeft() + paddingRight() + borderRight(),
+        borderTop() + paddingTop() + paddingBottom() + borderBottom());
+
+    if (thumb) {
+        // Allow the theme to set the size of the thumb.
+        if (thumb->style()->hasAppearance()) {
+            // FIXME: This should pass the style, not the renderer, to the theme.
+            theme()->adjustSliderThumbSize(thumb);
+        }
+
+        baseSize.expand(thumb->style()->width().calcMinValue(0), thumb->style()->height().calcMinValue(0));
+    }
+
+    LayoutRepainter repainter(*this, checkForRepaintDuringLayout());
+
+    IntSize oldSize = size();
+
+    setSize(baseSize);
+    calcWidth();
+    calcHeight();
+
+    IntRect overflowRect(IntPoint(), size());
+
+    if (thumb) {
+        if (oldSize != size())
+            thumb->setChildNeedsLayout(true, false);
+
+        LayoutStateMaintainer statePusher(view(), this, size());
+
+        IntRect oldThumbRect = thumb->frameRect();
 
+        thumb->layoutIfNeeded();
+
+        IntRect thumbRect;
+
+        thumbRect.setWidth(thumb->style()->width().calcMinValue(contentWidth()));
+        thumbRect.setHeight(thumb->style()->height().calcMinValue(contentHeight()));
+
+        double fraction = sliderPosition(static_cast<HTMLInputElement*>(node()));
+        IntRect contentRect = contentBoxRect();
         if (style()->appearance() == SliderVerticalPart) {
-            // FIXME: Handle percentage widths correctly. See http://bugs.webkit.org/show_bug.cgi?id=12104
-            m_thumb->renderer()->style()->setLeft(Length(contentWidth() / 2 - m_thumb->renderer()->style()->width().value() / 2, Fixed));
+            thumbRect.setX(contentRect.x() + (contentRect.width() - thumbRect.width()) / 2);
+            thumbRect.setY(contentRect.y() + static_cast<int>(nextafter((contentRect.height() - thumbRect.height()) + 1, 0) * (1 - fraction)));
         } else {
-            // FIXME: Handle percentage heights correctly. See http://bugs.webkit.org/show_bug.cgi?id=12104
-            m_thumb->renderer()->style()->setTop(Length(contentHeight() / 2 - m_thumb->renderer()->style()->height().value() / 2, Fixed));
+            thumbRect.setX(contentRect.x() + static_cast<int>(nextafter((contentRect.width() - thumbRect.width()) + 1, 0) * fraction));
+            thumbRect.setY(contentRect.y() + (contentRect.height() - thumbRect.height()) / 2);
         }
 
-        if (relayoutChildren)
-            setPositionFromValue(true);
+        thumb->setFrameRect(thumbRect);
+
+        if (thumb->checkForRepaintDuringLayout())
+            thumb->repaintDuringLayoutIfMoved(oldThumbRect);
+
+        statePusher.pop();
+
+        IntRect thumbOverflowRect = thumb->overflowRect();
+        thumbOverflowRect.move(thumb->x(), thumb->y());
+        overflowRect.unite(thumbOverflowRect);
     }
 
-    RenderBlock::layoutBlock(relayoutChildren);
+    // FIXME: m_overflowWidth and m_overflowHeight should be renamed
+    // m_overflowRight and m_overflowBottom.
+    m_overflowLeft = overflowRect.x();
+    m_overflowTop = overflowRect.y();
+    m_overflowWidth = overflowRect.right();
+    m_overflowHeight = overflowRect.bottom();
+
+    repainter.repaintAfterLayout();    
+
+    setNeedsLayout(false);
 }
 
 void RenderSlider::updateFromElement()
 {
+    HTMLInputElement* element = static_cast<HTMLInputElement*>(node());
+
+    // Send the value back to the element if the range changes it.
+    SliderRange range(element);
+    bool clamped;
+    double value = range.valueFromElement(element, &clamped);
+    if (clamped)
+        element->setValueFromRenderer(String::number(value));
+
+    // Layout will take care of the thumb's size and position.
     if (!m_thumb) {
-        m_thumb = new HTMLSliderThumbElement(document(), node());
+        m_thumb = new SliderThumbElement(document(), node());
         RefPtr<RenderStyle> thumbStyle = createThumbStyle(style());
         m_thumb->setRenderer(m_thumb->createRenderer(renderArena(), thumbStyle.get()));
         m_thumb->renderer()->setStyle(thumbStyle.release());
@@ -254,8 +352,7 @@ void RenderSlider::updateFromElement()
         m_thumb->setInDocument(true);
         addChild(m_thumb->renderer());
     }
-    setPositionFromValue();
-    setNeedsLayout(true, false);
+    setNeedsLayout(true);
 }
 
 bool RenderSlider::mouseEventIsInThumb(MouseEvent* evt)
@@ -279,77 +376,32 @@ void RenderSlider::setValueForPosition(int position)
 {
     if (!m_thumb || !m_thumb->renderer())
         return;
-    
-    const AtomicString& minStr = static_cast<HTMLInputElement*>(node())->getAttribute(minAttr);
-    const AtomicString& maxStr = static_cast<HTMLInputElement*>(node())->getAttribute(maxAttr);
-    const AtomicString& precision = static_cast<HTMLInputElement*>(node())->getAttribute(precisionAttr);
-    
-    double minVal = minStr.isNull() ? 0.0 : minStr.toDouble();
-    double maxVal = maxStr.isNull() ? 100.0 : maxStr.toDouble();
-    minVal = min(minVal, maxVal); // Make sure the range is sane.
-    
-    // Calculate the new value based on the position
-    double factor = (double)position / (double)trackSize();
-    if (style()->appearance() == SliderVerticalPart)
-        factor = 1.0 - factor;
-    double val = minVal + factor * (maxVal - minVal);
-            
-    val = max(minVal, min(val, maxVal)); // Make sure val is within min/max.
-
-    // Force integer value if not float.
-    if (!equalIgnoringCase(precision, "float"))
-        val = lround(val);
 
-    static_cast<HTMLInputElement*>(node())->setValueFromRenderer(String::number(val));
-    
-    if (position != currentPosition()) {
-        setCurrentPosition(position);
-        static_cast<HTMLInputElement*>(node())->onChange();
-    }
-}
+    HTMLInputElement* element = static_cast<HTMLInputElement*>(node());
 
-double RenderSlider::setPositionFromValue(bool inLayout)
-{
-    if (!m_thumb || !m_thumb->renderer())
-        return 0;
-    
-    if (!inLayout)
-        document()->updateLayout();
-        
-    String value = static_cast<HTMLInputElement*>(node())->value();
-    const AtomicString& minStr = static_cast<HTMLInputElement*>(node())->getAttribute(minAttr);
-    const AtomicString& maxStr = static_cast<HTMLInputElement*>(node())->getAttribute(maxAttr);
-    const AtomicString& precision = static_cast<HTMLInputElement*>(node())->getAttribute(precisionAttr);
-    
-    double minVal = minStr.isNull() ? 0.0 : minStr.toDouble();
-    double maxVal = maxStr.isNull() ? 100.0 : maxStr.toDouble();
-    minVal = min(minVal, maxVal); // Make sure the range is sane.
-    
-    double oldVal = value.isNull() ? (maxVal + minVal)/2.0 : value.toDouble();
-    double val = max(minVal, min(oldVal, maxVal)); // Make sure val is within min/max.
-        
-    // Force integer value if not float.
-    if (!equalIgnoringCase(precision, "float"))
-        val = lround(val);
-
-    // Calculate the new position based on the value
-    double factor = (val - minVal) / (maxVal - minVal);
+    // Calculate the new value based on the position, and send it to the element.
+    SliderRange range(element);
+    double fraction = static_cast<double>(position) / trackSize();
     if (style()->appearance() == SliderVerticalPart)
-        factor = 1.0 - factor;
+        fraction = 1 - fraction;
+    double value = range.clampValue(range.minimum + fraction * (range.maximum - range.minimum));
+    element->setValueFromRenderer(String::number(value));
 
-    setCurrentPosition((int)(factor * trackSize()));
-    
-    if (value.isNull() || val != oldVal)
-        static_cast<HTMLInputElement*>(node())->setValueFromRenderer(String::number(val));
-    
-    return val;
+    // 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->onChange();
+    }
 }
 
 int RenderSlider::positionForOffset(const IntPoint& p)
 {
     if (!m_thumb || !m_thumb->renderer())
         return 0;
-   
+
     int position;
     if (style()->appearance() == SliderVerticalPart)
         position = p.y() - m_thumb->renderBox()->height() / 2;
@@ -359,65 +411,44 @@ int RenderSlider::positionForOffset(const IntPoint& p)
     return max(0, min(position, trackSize()));
 }
 
-void RenderSlider::valueChanged()
-{
-    setValueForPosition(currentPosition());
-    static_cast<HTMLInputElement*>(node())->onChange();
-}
-
 int RenderSlider::currentPosition()
 {
-    if (!m_thumb || !m_thumb->renderer())
-        return 0;
-
-    if (style()->appearance() == SliderVerticalPart)
-        return m_thumb->renderer()->style()->top().value();
-    return m_thumb->renderer()->style()->left().value();
-}
-
-void RenderSlider::setCurrentPosition(int pos)
-{
-    if (!m_thumb || !m_thumb->renderer())
-        return;
+    ASSERT(m_thumb);
+    ASSERT(m_thumb->renderer());
 
     if (style()->appearance() == SliderVerticalPart)
-        m_thumb->renderer()->style()->setTop(Length(pos, Fixed));
-    else
-        m_thumb->renderer()->style()->setLeft(Length(pos, Fixed));
-
-    m_thumb->renderBox()->layer()->updateLayerPosition();
-    repaint();
-    m_thumb->renderer()->repaint();
+        return toRenderBox(m_thumb->renderer())->y() - contentBoxRect().y();
+    return toRenderBox(m_thumb->renderer())->x() - contentBoxRect().x();
 }
 
 int RenderSlider::trackSize()
 {
-    if (!m_thumb || !m_thumb->renderer())
-        return 0;
+    ASSERT(m_thumb);
+    ASSERT(m_thumb->renderer());
 
     if (style()->appearance() == SliderVerticalPart)
         return contentHeight() - m_thumb->renderBox()->height();
     return contentWidth() - m_thumb->renderBox()->width();
 }
 
-void RenderSlider::forwardEvent(Event* evt)
+void RenderSlider::forwardEvent(Event* event)
 {
-    if (evt->isMouseEvent()) {
-        MouseEvent* mouseEvt = static_cast<MouseEvent*>(evt);
-        if (evt->type() == eventNames().mousedownEvent && mouseEvt->button() == LeftButton) {
-            if (!mouseEventIsInThumb(mouseEvt)) {
-                IntPoint eventOffset = roundedIntPoint(absoluteToLocal(mouseEvt->absoluteLocation(), false, true));
+    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));
             }
         }
     }
 
-    m_thumb->defaultEventHandler(evt);
+    m_thumb->defaultEventHandler(event);
 }
 
 bool RenderSlider::inDragMode() const
 {
-    return m_thumb->inDragMode();
+    return m_thumb && m_thumb->inDragMode();
 }
 
 } // namespace WebCore
index 327f2d1..f1eab9c 100644 (file)
@@ -1,6 +1,5 @@
-/**
- *
- * Copyright (C) 2006 Apple Computer, Inc.
+/*
+ * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
 
 namespace WebCore {
 
-    class HTMLDivElement;
     class HTMLInputElement;
-    class HTMLSliderThumbElement;
     class MouseEvent;
+    class SliderThumbElement;
     
     class RenderSlider : public RenderBlock {
     public:
         RenderSlider(HTMLInputElement*);
-        ~RenderSlider();
+        virtual ~RenderSlider();
+
+        void forwardEvent(Event*);
+        bool inDragMode() const;
 
+    private:
         virtual const char* renderName() const { return "RenderSlider"; }
         virtual bool isSlider() const { return true; }
 
@@ -43,30 +45,25 @@ namespace WebCore {
         virtual void calcPrefWidths();
         virtual void layout();
         virtual void updateFromElement();
-        
-        virtual bool mouseEventIsInThumb(MouseEvent*);
+
+        bool mouseEventIsInThumb(MouseEvent*);
 
         void setValueForPosition(int position);
-        double setPositionFromValue(bool inLayout = false);
+        void setPositionFromValue();
         int positionForOffset(const IntPoint&);
 
-        void valueChanged();
-        
         int currentPosition();
-        void setCurrentPosition(int pos);        
-        
-        void forwardEvent(Event*);
-        bool inDragMode() const;
 
-    protected:
         virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
-    
-    private:
-        PassRefPtr<RenderStyle> createThumbStyle(const RenderStyle* parentStyle, const RenderStyle* oldStyle = 0);
+
+        PassRefPtr<RenderStyle> createThumbStyle(const RenderStyle* parentStyle);
+
         int trackSize();
 
-        RefPtr<HTMLSliderThumbElement> m_thumb;
-};
+        RefPtr<SliderThumbElement> m_thumb;
+
+        friend class SliderThumbElement;
+    };
 
 } // namespace WebCore