Title: [123637] trunk
- Revision
- 123637
- Author
- bda...@apple.com
- Date
- 2012-07-25 11:15:17 -0700 (Wed, 25 Jul 2012)
Log Message
https://bugs.webkit.org/show_bug.cgi?id=89114
REGRESSION (r112919): Setting scrollTop after setting display from none to block
fails
-and corresponding-
<rdar://problem/11656050>
Reviewed by Simon Fraser.
Source/WebCore:
ScrollAnimatorMac::immediateScrollTo() and ScrollAnimatorMac::immediateScrollBy()
both have an optimization in place so that they do not call
notifyPositionChanged() if the new scroll offset matches the ScrollAnimator's
cached m_currentPosX and m_currentPosY. So revision 112919 caused troubled with
this optimization because it allowed RenderLayers to restore a scrollOffset from
the Element if there is one cached there. This caused the RenderLayer to have a
scrollOffset that is improperly out-of-synch with the ScrollAnimator's
currentPosition (which will just be 0,0 since it is being re-created like the
RenderLayer). This fix makes sure they are in synch by calling
setCurrentPosition() on the ScrollAnimator when the cached position is non-zero.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::RenderLayer):
LayoutTests:
* fast/overflow/setting-scrollTop-after-hide-show-expected.txt: Added.
* fast/overflow/setting-scrollTop-after-hide-show.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (123636 => 123637)
--- trunk/LayoutTests/ChangeLog 2012-07-25 18:14:13 UTC (rev 123636)
+++ trunk/LayoutTests/ChangeLog 2012-07-25 18:15:17 UTC (rev 123637)
@@ -1,3 +1,16 @@
+2012-07-25 Beth Dakin <bda...@apple.com>
+
+ https://bugs.webkit.org/show_bug.cgi?id=89114
+ REGRESSION (r112919): Setting scrollTop after setting display from none to block
+ fails
+ -and corresponding-
+ <rdar://problem/11656050>
+
+ Reviewed by Simon Fraser.
+
+ * fast/overflow/setting-scrollTop-after-hide-show-expected.txt: Added.
+ * fast/overflow/setting-scrollTop-after-hide-show.html: Added.
+
2012-07-25 Andreas Kling <kl...@webkit.org>
Make ElementAttributeData a variable-sized object to reduce memory use.
Added: trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt (0 => 123637)
--- trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt 2012-07-25 18:15:17 UTC (rev 123637)
@@ -0,0 +1,7 @@
+In this test, we set a new scrollTop for a scrolling div, and then we make the div display:none. The test ensures that when we bring the div back by giving it a display value of block, that we also restore its scroll position. The test also ensures that we are able to set a new scrollTop value of 0 after that.
+
+scrollTop after restoring div: 20
+
+scrollTop after setting scrollTop back to 0: 0
+
+
Added: trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html (0 => 123637)
--- trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html (rev 0)
+++ trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html 2012-07-25 18:15:17 UTC (rev 123637)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<script>
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ function log(message)
+ {
+ document.getElementById("console").appendChild(document.createTextNode(message + "\n"));
+ }
+</script>
+<body>
+ <p>In this test, we set a new scrollTop for a scrolling div, and then we make the div display:none. The test ensures that when we bring the div back by giving it a display value of block, that we also restore its scroll position. The test also ensures that we are able to set a new scrollTop value of 0 after that.</p>
+ <div id="scroller" style="height: 20px; overflow: scroll;">
+ <div style="height: 60px;"></div>
+ </div>
+ <pre id="console"></pre>
+ <script>
+ a = document.getElementById('scroller');
+ a.scrollTop = 20;
+ a.style.display = 'none';
+ a.scrollTop = 20;
+ a.style.display = 'block';
+ log('scrollTop after restoring div: ' + a.scrollTop + '\n');
+ a.scrollTop = 0;
+ log('scrollTop after setting scrollTop back to 0: ' + a.scrollTop + '\n');
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (123636 => 123637)
--- trunk/Source/WebCore/ChangeLog 2012-07-25 18:14:13 UTC (rev 123636)
+++ trunk/Source/WebCore/ChangeLog 2012-07-25 18:15:17 UTC (rev 123637)
@@ -1,3 +1,26 @@
+2012-07-25 Beth Dakin <bda...@apple.com>
+
+ https://bugs.webkit.org/show_bug.cgi?id=89114
+ REGRESSION (r112919): Setting scrollTop after setting display from none to block
+ fails
+ -and corresponding-
+ <rdar://problem/11656050>
+
+ Reviewed by Simon Fraser.
+
+ ScrollAnimatorMac::immediateScrollTo() and ScrollAnimatorMac::immediateScrollBy()
+ both have an optimization in place so that they do not call
+ notifyPositionChanged() if the new scroll offset matches the ScrollAnimator's
+ cached m_currentPosX and m_currentPosY. So revision 112919 caused troubled with
+ this optimization because it allowed RenderLayers to restore a scrollOffset from
+ the Element if there is one cached there. This caused the RenderLayer to have a
+ scrollOffset that is improperly out-of-synch with the ScrollAnimator's
+ currentPosition (which will just be 0,0 since it is being re-created like the
+ RenderLayer). This fix makes sure they are in synch by calling
+ setCurrentPosition() on the ScrollAnimator when the cached position is non-zero.
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::RenderLayer):
+
2012-07-25 Andreas Kling <kl...@webkit.org>
Make ElementAttributeData a variable-sized object to reduce memory use.
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (123636 => 123637)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-07-25 18:14:13 UTC (rev 123636)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2012-07-25 18:15:17 UTC (rev 123637)
@@ -86,6 +86,7 @@
#include "RenderTreeAsText.h"
#include "RenderView.h"
#include "ScaleTransformOperation.h"
+#include "ScrollAnimator.h"
#include "Scrollbar.h"
#include "ScrollbarTheme.h"
#include "Settings.h"
@@ -195,6 +196,8 @@
// We save and restore only the scrollOffset as the other scroll values are recalculated.
Element* element = toElement(node);
m_scrollOffset = element->savedLayerScrollOffset();
+ if (!m_scrollOffset.isZero())
+ scrollAnimator()->setCurrentPosition(FloatPoint(m_scrollOffset.width(), m_scrollOffset.height()));
element->setSavedLayerScrollOffset(IntSize());
}
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
