Scroll position is lost after hide/show element
https://bugs.webkit.org/show_bug.cgi?id=72852
Source/WebCore:
Maintain the scroll position of an overflowing element in the ElementRareData when the scrollable
RenderLayer is destroyed, which can be used to restore the scroll position if the same element gets
back a RenderLayer.
WebKit behaviour will be the same as Firefox and IE. It differs from Opera as it does not reset the
scroll position when an element is moved to another location in the same document. However Opera resets
the scroll position for elements moved to another document, which matches other browsers.
Patch by Rakesh KN <rakesh.kn@motorola.com> on 2012-04-02
Reviewed by Julien Chaffraix.
Test: fast/overflow/scroll-div-hide-show.html
* dom/Element.cpp:
(WebCore::Element::removedFromDocument):
Reset the saved scroll offset if the node is moved to another location in the same document or another one.
(WebCore::Element::savedLayerScrollOffset):
(WebCore::Element::setSavedLayerScrollOffset):
* dom/Element.h:
Add helper functions to access the layer scroll offset from the element's rare data.
* dom/ElementRareData.h:
(ElementRareData):
Add the scroll offset book-keeping.
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::RenderLayer):
Restore the scroll offset.
(WebCore::RenderLayer::~RenderLayer):
Store the scroll offset if document is not being destroyed.
LayoutTests:
Patch by Rakesh KN <rakesh.kn@motorola.com> on 2012-04-02
Reviewed by Julien Chaffraix.
* fast/overflow/scroll-div-hide-show-expected.txt: Added.
* fast/overflow/scroll-div-hide-show.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@112919 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 19dc3ca..53d813a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-04-02 Rakesh KN <rakesh.kn@motorola.com>
+
+ Scroll position is lost after hide/show element
+ https://bugs.webkit.org/show_bug.cgi?id=72852
+
+ Reviewed by Julien Chaffraix.
+
+ * fast/overflow/scroll-div-hide-show-expected.txt: Added.
+ * fast/overflow/scroll-div-hide-show.html: Added.
+
2012-04-02 Stephen Chenney <schenney@chromium.org>
[Chromium] Update expectations for SVG Lion results
diff --git a/LayoutTests/fast/overflow/scroll-div-hide-show-expected.txt b/LayoutTests/fast/overflow/scroll-div-hide-show-expected.txt
new file mode 100644
index 0000000..9f48945
--- /dev/null
+++ b/LayoutTests/fast/overflow/scroll-div-hide-show-expected.txt
@@ -0,0 +1,78 @@
+Scrolled position should be restored when the div is hidden and shown again
+
+Div will be moved here.
+
+Scrolling to 0, 75
+Div's display is none now
+PASS e.scrollTop is 0
+Div's display is block again
+PASS e.scrollTop is 75
+
+Test that after changing the scroll position to different values the scroll position is restored
+Scrolling to 50, 50
+Div's display is none now
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+Div's display is block again
+PASS e.scrollTop is 50
+PASS e.scrollLeft is 50
+Scrolling to 30, 10
+Div's display is none now
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+Div's display is block again
+PASS e.scrollTop is 10
+PASS e.scrollLeft is 30
+Scrolling to 100, 0
+Div's display is none now
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+Div's display is block again
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 100
+
+Testing that scroll position is restored when height and width of scrolling area is changed
+Scrolling to 35, 75
+Div's display is none now
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+Div's display is block again
+PASS e.scrollTop is 75
+PASS e.scrollLeft is 35
+Adding some more text to div
+Scrolling to 125, 100
+Div's display is none now
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+Div's display is block again
+PASS e.scrollTop is 100
+PASS e.scrollLeft is 125
+
+Test that scroll position is reset when the node is moved to different location in the document
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+
+Testing that scroll position is restored for RTL texts
+Scrolling to 50, 150
+Div's display is none now
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+Div's display is block again
+PASS e.scrollTop is 150
+PASS e.scrollLeft is 50
+
+Test iframe scrolling
+Scrolling Iframe to (50, 75)
+Iframe's display is none now
+Iframe's display is block again
+PASS frame.contentWindow.pageYOffset is 75
+PASS frame.contentWindow.pageXOffset is 50
+
+Testing scroll offset getting reset when moved to other document
+Scrolling div to (50, 75)
+PASS e.scrollTop is 0
+PASS e.scrollLeft is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/overflow/scroll-div-hide-show.html b/LayoutTests/fast/overflow/scroll-div-hide-show.html
new file mode 100644
index 0000000..54d6a64
--- /dev/null
+++ b/LayoutTests/fast/overflow/scroll-div-hide-show.html
@@ -0,0 +1,191 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p>Scrolled position should be restored when the div is hidden and shown again</p>
+<div style="height:100px; width:100px; overflow: scroll; display: block;" id="main">
+<pre id="preId">
+line 1 with some text which has to be scroll to be visible
+line 2 with some text which has to be scroll to be visible
+line 3 with some text which has to be scroll to be visible
+line 4 with some text which has to be scroll to be visible
+line 5 with some text which has to be scroll to be visible
+line 6 with some text which has to be scroll to be visible
+line 7 with some text which has to be scroll to be visible
+line 8 with some text which has to be scroll to be visible
+line 9 with some text which has to be scroll to be visible
+line 10 with some text which has to be scroll to be visible
+line 11 with some text which has to be scroll to be visible
+line 12 with some text which has to be scroll to be visible
+</pre>
+</div>
+
+<div id="second">
+Div will be moved here.
+</div>
+
+<iframe id="frame" style="display:block; width:400px; height: 400px;"></iframe>
+<div id="console"></div>
+<script>
+var e = document.getElementById("main");
+debug("Scrolling to 0, 75");
+e.scrollTop = 75;
+e.style.display = 'none';
+debug("Div's display is none now");
+shouldBe('e.scrollTop', '0');
+e.style.display = 'block';
+debug("Div's display is block again");
+shouldBe('e.scrollTop', '75');
+
+debug("");
+debug("Test that after changing the scroll position to different values the scroll position is restored");
+debug("Scrolling to 50, 50");
+e.scrollTop = 50;
+e.scrollLeft = 50;
+debug("Div's display is none now");
+e.style.display = 'none';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+debug("Div's display is block again");
+e.style.display = 'block';
+shouldBe('e.scrollTop', '50');
+shouldBe('e.scrollLeft', '50');
+
+debug("Scrolling to 30, 10");
+e.scrollTop = 10;
+e.scrollLeft = 30;
+debug("Div's display is none now");
+e.style.display = 'none';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+debug("Div's display is block again");
+e.style.display = 'block';
+shouldBe('e.scrollTop', '10');
+shouldBe('e.scrollLeft', '30');
+
+debug("Scrolling to 100, 0");
+e.scrollTop = 0;
+e.scrollLeft = 100;
+debug("Div's display is none now");
+e.style.display = 'none';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+debug("Div's display is block again");
+e.style.display = 'block';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '100');
+
+debug("");
+debug("Testing that scroll position is restored when height and width of scrolling area is changed");
+e.style.height = e.scrollHeight + 50;
+e.style.width = e.scrollWidth + 50;
+debug("Scrolling to 35, 75");
+e.scrollTop = 75;
+e.scrollLeft = 35;
+debug("Div's display is none now");
+e.style.display = 'none';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+debug("Div's display is block again");
+e.style.display = 'block';
+shouldBe('e.scrollTop', '75');
+shouldBe('e.scrollLeft', '35');
+debug("Adding some more text to div");
+e.innerHTML += "line 13 <br/> line 14 <br/> line 15 <br/> line 16 <br/> line 17 <br/>";
+debug("Scrolling to 125, 100");
+e.scrollTop = 100;
+e.scrollLeft = 125;
+debug("Div's display is none now");
+e.style.display = 'none';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+debug("Div's display is block again");
+e.style.display = 'block';
+shouldBe('e.scrollTop', '100');
+shouldBe('e.scrollLeft', '125');
+
+debug("");
+debug("Test that scroll position is reset when the node is moved to different location in the document");
+e.style.display = 'none';
+document.getElementById("second").appendChild(e);
+e.style.display = 'block';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+
+var divContent = document.getElementById("preId").innerHTML;
+
+debug("");
+debug("Testing that scroll position is restored for RTL texts");
+document.getElementById("preId").innerHTML = "ممرحبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nحبًامرحبًامرحبًامرحبًارحبًا\
+ \nممرحبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nممرحبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا\
+ \nبًامرحبًامرحبًامرحبًامرحبًامرحبًارحبًا";
+
+e.dir="rtl";
+
+debug("Scrolling to 50, 150");
+e.scrollTop = 150;
+e.scrollLeft = 50;
+debug("Div's display is none now");
+e.style.display = 'none';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+debug("Div's display is block again");
+e.style.display = 'block';
+shouldBe('e.scrollTop', '150');
+shouldBe('e.scrollLeft', '50');
+
+debug("");
+debug("Test iframe scrolling");
+var frame = document.getElementById('frame');
+var doc = frame.contentDocument.open();
+doc.write("<div style='width:100px; height:100px; background:yellow;'></div>");
+doc.close();
+frame.contentDocument.body.style.width = "2000px";
+frame.contentDocument.body.style.height = "2000px";
+
+debug("Scrolling Iframe to (50, 75)");
+frame.contentWindow.scrollTo(50, 75);
+
+frame.style.display = 'none';
+debug("Iframe's display is none now");
+
+frame.style.display = 'block';
+debug("Iframe's display is block again");
+
+shouldBe('frame.contentWindow.pageYOffset', '75');
+shouldBe('frame.contentWindow.pageXOffset', '50');
+
+debug("");
+debug("Testing scroll offset getting reset when moved to other document");
+document.getElementById("preId").innerHTML = divContent;
+e.dir="ltr";
+debug("Scrolling div to (50, 75)");
+e.scrollTop = 75;
+e.scrollLeft = 50;
+e.style.display = 'none';
+frame.contentDocument.body.appendChild(e);
+e.style.display = 'block';
+shouldBe('e.scrollTop', '0');
+shouldBe('e.scrollLeft', '0');
+
+successfullyParsed = true;
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index bc6aeb8..ed4a4e2 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2012-04-02 Rakesh KN <rakesh.kn@motorola.com>
+
+ Scroll position is lost after hide/show element
+ https://bugs.webkit.org/show_bug.cgi?id=72852
+
+ Maintain the scroll position of an overflowing element in the ElementRareData when the scrollable
+ RenderLayer is destroyed, which can be used to restore the scroll position if the same element gets
+ back a RenderLayer.
+
+ WebKit behaviour will be the same as Firefox and IE. It differs from Opera as it does not reset the
+ scroll position when an element is moved to another location in the same document. However Opera resets
+ the scroll position for elements moved to another document, which matches other browsers.
+
+ Reviewed by Julien Chaffraix.
+
+ Test: fast/overflow/scroll-div-hide-show.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::removedFromDocument):
+ Reset the saved scroll offset if the node is moved to another location in the same document or another one.
+
+ (WebCore::Element::savedLayerScrollOffset):
+ (WebCore::Element::setSavedLayerScrollOffset):
+ * dom/Element.h:
+ Add helper functions to access the layer scroll offset from the element's rare data.
+
+ * dom/ElementRareData.h:
+ (ElementRareData):
+ Add the scroll offset book-keeping.
+
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::RenderLayer):
+ Restore the scroll offset.
+ (WebCore::RenderLayer::~RenderLayer):
+ Store the scroll offset if document is not being destroyed.
+
2012-04-02 Alexis Menard <alexis.menard@openbossa.org>
Rename CSSPropertyLonghand files to StylePropertyShorthand.
diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
index ca2a7e0..da570ba 100644
--- a/Source/WebCore/dom/Element.cpp
+++ b/Source/WebCore/dom/Element.cpp
@@ -903,6 +903,8 @@
void Element::removedFromDocument()
{
+ setSavedLayerScrollOffset(IntSize());
+
if (m_attributeData) {
if (hasID()) {
Attribute* idItem = getAttributeItem(document()->idAttributeName());
@@ -2066,4 +2068,16 @@
return ensureRareData()->ensureCachedHTMLCollection(this, type);
}
+IntSize Element::savedLayerScrollOffset() const
+{
+ return hasRareData() ? rareData()->m_savedLayerScrollOffset : IntSize();
+}
+
+void Element::setSavedLayerScrollOffset(const IntSize& size)
+{
+ if (size.isZero() && !hasRareData())
+ return;
+ ensureRareData()->m_savedLayerScrollOffset = size;
+}
+
} // namespace WebCore
diff --git a/Source/WebCore/dom/Element.h b/Source/WebCore/dom/Element.h
index 69c7e8d..be26547 100644
--- a/Source/WebCore/dom/Element.h
+++ b/Source/WebCore/dom/Element.h
@@ -402,6 +402,9 @@
bool hasID() const;
bool hasClass() const;
+ IntSize savedLayerScrollOffset() const;
+ void setSavedLayerScrollOffset(const IntSize&);
+
protected:
Element(const QualifiedName& tagName, Document* document, ConstructionType type)
: ContainerNode(document, type)
diff --git a/Source/WebCore/dom/ElementRareData.h b/Source/WebCore/dom/ElementRareData.h
index fa525d3..325474b 100644
--- a/Source/WebCore/dom/ElementRareData.h
+++ b/Source/WebCore/dom/ElementRareData.h
@@ -74,6 +74,8 @@
bool m_styleAffectedByEmpty;
+ IntSize m_savedLayerScrollOffset;
+
#if ENABLE(FULLSCREEN_API)
bool m_containsFullScreenElement;
#endif
diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
index 1617c04..c557135 100644
--- a/Source/WebCore/rendering/RenderLayer.cpp
+++ b/Source/WebCore/rendering/RenderLayer.cpp
@@ -194,6 +194,14 @@
m_visibleContentStatusDirty = false;
m_hasVisibleContent = renderer->style()->visibility() == VISIBLE;
}
+
+ Node* node = renderer->node();
+ if (node && node->isElementNode()) {
+ // We save and restore only the scrollOffset as the other scroll values are recalculated.
+ Element* element = toElement(node);
+ m_scrollOffset = element->savedLayerScrollOffset();
+ element->setSavedLayerScrollOffset(IntSize());
+ }
}
RenderLayer::~RenderLayer()
@@ -208,6 +216,12 @@
frameView->removeScrollableArea(this);
}
+ if (!m_renderer->documentBeingDestroyed()) {
+ Node* node = m_renderer->node();
+ if (node && node->isElementNode())
+ toElement(node)->setSavedLayerScrollOffset(m_scrollOffset);
+ }
+
destroyScrollbar(HorizontalScrollbar);
destroyScrollbar(VerticalScrollbar);
