Radio button group state is not restored correctly
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jan 2011 04:18:51 +0000 (04:18 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Jan 2011 04:18:51 +0000 (04:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=50442

Reviewed by Dimitri Glazkov.

Source/WebCore:

Fixes a bug that radio button states are not restored correctly in
a case that non-first radio button in a group is checked.

If "checked" attribute is present, the radio button is checked and
other radio buttons in the group are unchecked. This behavior
disturbs form state restoring. This patch changes this behavior so
that the "checked" attribute handling is delayed after form state
restoring.

Test: fast/forms/state-restore-radio-group.html

* html/HTMLFormControlElement.h:
 Make finishParsingChildren() protected so that HTMLInpuElement can call it.
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::HTMLInputElement):
 - Add createdByParser parameter.
 - Initialize m_stateRestored and m_parsingInProgress.
(WebCore::HTMLInputElement::create): Sync with the constructor.
(WebCore::HTMLInputElement::restoreFormControlState):
 Set m_stateRestored in order to refer it in finishParsingChildren().
(WebCore::HTMLInputElement::parseMappedAttribute):
 Don't call setChecked() during parsing. Move setNeedsValidityCheck()
 to setChecked().
(WebCore::HTMLInputElement::finishParsingChildren):
 Call setChecked() if form state is not restored.
(WebCore::HTMLInputElement::setChecked):
 Move setNeedsValidityCheck() from parseMappedAttribute() because
 finishParsingChildren() also needs to call setNeedsValidityCheck().
* html/HTMLInputElement.h:
 - Remove the default value of HTMLFormElement* of the HTMLInputElement
   constructor, and add createdByParser parameter.
 - Introduce m_parsingInProgress and m_stateRestored.
* html/HTMLIsIndexElement.cpp:
(WebCore::HTMLIsIndexElement::HTMLIsIndexElement):
 Sync with the HTMLInputElement constructor change.
* html/HTMLTagNames.in: Add constructorNeedsCreatedByParser flag.
* rendering/MediaControlElements.cpp:
(WebCore::MediaControlInputElement::MediaControlInputElement):
 Sync with the HTMLInputElement constructor change.
* rendering/ShadowElement.cpp:
(WebCore::ShadowInputElement::ShadowInputElement): ditto.
* rendering/ShadowElement.h:
(WebCore::ShadowElement::ShadowElement): ditto.

LayoutTests:

* fast/forms/state-restore-radio-group-expected.txt: Added.
* fast/forms/state-restore-radio-group.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@76664 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/forms/state-restore-radio-group-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/state-restore-radio-group.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLFormControlElement.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLInputElement.h
Source/WebCore/html/HTMLIsIndexElement.cpp
Source/WebCore/html/HTMLTagNames.in
Source/WebCore/rendering/MediaControlElements.cpp
Source/WebCore/rendering/ShadowElement.cpp
Source/WebCore/rendering/ShadowElement.h

index b5a9589..69ee0b0 100644 (file)
@@ -2,6 +2,16 @@
 
         Reviewed by Dimitri Glazkov.
 
+        Radio button group state is not restored correctly
+        https://bugs.webkit.org/show_bug.cgi?id=50442
+
+        * fast/forms/state-restore-radio-group-expected.txt: Added.
+        * fast/forms/state-restore-radio-group.html: Added.
+
+2011-01-25  Kent Tamura  <tkent@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
         HTMLFormElement::checkValidity() returns incorrect result if 'invalid' events are canceled.
         https://bugs.webkit.org/show_bug.cgi?id=52565
 
diff --git a/LayoutTests/fast/forms/state-restore-radio-group-expected.txt b/LayoutTests/fast/forms/state-restore-radio-group-expected.txt
new file mode 100644 (file)
index 0000000..4dda02a
--- /dev/null
@@ -0,0 +1,6 @@
+Test to restore form value states for a radio group.
+
+PASS document.getElementById("input1").checked is true
+PASS document.getElementById("input2").checked is false
+
diff --git a/LayoutTests/fast/forms/state-restore-radio-group.html b/LayoutTests/fast/forms/state-restore-radio-group.html
new file mode 100644 (file)
index 0000000..39715ec
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body onload="runTest()">
+<p>Test to restore form value states for a radio group.</p>
+<div id="console"></div>
+
+<input id=emptyOnFirstVisit>
+<div id=parent>
+<form action="data:text/html,<script>history.back()&lt;/script>" id=form1>
+<input name=user type=radio id=input1>
+<input checked name=user type=radio id=input2>
+</form>
+</div>
+
+<script>
+
+function runTest() {
+    var parent = document.getElementById('parent');
+    var state = document.getElementById('emptyOnFirstVisit');
+    if (!state.value) {
+        // First visit.
+        if (window.layoutTestController)
+            layoutTestController.waitUntilDone();
+        state.value = 'visited';
+
+        document.getElementById('input1').checked = true;
+        // 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.
+        shouldBeTrue('document.getElementById("input1").checked');
+        shouldBeFalse('document.getElementById("input2").checked');
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+}
+</script>
+</body>
index ee6906e..3a07b49 100644 (file)
@@ -2,6 +2,57 @@
 
         Reviewed by Dimitri Glazkov.
 
+        Radio button group state is not restored correctly
+        https://bugs.webkit.org/show_bug.cgi?id=50442
+
+        Fixes a bug that radio button states are not restored correctly in
+        a case that non-first radio button in a group is checked.
+
+        If "checked" attribute is present, the radio button is checked and
+        other radio buttons in the group are unchecked. This behavior
+        disturbs form state restoring. This patch changes this behavior so
+        that the "checked" attribute handling is delayed after form state
+        restoring.
+
+        Test: fast/forms/state-restore-radio-group.html
+
+        * html/HTMLFormControlElement.h:
+         Make finishParsingChildren() protected so that HTMLInpuElement can call it.
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::HTMLInputElement):
+         - Add createdByParser parameter.
+         - Initialize m_stateRestored and m_parsingInProgress.
+        (WebCore::HTMLInputElement::create): Sync with the constructor.
+        (WebCore::HTMLInputElement::restoreFormControlState):
+         Set m_stateRestored in order to refer it in finishParsingChildren().
+        (WebCore::HTMLInputElement::parseMappedAttribute):
+         Don't call setChecked() during parsing. Move setNeedsValidityCheck()
+         to setChecked().
+        (WebCore::HTMLInputElement::finishParsingChildren):
+         Call setChecked() if form state is not restored.
+        (WebCore::HTMLInputElement::setChecked):
+         Move setNeedsValidityCheck() from parseMappedAttribute() because
+         finishParsingChildren() also needs to call setNeedsValidityCheck().
+        * html/HTMLInputElement.h:
+         - Remove the default value of HTMLFormElement* of the HTMLInputElement
+           constructor, and add createdByParser parameter.
+         - Introduce m_parsingInProgress and m_stateRestored.
+        * html/HTMLIsIndexElement.cpp:
+        (WebCore::HTMLIsIndexElement::HTMLIsIndexElement):
+         Sync with the HTMLInputElement constructor change.
+        * html/HTMLTagNames.in: Add constructorNeedsCreatedByParser flag.
+        * rendering/MediaControlElements.cpp:
+        (WebCore::MediaControlInputElement::MediaControlInputElement):
+         Sync with the HTMLInputElement constructor change.
+        * rendering/ShadowElement.cpp:
+        (WebCore::ShadowInputElement::ShadowInputElement): ditto.
+        * rendering/ShadowElement.h:
+        (WebCore::ShadowElement::ShadowElement): ditto.
+
+2011-01-25  Kent Tamura  <tkent@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
         HTMLFormElement::checkValidity() returns incorrect result if 'invalid' events are canceled.
         https://bugs.webkit.org/show_bug.cgi?id=52565
 
index c88905c..e0be3f0 100644 (file)
@@ -170,14 +170,13 @@ protected:
     HTMLFormControlElementWithState(const QualifiedName& tagName, Document*, HTMLFormElement*);
 
     virtual bool autoComplete() const;
-
+    virtual void finishParsingChildren();
     virtual void willMoveToNewOwnerDocument();
     virtual void didMoveToNewOwnerDocument();
     virtual void defaultEventHandler(Event*);
 
 private:
     virtual bool shouldSaveAndRestoreFormControlState() const;
-    virtual void finishParsingChildren();
 };
 
 // FIXME: Give this class its own header file.
index 3e51e26..7b12767 100644 (file)
@@ -62,7 +62,7 @@ using namespace HTMLNames;
 
 const int maxSavedResults = 256;
 
-HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
+HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document* document, HTMLFormElement* form, bool createdByParser)
     : HTMLTextFormControlElement(tagName, document, form)
     , m_maxResults(-1)
     , m_isChecked(false)
@@ -72,14 +72,16 @@ HTMLInputElement::HTMLInputElement(const QualifiedName& tagName, Document* docum
     , m_isActivatedSubmit(false)
     , m_autocomplete(Uninitialized)
     , m_isAutofilled(false)
+    , m_stateRestored(false)
+    , m_parsingInProgress(createdByParser)
     , m_inputType(InputType::createText(this))
 {
     ASSERT(hasTagName(inputTag) || hasTagName(isindexTag));
 }
 
-PassRefPtr<HTMLInputElement> HTMLInputElement::create(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
+PassRefPtr<HTMLInputElement> HTMLInputElement::create(const QualifiedName& tagName, Document* document, HTMLFormElement* form, bool createdByParser)
 {
-    return adoptRef(new HTMLInputElement(tagName, document, form));
+    return adoptRef(new HTMLInputElement(tagName, document, form, createdByParser));
 }
 
 HTMLInputElement::~HTMLInputElement()
@@ -506,6 +508,7 @@ bool HTMLInputElement::saveFormControlState(String& result) const
 void HTMLInputElement::restoreFormControlState(const String& state)
 {
     m_inputType->restoreFormControlState(state);
+    m_stateRestored = true;
 }
 
 bool HTMLInputElement::canStartSelection() const
@@ -574,11 +577,14 @@ void HTMLInputElement::parseMappedAttribute(Attribute* attr)
         setFormControlValueMatchesRenderer(false);
         setNeedsValidityCheck();
     } else if (attr->name() == checkedAttr) {
-        if (m_reflectsCheckedAttribute) {
+        // Another radio button in the same group might be checked by state
+        // restore. We shouldn't call setChecked() even if this has the checked
+        // attribute. So, delay the setChecked() call until
+        // finishParsingChildren() is called if parsing is in progress.
+        if (!m_parsingInProgress && m_reflectsCheckedAttribute) {
             setChecked(!attr->isNull());
             m_reflectsCheckedAttribute = true;
         }
-        setNeedsValidityCheck();
     } else if (attr->name() == maxlengthAttr) {
         InputElement::parseMaxLengthAttribute(m_data, this, this, attr);
         setNeedsValidityCheck();
@@ -647,6 +653,18 @@ void HTMLInputElement::parseMappedAttribute(Attribute* attr)
         HTMLTextFormControlElement::parseMappedAttribute(attr);
 }
 
+void HTMLInputElement::finishParsingChildren()
+{
+    m_parsingInProgress = false;
+    HTMLFormControlElementWithState::finishParsingChildren();
+    if (!m_stateRestored) {
+        bool checked = hasAttribute(checkedAttr);
+        if (checked)
+            setChecked(checked);
+        m_reflectsCheckedAttribute = true;
+    }
+}
+
 bool HTMLInputElement::rendererIsNeeded(RenderStyle* style)
 {
     return m_inputType->rendererIsNeeded() && HTMLFormControlElementWithState::rendererIsNeeded(style);
@@ -745,6 +763,7 @@ void HTMLInputElement::setChecked(bool nowChecked, bool sendChangeEvent)
     setNeedsStyleRecalc();
 
     updateCheckedRadioButtons();
+    setNeedsValidityCheck();
 
     // Ideally we'd do this from the render tree (matching
     // RenderTextView), but it's not possible to do it at the moment
index de9114a..757992a 100644 (file)
@@ -37,7 +37,7 @@ class KURL;
 
 class HTMLInputElement : public HTMLTextFormControlElement, public InputElement {
 public:
-    static PassRefPtr<HTMLInputElement> create(const QualifiedName&, Document*, HTMLFormElement*);
+    static PassRefPtr<HTMLInputElement> create(const QualifiedName&, Document*, HTMLFormElement*, bool createdByParser);
     virtual ~HTMLInputElement();
 
     DEFINE_ATTRIBUTE_EVENT_LISTENER(webkitspeechchange);
@@ -198,7 +198,7 @@ public:
     void updateCheckedRadioButtons();
     
 protected:
-    HTMLInputElement(const QualifiedName&, Document*, HTMLFormElement* = 0);
+    HTMLInputElement(const QualifiedName&, Document*, HTMLFormElement*, bool createdByParser);
 
     virtual void defaultEventHandler(Event*);
 
@@ -240,6 +240,7 @@ private:
 
     virtual bool mapToEntry(const QualifiedName& attrName, MappedAttributeEntry& result) const;
     virtual void parseMappedAttribute(Attribute*);
+    virtual void finishParsingChildren();
 
     virtual void copyNonAttributeProperties(const Element* source);
 
@@ -313,6 +314,8 @@ private:
 #if ENABLE(DATALIST)
     bool m_hasNonEmptyList : 1;
 #endif
+    bool m_stateRestored : 1;
+    bool m_parsingInProgress : 1;
     OwnPtr<InputType> m_inputType;
 };
 
index a23a353..f865790 100644 (file)
@@ -33,7 +33,7 @@ namespace WebCore {
 using namespace HTMLNames;
 
 HTMLIsIndexElement::HTMLIsIndexElement(const QualifiedName& tagName, Document* document, HTMLFormElement* form)
-    : HTMLInputElement(tagName, document, form)
+    : HTMLInputElement(tagName, document, form, false)
 {
     ASSERT(hasTagName(isindexTag));
     setDefaultName(isindexTag.localName());
index f5ff077..83d41de 100644 (file)
@@ -67,7 +67,7 @@ i interfaceName=HTMLElement
 iframe interfaceName=HTMLIFrameElement
 image mapToTagName=img
 img interfaceName=HTMLImageElement, constructorNeedsFormElement
-input constructorNeedsFormElement
+input constructorNeedsFormElement, constructorNeedsCreatedByParser
 ins interfaceName=HTMLModElement
 isindex interfaceName=HTMLIsIndexElement, constructorNeedsFormElement
 kbd interfaceName=HTMLElement
index e27181e..113ca3b 100644 (file)
@@ -353,7 +353,7 @@ bool MediaControlStatusDisplayElement::rendererIsNeeded(RenderStyle* style)
 // ----------------------------
     
 MediaControlInputElement::MediaControlInputElement(HTMLMediaElement* mediaElement, PseudoId pseudo)
-    : HTMLInputElement(inputTag, mediaElement->document())
+    : HTMLInputElement(inputTag, mediaElement->document(), 0, false)
     , m_mediaElement(mediaElement)
     , m_pseudoStyleId(pseudo)
 {
index e1b247c..5b1a962 100644 (file)
@@ -114,7 +114,7 @@ PassRefPtr<ShadowInputElement> ShadowInputElement::create(HTMLElement* shadowPar
 }
 
 ShadowInputElement::ShadowInputElement(HTMLElement* shadowParent)
-    : ShadowElement<HTMLInputElement>(inputTag, shadowParent)
+    : ShadowElement<HTMLInputElement>(inputTag, shadowParent, 0, false)
 {
 }
 
index 8bcb34e..9a5d118 100644 (file)
@@ -43,6 +43,12 @@ protected:
         BaseElement::setShadowHost(shadowParent);
     }
 
+    ShadowElement(const QualifiedName& name, HTMLElement* shadowParent, HTMLFormElement* form, bool createdByParser)
+        : BaseElement(name, shadowParent->document(), form, createdByParser)
+    {
+        BaseElement::setShadowHost(shadowParent);
+    }
+
 public:
     virtual void detach();
 };