Title: [125733] branches/chromium/1180
- Revision
- 125733
- Author
- [email protected]
- Date
- 2012-08-15 18:27:41 -0700 (Wed, 15 Aug 2012)
Log Message
Merge 123637 - 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.
[email protected]
Review URL: https://chromiumcodereview.appspot.com/10823356
Modified Paths
Added Paths
Diff
Copied: branches/chromium/1180/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt (from rev 123637, trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt) (0 => 125733)
--- branches/chromium/1180/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt (rev 0)
+++ branches/chromium/1180/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show-expected.txt 2012-08-16 01:27:41 UTC (rev 125733)
@@ -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
+
+
Copied: branches/chromium/1180/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html (from rev 123637, trunk/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html) (0 => 125733)
--- branches/chromium/1180/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html (rev 0)
+++ branches/chromium/1180/LayoutTests/fast/overflow/setting-scrollTop-after-hide-show.html 2012-08-16 01:27:41 UTC (rev 125733)
@@ -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: branches/chromium/1180/Source/WebCore/rendering/RenderLayer.cpp (125732 => 125733)
--- branches/chromium/1180/Source/WebCore/rendering/RenderLayer.cpp 2012-08-16 01:22:26 UTC (rev 125732)
+++ branches/chromium/1180/Source/WebCore/rendering/RenderLayer.cpp 2012-08-16 01:27:41 UTC (rev 125733)
@@ -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"
@@ -190,6 +191,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
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
