Attachment #50244 for bug #25648

View Details Raw Unified Return to Bug 25648

| Differences between



2009-03-08  Alpha Lam  <hclam@chromium.org>

        Reviewed by NOBODY (OOPS!).

        When the video controls are updated the whole video is redisplayed.
        https://bugs.webkit.org/show_bug.cgi?id=25648

        This change will prevent RenderSlider to propogate relayout to parents when
        updateFromElement() is called or value of the slider is updated. RenderSlider
        will instead perform a layout rooted at itself.
 
        Note: This patch will break some media Chromium layout tests.
 
        Test: fast/repaint/slider-update-value.html
        Test: fast/repaint/slider-thumb-change-height.html
 
        The following tests should still pass:
        fast/forms/slider-delete-while-dragging-thumb.html
        fast/forms/slider-mouse-events.html
        fast/forms/slider-onchange-event.html
        fast/forms/slider-padding.html
        fast/forms/slider-thumb-shared-style.html
        fast/forms/slider-thumb-stylability.html
        fast/forms/slider-transformed.html
        fast/forms/slider-zoomed.html
        fast/forms/thumbslider-crash.html
        fast/forms/thumbslider-no-parent-slider.html
        fast/forms/range-default-value.html
        fast/forms/range-reset.html
        fast/forms/range-thumb-height-percentage.html
        media/audio-delete-while-slider-thumb-clicked.html

        * rendering/RenderObject.h:
        (WebCore::objectIsRelayoutBoundary): RenderSlider is a layout subtree root.
        * rendering/RenderSlider.cpp:
        (WebCore::RenderSlider::layout):
        If RenderSlider is the subtree layout root the width doesn't change, so
        only adjust height in such case.
        Also fix the use of layout state pusher.
        (WebCore::RenderSlider::updateFromElement):
        Call setNeedsLayout(true) of the thumb so a relayout is scheduled.
        In the case of style change of the thumb that affects the slider, a relayout of the slider is scheduled.
        (WebCore::RenderSlider::setValueForPosition): ditto.
 
2010-03-05  Csaba Osztrogonác  <ossy@webkit.org>

        Unreviewed buildfix after r55593. (To fix Qt --minimal build.)

Lines 955-961 inline bool objectIsRelayoutBoundary(con WebCore/rendering/RenderObject.h_sec1
- WebCore/rendering/RenderObject.h -1 / +1 lines
955	// FIXME: In future it may be possible to broaden this condition in order to improve performance.	955	// FIXME: In future it may be possible to broaden this condition in order to improve performance.
956	// Table cells are excluded because even when their CSS height is fixed, their height()	956	// Table cells are excluded because even when their CSS height is fixed, their height()
957	// may depend on their contents.	957	// may depend on their contents.
958	return obj->isTextControl()	958	return obj->isTextControl() \|\| obj->isSlider()
959	\|\| (obj->hasOverflowClip() && !obj->style()->width().isIntrinsicOrAuto() && !obj->style()->height().isIntrinsicOrAuto() && !obj->style()->height().isPercent() && !obj->isTableCell())	959	\|\| (obj->hasOverflowClip() && !obj->style()->width().isIntrinsicOrAuto() && !obj->style()->height().isIntrinsicOrAuto() && !obj->style()->height().isPercent() && !obj->isTableCell())
960	#if ENABLE(SVG)	960	#if ENABLE(SVG)
961	\|\| obj->isSVGRoot()	961	\|\| obj->isSVGRoot()

Lines 342-348 void RenderSlider::layout() WebCore/rendering/RenderSlider.cpp_sec1
- WebCore/rendering/RenderSlider.cpp -4 / +13 lines
342		342
343	IntSize oldSize = size();	343	IntSize oldSize = size();
344		344
345	setSize(baseSize);	345	// If layout is limited to this subtree, then width does not change.
		346	if (!node() \|\| !view()->frameView() \|\| view()->frameView()->layoutRoot(true) != this)
		347	setWidth(baseSize.width());
		348	setHeight(baseSize.height());
		349
346	calcWidth();	350	calcWidth();
347	calcHeight();	351	calcHeight();
348		352
Lines 350-356 void RenderSlider::layout() WebCore/rendering/RenderSlider.cpp_sec2
350	if (oldSize != size())	354	if (oldSize != size())
351	thumb->setChildNeedsLayout(true, false);	355	thumb->setChildNeedsLayout(true, false);
352		356
353	LayoutStateMaintainer statePusher(view(), this, size());	357	LayoutStateMaintainer statePusher(view(), this, IntSize(x(), y()));
354		358
355	IntRect oldThumbRect = thumb->frameRect();	359	IntRect oldThumbRect = thumb->frameRect();
356		360
Lines 391-397 void RenderSlider::updateFromElement() WebCore/rendering/RenderSlider.cpp_sec3
391	m_thumb->setInDocument(true);	395	m_thumb->setInDocument(true);
392	addChild(m_thumb->renderer());	396	addChild(m_thumb->renderer());
393	}	397	}
394	setNeedsLayout(true);	398
		399	// The slider needs a relayout if its height doesn't match with the thumb.
		400	if (toRenderBox(m_thumb->renderer())->style()->height().calcMinValue(0) != height())
		401	setNeedsLayout(true);
		402	else
		403	m_thumb->renderer()->setNeedsLayout(true);
395	}	404	}
396		405
397	bool RenderSlider::mouseEventIsInThumb(MouseEvent* evt)	406	bool RenderSlider::mouseEventIsInThumb(MouseEvent* evt)
Lines 439-445 void RenderSlider::setValueForPosition(i WebCore/rendering/RenderSlider.cpp_sec4
439		448
440	// Also update the position if appropriate.	449	// Also update the position if appropriate.
441	if (position != currentPosition()) {	450	if (position != currentPosition()) {
442	setNeedsLayout(true);	451	m_thumb->renderer()->setNeedsLayout(true);
443		452
444	// FIXME: It seems like this could send extra change events if the same value is set	453	// FIXME: It seems like this could send extra change events if the same value is set
445	// multiple times with no layout in between.	454	// multiple times with no layout in between.



2009-03-08  Alpha Lam  <hclam@chromium.org>

        Reviewed by NOBODY (OOPS!).

        When the video controls are updated the whole video is redisplayed.
        https://bugs.webkit.org/show_bug.cgi?id=25648

        Adding the following two tests for slider.
        fast/repaint/slider-update-value.html
        fast/repaint/slider-thumb-change-height.html 

        * fast/repaint/slider-update-value.html: Added.
        Tests that updating value of a slider would cause repaint only within the boundary of the slider.
        * platform/mac/fast/repaint/slider-update-value-expected.checksum: Added.
        * platform/mac/fast/repaint/slider-update-value-expected.png: Added.
        * platform/mac/fast/repaint/slider-update-value-expected.txt: Added.
        * fast/repaint/slider-thumb-change-height.html: Added.
        Tests that updating the height of the slider thumb causes a relayout of the slider.
        * platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum: Added.
        * platform/mac/fast/repaint/slider-thumb-change-height-expected.png: Added.
        * platform/mac/fast/repaint/slider-thumb-change-height-expected.txt: Added.

2010-03-05  Simon Fraser  <simon.fraser@apple.com>

        Reviewed by Darin Adler.



<html>
<head>
  <title>Slider repaint test - test repaint after thumb style changes</title>
  <style>
    input
    {
        -webkit-appearance: none;
        border: solid;
    }

    input::-webkit-slider-thumb
    {
        -webkit-appearance: none;
        background: blue;
        width: 20px;
        height: 20px;
    }

    div
    {
        border-style: solid;
        border-width: medium;
        border-color: blue;
        background-color: yellow;
    }

    body
    {
        background-color: green;
    }
  </style>
  <script src="resources/repaint.js" type="text/javascript"></script>
  <script type="text/javascript">
    function repaintTest()
    {
       document.styleSheets[0].cssRules[1].style.height = "40px";
    }
  </script>
</head>
<body onload="runRepaintTest()">
  Tests that the height of a slider thumb will cause relayout and repaint of the slider<br>
  <div>
    <input type="range" id="slider">
  </div>
</body>
</html>



<html>
<head>
  <title>Slider repaint test</title>
  <style>
    input[type="range"] {
      width: 200px;
      margin: auto;
      outline: 1px solid orange;
    }
  </style>
  <script src="resources/repaint.js" type="text/javascript"></script>
  <script type="text/javascript">
    function repaintTest()
    {
        var slider = document.getElementById('slider');
        slider.value = "50";
    }
  </script>
</head>
<body onload="runRepaintTest()">
  Tests that updating value of slider causes repaint only within the slider. <br>
  <div>
    <input id="slider" type="range" max="100" value="0">
  </div>
</body>
</html>

Line 0 LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum_sec1
- LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum +1 lines
		1	6e3355f2ea671c3956c85dcb3b0c97a0



layer at (0,0) size 800x600
  RenderView at (0,0) size 800x600
layer at (0,0) size 800x600
  RenderBlock {HTML} at (0,0) size 800x600
    RenderBody {BODY} at (8,8) size 784x584 [bgcolor=#008000]
      RenderBlock (anonymous) at (0,0) size 784x18
        RenderText {#text} at (0,0) size 510x18
          text run at (0,0) width 510: "Tests that the height of a slider thumb will cause relayout and repaint of the slider"
        RenderBR {BR} at (510,14) size 0x0
      RenderBlock {DIV} at (0,18) size 784x56 [bgcolor=#FFFF00] [border: (3px solid #0000FF)]
        RenderSlider {INPUT} at (5,5) size 135x46 [bgcolor=#FFFFFF] [border: (3px solid #000000)]
          RenderBlock {DIV} at (57,3) size 20x40 [bgcolor=#0000FF]
        RenderText {#text} at (0,0) size 0x0

Line 0 LayoutTests/platform/mac/fast/repaint/slider-update-value-expected.checksum_sec1
- LayoutTests/platform/mac/fast/repaint/slider-update-value-expected.checksum +1 lines
		1	db239108460cd47a5065af7fa6a68b92



layer at (0,0) size 800x600
  RenderView at (0,0) size 800x600
layer at (0,0) size 800x600
  RenderBlock {HTML} at (0,0) size 800x600
    RenderBody {BODY} at (8,8) size 784x584
      RenderBlock (anonymous) at (0,0) size 784x18
        RenderText {#text} at (0,0) size 449x18
          text run at (0,0) width 449: "Tests that updating value of slider causes repaint only within the slider. "
        RenderBR {BR} at (449,14) size 0x0
      RenderBlock {DIV} at (0,18) size 784x15
        RenderSlider {INPUT} at (0,0) size 200x15 [bgcolor=#FFFFFF]
          RenderBlock {DIV} at (92,0) size 15x15
        RenderText {#text} at (0,0) size 0x0

Return to Bug 25648

Lines 1-3 WebCore/ChangeLog_sec1
- WebCore/ChangeLog +44 lines
		1	2009-03-08 Alpha Lam <hclam@chromium.org>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	When the video controls are updated the whole video is redisplayed.
		6	https://bugs.webkit.org/show_bug.cgi?id=25648
		7
		8	This change will prevent RenderSlider to propogate relayout to parents when
		9	updateFromElement() is called or value of the slider is updated. RenderSlider
		10	will instead perform a layout rooted at itself.
		11
		12	Note: This patch will break some media Chromium layout tests.
		13
		14	Test: fast/repaint/slider-update-value.html
		15	Test: fast/repaint/slider-thumb-change-height.html
		16
		17	The following tests should still pass:
		18	fast/forms/slider-delete-while-dragging-thumb.html
		19	fast/forms/slider-mouse-events.html
		20	fast/forms/slider-onchange-event.html
		21	fast/forms/slider-padding.html
		22	fast/forms/slider-thumb-shared-style.html
		23	fast/forms/slider-thumb-stylability.html
		24	fast/forms/slider-transformed.html
		25	fast/forms/slider-zoomed.html
		26	fast/forms/thumbslider-crash.html
		27	fast/forms/thumbslider-no-parent-slider.html
		28	fast/forms/range-default-value.html
		29	fast/forms/range-reset.html
		30	fast/forms/range-thumb-height-percentage.html
		31	media/audio-delete-while-slider-thumb-clicked.html
		32
		33	* rendering/RenderObject.h:
		34	(WebCore::objectIsRelayoutBoundary): RenderSlider is a layout subtree root.
		35	* rendering/RenderSlider.cpp:
		36	(WebCore::RenderSlider::layout):
		37	If RenderSlider is the subtree layout root the width doesn't change, so
		38	only adjust height in such case.
		39	Also fix the use of layout state pusher.
		40	(WebCore::RenderSlider::updateFromElement):
		41	Call setNeedsLayout(true) of the thumb so a relayout is scheduled.
		42	In the case of style change of the thumb that affects the slider, a relayout of the slider is scheduled.
		43	(WebCore::RenderSlider::setValueForPosition): ditto.
		44
1	2010-03-05 Csaba Osztrogonác <ossy@webkit.org>	45	2010-03-05 Csaba Osztrogonác <ossy@webkit.org>
2		46
3	Unreviewed buildfix after r55593. (To fix Qt --minimal build.)	47	Unreviewed buildfix after r55593. (To fix Qt --minimal build.)

Lines 1-3 LayoutTests/ChangeLog_sec1
- LayoutTests/ChangeLog +22 lines
		1	2009-03-08 Alpha Lam <hclam@chromium.org>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	When the video controls are updated the whole video is redisplayed.
		6	https://bugs.webkit.org/show_bug.cgi?id=25648
		7
		8	Adding the following two tests for slider.
		9	fast/repaint/slider-update-value.html
		10	fast/repaint/slider-thumb-change-height.html
		11
		12	* fast/repaint/slider-update-value.html: Added.
		13	Tests that updating value of a slider would cause repaint only within the boundary of the slider.
		14	* platform/mac/fast/repaint/slider-update-value-expected.checksum: Added.
		15	* platform/mac/fast/repaint/slider-update-value-expected.png: Added.
		16	* platform/mac/fast/repaint/slider-update-value-expected.txt: Added.
		17	* fast/repaint/slider-thumb-change-height.html: Added.
		18	Tests that updating the height of the slider thumb causes a relayout of the slider.
		19	* platform/mac/fast/repaint/slider-thumb-change-height-expected.checksum: Added.
		20	* platform/mac/fast/repaint/slider-thumb-change-height-expected.png: Added.
		21	* platform/mac/fast/repaint/slider-thumb-change-height-expected.txt: Added.
		22
1	2010-03-05 Simon Fraser <simon.fraser@apple.com>	23	2010-03-05 Simon Fraser <simon.fraser@apple.com>
2		24
3	Reviewed by Darin Adler.	25	Reviewed by Darin Adler.

Line 0 LayoutTests/fast/repaint/slider-thumb-change-height.html_sec1
- LayoutTests/fast/repaint/slider-thumb-change-height.html +46 lines
	1	<html>
	2	<head>
	3	<title>Slider repaint test - test repaint after thumb style changes</title>
	4	<style>
	5	input
	6	{
	7	-webkit-appearance: none;
	8	border: solid;
	9	}
	10
	11	input::-webkit-slider-thumb
	12	{
	13	-webkit-appearance: none;
	14	background: blue;
	15	width: 20px;
	16	height: 20px;
	17	}
	18
	19	div
	20	{
	21	border-style: solid;
	22	border-width: medium;
	23	border-color: blue;
	24	background-color: yellow;
	25	}
	26
	27	body
	28	{
	29	background-color: green;
	30	}
	31	</style>
	32	<script src="resources/repaint.js" type="text/javascript"></script>
	33	<script type="text/javascript">
	34	function repaintTest()
	35	{
	36	document.styleSheets[0].cssRules[1].style.height = "40px";
	37	}
	38	</script>
	39	</head>
	40	<body onload="runRepaintTest()">
	41	Tests that the height of a slider thumb will cause relayout and repaint of the slider<br>
	42	<div>
	43	<input type="range" id="slider">
	44	</div>
	45	</body>
	46	</html>

Line 0 LayoutTests/fast/repaint/slider-update-value.html_sec1
- LayoutTests/fast/repaint/slider-update-value.html +26 lines
	1	<html>
	2	<head>
	3	<title>Slider repaint test</title>
	4	<style>
	5	input[type="range"] {
	6	width: 200px;
	7	margin: auto;
	8	outline: 1px solid orange;
	9	}
	10	</style>
	11	<script src="resources/repaint.js" type="text/javascript"></script>
	12	<script type="text/javascript">
	13	function repaintTest()
	14	{
	15	var slider = document.getElementById('slider');
	16	slider.value = "50";
	17	}
	18	</script>
	19	</head>
	20	<body onload="runRepaintTest()">
	21	Tests that updating value of slider causes repaint only within the slider. <br>
	22	<div>
	23	<input id="slider" type="range" max="100" value="0">
	24	</div>
	25	</body>
	26	</html>

Line 0 LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.txt_sec1
- LayoutTests/platform/mac/fast/repaint/slider-thumb-change-height-expected.txt +13 lines
	1	layer at (0,0) size 800x600
	2	RenderView at (0,0) size 800x600
	3	layer at (0,0) size 800x600
	4	RenderBlock {HTML} at (0,0) size 800x600
	5	RenderBody {BODY} at (8,8) size 784x584 [bgcolor=#008000]
	6	RenderBlock (anonymous) at (0,0) size 784x18
	7	RenderText {#text} at (0,0) size 510x18
	8	text run at (0,0) width 510: "Tests that the height of a slider thumb will cause relayout and repaint of the slider"
	9	RenderBR {BR} at (510,14) size 0x0
	10	RenderBlock {DIV} at (0,18) size 784x56 [bgcolor=#FFFF00] [border: (3px solid #0000FF)]
	11	RenderSlider {INPUT} at (5,5) size 135x46 [bgcolor=#FFFFFF] [border: (3px solid #000000)]
	12	RenderBlock {DIV} at (57,3) size 20x40 [bgcolor=#0000FF]
	13	RenderText {#text} at (0,0) size 0x0