- Revision
- 126839
- Author
- [email protected]
- Date
- 2012-08-27 21:20:12 -0700 (Mon, 27 Aug 2012)
Log Message
REGRESSION(r109480): Form state for iframe content is not restored
https://bugs.webkit.org/show_bug.cgi?id=90870
Reviewed by Jochen Eisinger.
Source/WebCore:
Since r109480, we have restored form state only for documents
loaded by FrameLoader::loadItem(). However we should restore form
state for documents in sub-frames of documents loaded by
FrameLoader::loadItem().
Test: fast/loader/form-state-restore-with-frames.html
* history/HistoryItem.cpp:
(WebCore::HistoryItem::isAncestorOf):
Added. A function to search descendants for the specified
HistoryItem. This is used by isAssociatedToRequestedHistoryItem().
* history/HistoryItem.h:
(HistoryItem): Declare isAncestorOf().
* loader/HistoryController.cpp:
(WebCore::HistoryController::saveDocumentState):
Don't save form state for detached document.
This is needed because saveDocumentState() is called twice; before
document detach and after document detach. We need to avoid the
latter call because formElementsState() for a detached document
produces an empty state.
(WebCore::isAssociatedToRequestedHistoryItem):
Added. This function checks the current HistoryItem is associated
to the HistoryItem specified to FrameLoader::loadItem().
(WebCore::HistoryController::restoreDocumentState):
Uses isAssociatedToRequestedHistoryItem().
LayoutTests:
* fast/loader/form-state-restore-with-frames.html: Added.
* fast/loader/form-state-restore-with-frames-expected.txt: Added.
* fast/loader/resources/form-state-restore-with-frames-1.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (126838 => 126839)
--- trunk/LayoutTests/ChangeLog 2012-08-28 03:39:01 UTC (rev 126838)
+++ trunk/LayoutTests/ChangeLog 2012-08-28 04:20:12 UTC (rev 126839)
@@ -1,3 +1,14 @@
+2012-08-27 Kent Tamura <[email protected]>
+
+ REGRESSION(r109480): Form state for iframe content is not restored
+ https://bugs.webkit.org/show_bug.cgi?id=90870
+
+ Reviewed by Jochen Eisinger.
+
+ * fast/loader/form-state-restore-with-frames.html: Added.
+ * fast/loader/form-state-restore-with-frames-expected.txt: Added.
+ * fast/loader/resources/form-state-restore-with-frames-1.html: Added.
+
2012-08-27 Julien Chaffraix <[email protected]>
Crash in RenderTable::removeCaption
Added: trunk/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt (0 => 126839)
--- trunk/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt 2012-08-28 04:20:12 UTC (rev 126839)
@@ -0,0 +1,7 @@
+PASS frame$($("frame1"), "input2").value is "value2"
+PASS frame$(frame$($("frame1"), "frame2"), "input3").value is "value3"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
Added: trunk/LayoutTests/fast/loader/form-state-restore-with-frames.html (0 => 126839)
--- trunk/LayoutTests/fast/loader/form-state-restore-with-frames.html (rev 0)
+++ trunk/LayoutTests/fast/loader/form-state-restore-with-frames.html 2012-08-28 04:20:12 UTC (rev 126839)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<meta http-equiv="pragma" content="no-cache">
+<meta http-equiv="cache-control" content="no-cache">
+</head>
+<body _onload_="startTest()">
+<input name="name1" id="input1">
+<iframe id="frame1" src=""
+</iframe>
+<form id="form1" action=""
+<input type="submit">
+</form>
+
+<script>
+function $(id) {
+ return document.getElementById(id);
+}
+function frame$(frame, id) {
+ return frame.contentDocument.getElementById(id);
+}
+
+function startTest() {
+ if ($('input1').value == 'visited') {
+ shouldBeEqualToString('frame$($("frame1"), "input2").value', 'value2');
+ shouldBeEqualToString('frame$(frame$($("frame1"), "frame2"), "input3").value', 'value3');
+ finishJSTest();
+ } else {
+ setTimeout(function() {
+ $('input1').value = 'visited';
+ frame$($('frame1'), 'input2').value = 'value2';
+ frame$(frame$($('frame1'), 'frame2'), 'input3').value = 'value3';
+ $('form1').submit();
+ }, 0);
+ }
+}
+
+jsTestIsAsync = true;
+</script>
+<script src=""
+</body>
Added: trunk/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html (0 => 126839)
--- trunk/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html (rev 0)
+++ trunk/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html 2012-08-28 04:20:12 UTC (rev 126839)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<input name="name2" id="input2">
+<iframe id="frame2" srcdoc="<input name=name3 id=input3>">
+</iframe>
Modified: trunk/Source/WebCore/ChangeLog (126838 => 126839)
--- trunk/Source/WebCore/ChangeLog 2012-08-28 03:39:01 UTC (rev 126838)
+++ trunk/Source/WebCore/ChangeLog 2012-08-28 04:20:12 UTC (rev 126839)
@@ -1,3 +1,36 @@
+2012-08-27 Kent Tamura <[email protected]>
+
+ REGRESSION(r109480): Form state for iframe content is not restored
+ https://bugs.webkit.org/show_bug.cgi?id=90870
+
+ Reviewed by Jochen Eisinger.
+
+ Since r109480, we have restored form state only for documents
+ loaded by FrameLoader::loadItem(). However we should restore form
+ state for documents in sub-frames of documents loaded by
+ FrameLoader::loadItem().
+
+ Test: fast/loader/form-state-restore-with-frames.html
+
+ * history/HistoryItem.cpp:
+ (WebCore::HistoryItem::isAncestorOf):
+ Added. A function to search descendants for the specified
+ HistoryItem. This is used by isAssociatedToRequestedHistoryItem().
+ * history/HistoryItem.h:
+ (HistoryItem): Declare isAncestorOf().
+ * loader/HistoryController.cpp:
+ (WebCore::HistoryController::saveDocumentState):
+ Don't save form state for detached document.
+ This is needed because saveDocumentState() is called twice; before
+ document detach and after document detach. We need to avoid the
+ latter call because formElementsState() for a detached document
+ produces an empty state.
+ (WebCore::isAssociatedToRequestedHistoryItem):
+ Added. This function checks the current HistoryItem is associated
+ to the HistoryItem specified to FrameLoader::loadItem().
+ (WebCore::HistoryController::restoreDocumentState):
+ Uses isAssociatedToRequestedHistoryItem().
+
2012-08-27 Ian Vollick <[email protected]>
[chromium] Should accelerate rotations of >= 180 degrees
Modified: trunk/Source/WebCore/history/HistoryItem.cpp (126838 => 126839)
--- trunk/Source/WebCore/history/HistoryItem.cpp 2012-08-28 03:39:01 UTC (rev 126838)
+++ trunk/Source/WebCore/history/HistoryItem.cpp 2012-08-28 04:20:12 UTC (rev 126839)
@@ -532,6 +532,18 @@
m_children.clear();
}
+bool HistoryItem::isAncestorOf(const HistoryItem* item) const
+{
+ for (size_t i = 0; i < m_children.size(); ++i) {
+ HistoryItem* child = m_children[i].get();
+ if (child == item)
+ return true;
+ if (child->isAncestorOf(item))
+ return true;
+ }
+ return false;
+}
+
// We do same-document navigation if going to a different item and if either of the following is true:
// - The other item corresponds to the same document (for history entries created via pushState or fragment changes).
// - The other item corresponds to the same set of documents, including frames (for history entries created via regular navigation)
Modified: trunk/Source/WebCore/history/HistoryItem.h (126838 => 126839)
--- trunk/Source/WebCore/history/HistoryItem.h 2012-08-28 03:39:01 UTC (rev 126838)
+++ trunk/Source/WebCore/history/HistoryItem.h 2012-08-28 04:20:12 UTC (rev 126839)
@@ -171,6 +171,7 @@
const HistoryItemVector& children() const;
bool hasChildren() const;
void clearChildren();
+ bool isAncestorOf(const HistoryItem*) const;
bool shouldDoSameDocumentNavigationTo(HistoryItem* otherItem) const;
bool hasSameFrames(HistoryItem* otherItem) const;
Modified: trunk/Source/WebCore/loader/HistoryController.cpp (126838 => 126839)
--- trunk/Source/WebCore/loader/HistoryController.cpp 2012-08-28 03:39:01 UTC (rev 126838)
+++ trunk/Source/WebCore/loader/HistoryController.cpp 2012-08-28 04:20:12 UTC (rev 126839)
@@ -163,7 +163,7 @@
Document* document = m_frame->document();
ASSERT(document);
- if (item->isCurrentDocument(document)) {
+ if (item->isCurrentDocument(document) && document->attached()) {
LOG(Loading, "WebCoreLoading %s: saving form state to %p", m_frame->tree()->uniqueName().string().utf8().data(), item);
item->setDocumentState(document->formElementsState());
}
@@ -179,6 +179,22 @@
}
}
+static inline bool isAssociatedToRequestedHistoryItem(const HistoryItem* current, Frame* frame, const HistoryItem* requested)
+{
+ if (requested == current)
+ return true;
+ if (requested)
+ return false;
+ while ((frame = frame->tree()->parent())) {
+ requested = frame->loader()->requestedHistoryItem();
+ if (!requested)
+ continue;
+ if (requested->isAncestorOf(current))
+ return true;
+ }
+ return false;
+}
+
void HistoryController::restoreDocumentState()
{
Document* doc = m_frame->document();
@@ -201,7 +217,7 @@
if (!itemToRestore)
return;
- if (m_frame->loader()->requestedHistoryItem() == m_currentItem.get() && !m_frame->loader()->documentLoader()->isClientRedirect()) {
+ if (isAssociatedToRequestedHistoryItem(itemToRestore, m_frame, m_frame->loader()->requestedHistoryItem()) && !m_frame->loader()->documentLoader()->isClientRedirect()) {
LOG(Loading, "WebCoreLoading %s: restoring form state from %p", m_frame->tree()->uniqueName().string().utf8().data(), itemToRestore);
doc->setStateForNewFormElements(itemToRestore->documentState());
}