- Revision
- 120895
- Author
- [email protected]
- Date
- 2012-06-20 18:31:08 -0700 (Wed, 20 Jun 2012)
Log Message
Unmodified form control value are overwritten by another form
control value with the same name after navigating and going back
https://bugs.webkit.org/show_bug.cgi?id=89409
Reviewed by Hajime Morita.
Source/WebCore:
Detail of the bug:
If a page had multiple form controls of which names and types were
identical like the following:
<input type=text name=name1 id=input1>
<input type=text name=name1 id=input2>
and a user updated the value of the second control, then went to
another page and went back to the page again, we restored the updated
value to the first element, and didn't update the second element.
We didn't save unmodified control state, and the form state data
had no ways to represent "this control should be skipped".
How to resovle the bug:
We need to represent "this control should be skipped" in the
seriazlied form state vector.
- A serialized control state had three items:
name, type, value.
Now we change it to:
name, type, flag, optional value
- It is definitely incompatible with serizlized state produced by
older WebCore. So, we need to add the signature string to
represent the version of serialized state format.
- Because the state for a form control is variable-length and we
can't deserialize it in reverse-order, we change the on-memory
representation from Vector<> to Deque<>.
Test: fast/forms/state-restore-to-non-edited-controls.html
* html/FormController.cpp:
(WebCore::FormControlState::serializeTo):
Added. Serialize a state for a form control to a string vector.
(WebCore::FormControlState::deserialize):
Added. Produce a FormControlState object from the specified string vector.
It can produce a FromControlState of the failure type.
(WebCore::formStateSignature): The signature string of the serialized state.
(WebCore::FormController::formElementsState):
- Capacity:
The size of seirlized data for one form control is typically 4.
+1 for the signature.
- We need to store a FormControlState with no values.
(WebCore::FormController::setStateForNewFormElements):
- We can't iterate over the stateVector in reverse order any more
because serialized control state is variable-length.
- We put FormControlState objects to HashMap instead of String objects.
(WebCore::FormController::takeStateForFormElement):
Updated for Deque<>.
* html/FormController.h:
(FormControlState): Declare deserialize() and serializeTo().
(WebCore::FormControlState::isFailure): Added.
(WebCore::FormControlState::FormControlState):
Added to create a FormControlState with failure type.
(FormController):
Change the value type of m_stateForNewFormElements from Vector<String>
to Deque<FormControlState>.
LayoutTests:
* fast/forms/resources/state-restore-broken-state-2.html:
Take care of the signature string at the beginning of the array.
* fast/forms/state-restore-broken-state-expected.txt:
Updated for the serialized format change.
* fast/forms/state-restore-to-non-edited-controls-expected.txt:
* fast/forms/state-restore-to-non-edited-controls.html:
Add a test for a case of duplicated names.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (120894 => 120895)
--- trunk/LayoutTests/ChangeLog 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/LayoutTests/ChangeLog 2012-06-21 01:31:08 UTC (rev 120895)
@@ -1,3 +1,19 @@
+2012-06-20 Kent Tamura <[email protected]>
+
+ Unmodified form control value are overwritten by another form
+ control value with the same name after navigating and going back
+ https://bugs.webkit.org/show_bug.cgi?id=89409
+
+ Reviewed by Hajime Morita.
+
+ * fast/forms/resources/state-restore-broken-state-2.html:
+ Take care of the signature string at the beginning of the array.
+ * fast/forms/state-restore-broken-state-expected.txt:
+ Updated for the serialized format change.
+ * fast/forms/state-restore-to-non-edited-controls-expected.txt:
+ * fast/forms/state-restore-to-non-edited-controls.html:
+ Add a test for a case of duplicated names.
+
2012-06-20 Adam Barth <[email protected]>
Unskip fast/hidpi/device-scale-factor-paint.html
Modified: trunk/LayoutTests/fast/forms/resources/state-restore-broken-state-2.html (120894 => 120895)
--- trunk/LayoutTests/fast/forms/resources/state-restore-broken-state-2.html 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/LayoutTests/fast/forms/resources/state-restore-broken-state-2.html 2012-06-21 01:31:08 UTC (rev 120895)
@@ -1,8 +1,11 @@
<script>
setTimeout(function() {
- console.log('Generated state: [' + internals.formControlStateOfPreviousHistoryItem() + ']');
+ var stateArray = internals.formControlStateOfPreviousHistoryItem();
+ // Omit the signature string because it is volatile.
+ console.log('Generated state: [' + stateArray.slice(1) + ']');
- internals.setFormControlStateOfPreviousHistoryItem(['name1', 'text', 'modified', '***broken***']);
+ stateArray.push('***broken***');
+ internals.setFormControlStateOfPreviousHistoryItem(stateArray);
parent.didLoadAnotherDocument();
}, 0);
</script>
Modified: trunk/LayoutTests/fast/forms/state-restore-broken-state-expected.txt (120894 => 120895)
--- trunk/LayoutTests/fast/forms/state-restore-broken-state-expected.txt 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/LayoutTests/fast/forms/state-restore-broken-state-expected.txt 2012-06-21 01:31:08 UTC (rev 120895)
@@ -1,4 +1,4 @@
-CONSOLE MESSAGE: line 3: Generated state: [name1,text,modified]
+CONSOLE MESSAGE: line 5: Generated state: [name1,text,1,modified]
The value was modified in the first load of state-restore-broken-state-1.html, but it should not be restored because the state-restore-broken-state-2.html breaks the state.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
Modified: trunk/LayoutTests/fast/forms/state-restore-to-non-edited-controls-expected.txt (120894 => 120895)
--- trunk/LayoutTests/fast/forms/state-restore-to-non-edited-controls-expected.txt 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/LayoutTests/fast/forms/state-restore-to-non-edited-controls-expected.txt 2012-06-21 01:31:08 UTC (rev 120895)
@@ -5,6 +5,7 @@
PASS document.getElementById("image").value is "2"
PASS document.getElementById("reset").value is "2"
PASS document.getElementById("submit1").value is "2"
+PASS document.getElementById("text0").value is "2"
PASS document.getElementById("text1").value is "edit"
PASS document.getElementById("text2").value is "2"
PASS document.getElementById("textarea").value is "2"
Modified: trunk/LayoutTests/fast/forms/state-restore-to-non-edited-controls.html (120894 => 120895)
--- trunk/LayoutTests/fast/forms/state-restore-to-non-edited-controls.html 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/LayoutTests/fast/forms/state-restore-to-non-edited-controls.html 2012-06-21 01:31:08 UTC (rev 120895)
@@ -13,13 +13,14 @@
<script>
-function makeForm(parent, buttonValue, hiddenValue, imageValue, resetValue, submitValue, textValue1, textValue2, textAreaValue) {
+function makeForm(parent, buttonValue, hiddenValue, imageValue, resetValue, submitValue, textValue0, textValue1, textValue2, textAreaValue) {
document.write('<form action="" id=form1>'
+ '<input name=button id=button type=button value="' + buttonValue + '">'
+ '<input name=hidden id=hidden type=hidden value="' + hiddenValue + '">'
+ '<input name=image id=image type=image value="' + imageValue + '">'
+ '<input name=reset id=reset type=reset value="' + resetValue + '">'
+ '<input name=submit1 id=submit1 type=submit value="' + submitValue + '">'
+ + '<input name=text1 id=text0 type=text value="' + textValue0 + '">'
+ '<input name=text1 id=text1 type=text value="' + textValue1 + '">'
+ '<input name=text2 id=text2 type=text value="' + textValue2 + '">'
+ '<textarea name=textarea id=textarea>' + textAreaValue + '</textarea>'
@@ -35,20 +36,21 @@
if (window.layoutTestController)
layoutTestController.waitUntilDone();
state.value = 'visited';
- makeForm(parent, '1', '1', '1', '1', '1', '1', '1', '1');
+ makeForm(parent, '1', '1', '1', '1', '1', '1', '1', '1', '1');
document.getElementById('text1').value = 'edit';
// Submit form in a timeout to make sure that we create a new back/forward list item.
setTimeout(function() {document.getElementById('form1').submit();}, 0);
} else {
// Second visit.
- makeForm(parent, '2', '2', '2', '2', '2', '2', '2', '2');
+ makeForm(parent, '2', '2', '2', '2', '2', '2', '2', '2', '2');
shouldBe('document.getElementById("button").value', '"2"');
shouldBe('document.getElementById("hidden").value', '"2"');
shouldBe('document.getElementById("image").value', '"2"');
shouldBe('document.getElementById("reset").value', '"2"');
shouldBe('document.getElementById("submit1").value', '"2"');
+ shouldBe('document.getElementById("text0").value', '"2"');
shouldBe('document.getElementById("text1").value', '"edit"');
shouldBe('document.getElementById("text2").value', '"2"');
shouldBe('document.getElementById("textarea").value', '"2"');
Modified: trunk/Source/WebCore/ChangeLog (120894 => 120895)
--- trunk/Source/WebCore/ChangeLog 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/Source/WebCore/ChangeLog 2012-06-21 01:31:08 UTC (rev 120895)
@@ -1,3 +1,69 @@
+2012-06-20 Kent Tamura <[email protected]>
+
+ Unmodified form control value are overwritten by another form
+ control value with the same name after navigating and going back
+ https://bugs.webkit.org/show_bug.cgi?id=89409
+
+ Reviewed by Hajime Morita.
+
+ Detail of the bug:
+ If a page had multiple form controls of which names and types were
+ identical like the following:
+ <input type=text name=name1 id=input1>
+ <input type=text name=name1 id=input2>
+ and a user updated the value of the second control, then went to
+ another page and went back to the page again, we restored the updated
+ value to the first element, and didn't update the second element.
+
+ We didn't save unmodified control state, and the form state data
+ had no ways to represent "this control should be skipped".
+
+ How to resovle the bug:
+ We need to represent "this control should be skipped" in the
+ seriazlied form state vector.
+
+ - A serialized control state had three items:
+ name, type, value.
+ Now we change it to:
+ name, type, flag, optional value
+
+ - It is definitely incompatible with serizlized state produced by
+ older WebCore. So, we need to add the signature string to
+ represent the version of serialized state format.
+
+ - Because the state for a form control is variable-length and we
+ can't deserialize it in reverse-order, we change the on-memory
+ representation from Vector<> to Deque<>.
+
+ Test: fast/forms/state-restore-to-non-edited-controls.html
+
+ * html/FormController.cpp:
+ (WebCore::FormControlState::serializeTo):
+ Added. Serialize a state for a form control to a string vector.
+ (WebCore::FormControlState::deserialize):
+ Added. Produce a FormControlState object from the specified string vector.
+ It can produce a FromControlState of the failure type.
+ (WebCore::formStateSignature): The signature string of the serialized state.
+ (WebCore::FormController::formElementsState):
+ - Capacity:
+ The size of seirlized data for one form control is typically 4.
+ +1 for the signature.
+ - We need to store a FormControlState with no values.
+ (WebCore::FormController::setStateForNewFormElements):
+ - We can't iterate over the stateVector in reverse order any more
+ because serialized control state is variable-length.
+ - We put FormControlState objects to HashMap instead of String objects.
+ (WebCore::FormController::takeStateForFormElement):
+ Updated for Deque<>.
+ * html/FormController.h:
+ (FormControlState): Declare deserialize() and serializeTo().
+ (WebCore::FormControlState::isFailure): Added.
+ (WebCore::FormControlState::FormControlState):
+ Added to create a FormControlState with failure type.
+ (FormController):
+ Change the value type of m_stateForNewFormElements from Vector<String>
+ to Deque<FormControlState>.
+
2012-06-20 Alexandru Chiculita <[email protected]>
[CSS Shaders] Rename CustomFilterShader class name to CustomFilterCompiledProgram
Modified: trunk/Source/WebCore/html/FormController.cpp (120894 => 120895)
--- trunk/Source/WebCore/html/FormController.cpp 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/Source/WebCore/html/FormController.cpp 2012-06-21 01:31:08 UTC (rev 120895)
@@ -27,6 +27,42 @@
using namespace HTMLNames;
+// ----------------------------------------------------------------------------
+
+// Serilized form of FormControlState:
+// (',' means strings around it are separated in stateVector.)
+//
+// SerializedControlState ::= SkipState | SingleValueState
+// SkipState ::= '0'
+// SingleValueState ::= '1', ControlValue
+// ControlValue ::= arbitrary string
+
+void FormControlState::serializeTo(Vector<String>& stateVector) const
+{
+ ASSERT(!isFailure());
+ if (!hasValue())
+ stateVector.append("0");
+ else {
+ stateVector.append("1");
+ stateVector.append(m_value);
+ }
+}
+
+FormControlState FormControlState::deserialize(const Vector<String>& stateVector, size_t& index)
+{
+ if (index >= stateVector.size())
+ return FormControlState(TypeFailure);
+ uint64_t valueSize = stateVector[index++].toUInt64();
+ if (!valueSize)
+ return FormControlState();
+ if (valueSize != 1 || index + 1 > stateVector.size())
+ return FormControlState(TypeFailure);
+ return FormControlState(stateVector[index++]);
+}
+
+// ----------------------------------------------------------------------------
+
+
FormController::FormController()
{
}
@@ -35,22 +71,29 @@
{
}
+static String formStateSignature()
+{
+ // In the legacy version of serialized state, the first item was a name
+ // attribute value of a form control. The following string literal should
+ // contain some characters which are rarely used for name attribute values.
+ DEFINE_STATIC_LOCAL(String, signature, ("\n\r?% WebKit serialized form state version 1 \n\r=&"));
+ return signature;
+}
+
Vector<String> FormController::formElementsState() const
{
Vector<String> stateVector;
- stateVector.reserveInitialCapacity(m_formElementsWithState.size() * 3);
+ stateVector.reserveInitialCapacity(m_formElementsWithState.size() * 4 + 1);
+ stateVector.append(formStateSignature());
typedef FormElementListHashSet::const_iterator Iterator;
Iterator end = m_formElementsWithState.end();
for (Iterator it = m_formElementsWithState.begin(); it != end; ++it) {
HTMLFormControlElementWithState* elementWithState = *it;
if (!elementWithState->shouldSaveAndRestoreFormControlState())
continue;
- FormControlState state = elementWithState->saveFormControlState();
- if (!state.hasValue())
- continue;
stateVector.append(elementWithState->name().string());
stateVector.append(elementWithState->formControlType().string());
- stateVector.append(state.value());
+ elementWithState->saveFormControlState().serializeTo(stateVector);
}
return stateVector;
}
@@ -62,33 +105,32 @@
void FormController::setStateForNewFormElements(const Vector<String>& stateVector)
{
- // Walk the state vector backwards so that the value to use for each
- // name/type pair first is the one at the end of each individual vector
- // in the FormElementStateMap. We're using them like stacks.
typedef FormElementStateMap::iterator Iterator;
m_formElementsWithState.clear();
- if (stateVector.size() % 3)
+ size_t i = 0;
+ if (stateVector.size() < 1 || stateVector[i++] != formStateSignature())
return;
- for (size_t i = 0; i < stateVector.size(); i += 3) {
- if (stateVector[i + 1].find(isNotFormControlTypeCharacter) != notFound)
- return;
- }
- for (size_t i = stateVector.size() / 3 * 3; i; i -= 3) {
- AtomicString name = stateVector[i - 3];
- AtomicString type = stateVector[i - 2];
- const String& value = stateVector[i - 1];
+ while (i + 2 < stateVector.size()) {
+ AtomicString name = stateVector[i++];
+ AtomicString type = stateVector[i++];
+ FormControlState state = FormControlState::deserialize(stateVector, i);
+ if (type.isEmpty() || type.impl()->find(isNotFormControlTypeCharacter) != notFound || state.isFailure())
+ break;
+
FormElementKey key(name.impl(), type.impl());
Iterator it = m_stateForNewFormElements.find(key);
if (it != m_stateForNewFormElements.end())
- it->second.append(value);
+ it->second.append(state);
else {
- Vector<String> valueList(1);
- valueList[0] = value;
- m_stateForNewFormElements.set(key, valueList);
+ Deque<FormControlState> stateList;
+ stateList.append(state);
+ m_stateForNewFormElements.set(key, stateList);
}
}
+ if (i != stateVector.size())
+ m_stateForNewFormElements.clear();
}
bool FormController::hasStateForNewFormElements() const
@@ -103,10 +145,8 @@
if (it == m_stateForNewFormElements.end())
return FormControlState();
ASSERT(it->second.size());
- FormControlState state(it->second.last());
- if (it->second.size() > 1)
- it->second.removeLast();
- else
+ FormControlState state = it->second.takeFirst();
+ if (!it->second.size())
m_stateForNewFormElements.remove(it);
return state;
}
Modified: trunk/Source/WebCore/html/FormController.h (120894 => 120895)
--- trunk/Source/WebCore/html/FormController.h 2012-06-21 01:29:36 UTC (rev 120894)
+++ trunk/Source/WebCore/html/FormController.h 2012-06-21 01:31:08 UTC (rev 120895)
@@ -23,6 +23,7 @@
#define FormController_h
#include "CheckedRadioButtons.h"
+#include <wtf/Deque.h>
#include <wtf/Forward.h>
#include <wtf/ListHashSet.h>
#include <wtf/Vector.h>
@@ -77,14 +78,19 @@
public:
FormControlState() : m_type(TypeSkip) { }
explicit FormControlState(const String& value) : m_type(TypeRestore), m_value(value) { }
+ static FormControlState deserialize(const Vector<String>& stateVector, size_t& index);
FormControlState(const FormControlState& another) : m_type(another.m_type), m_value(another.m_value) { }
FormControlState& operator=(const FormControlState&);
+ bool isFailure() const { return m_type == TypeFailure; }
bool hasValue() const { return m_type == TypeRestore; }
String value() const { return m_value; }
+ void serializeTo(Vector<String>& stateVector) const;
private:
- enum Type { TypeSkip, TypeRestore };
+ enum Type { TypeSkip, TypeRestore, TypeFailure };
+ explicit FormControlState(Type type) : m_type(type) { }
+
Type m_type;
String m_value;
};
@@ -129,7 +135,7 @@
typedef ListHashSet<RefPtr<FormAssociatedElement>, 32> FormAssociatedElementListHashSet;
FormAssociatedElementListHashSet m_formElementsWithFormAttribute;
- typedef HashMap<FormElementKey, Vector<String>, FormElementKeyHash, FormElementKeyHashTraits> FormElementStateMap;
+ typedef HashMap<FormElementKey, Deque<FormControlState>, FormElementKeyHash, FormElementKeyHashTraits> FormElementStateMap;
FormElementStateMap m_stateForNewFormElements;
};
