Checking a radio button doesn't uncheck other buttons in the same group in some cases.
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jun 2012 21:04:05 +0000 (21:04 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Jun 2012 21:04:05 +0000 (21:04 +0000)
https://bugs.webkit.org/show_bug.cgi?id=88835

Reviewed by Ryosuke Niwa.

Source/WebCore:

This change fixes a bug that checking a radio button in a radio button
group in a form detached from a document tree doesn't uncheck another
checked radio button in the radio button group.

A radio button participates in a radio button group in the following
conditions:
- If it is owned by a form element regardless of the form is in a
document tree or not, or

- If it is not owned by any form elements and it is in a document tree.
A radio button group for the radio button is owned by the document.

For HTMLInputElement::removedFrom():
The old code always unregistered the radio button if it was removed from
the document tree. It was incorrect because we don't need to unregister
it if it has an owner form and the owner form is not changed by
removedFrom().
If the owner form is cleared by removedFrom(), willChangeForm()
unregisters the radio button. So what we should do in removedFrom() is
to unregister the radio button only if the radio button group is owned
by the document.

For HTMLInputElement::insertedInto():
The old code always registered the radio button if it is inserted into
the document tree. It was incorrect because we don't need to register it
if it has an owner form and the owner form is not changed by
insertedInto().
If the owner form is changed by insertedInto(), didChangeForm()
registers the radio button. So We should register the radio button only
if its radio button group will be owned by the document.

Test: Add test cases to fast/forms/radio/radio-group.html

* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::insertedInto):
Register this to CheckedRadioButtons only if new group owner is Document.
(WebCore::HTMLInputElement::removedFrom):
Unregister this from CheckedRadioButtons only if old group owner was Document.

LayoutTests:

* fast/forms/radio/radio-group-expected.txt:
* fast/forms/radio/radio-group.html: Add test cases.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/radio/radio-group-expected.txt
LayoutTests/fast/forms/radio/radio-group.html
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLInputElement.cpp

index 152ec84..bad4f5f 100644 (file)
@@ -1,3 +1,13 @@
+2012-06-12  Kent Tamura  <tkent@chromium.org>
+
+        Checking a radio button doesn't uncheck other buttons in the same group in some cases.
+        https://bugs.webkit.org/show_bug.cgi?id=88835
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/forms/radio/radio-group-expected.txt:
+        * fast/forms/radio/radio-group.html: Add test cases.
+
 2012-06-12  Alec Flett  <alecflett@chromium.org>
 
         IndexedDB: Error codes, phase two
index ba70895..6a6b9c5 100644 (file)
@@ -8,6 +8,14 @@ PASS $("radio1-1").checked is true
 PASS $("radio1-2").checked is true
 PASS $("radio1-1").checked is false
 PASS $("radio1-2").checked is true
+PASS $("radio1-1").checked = true; $("radio1-1").checked is true
+PASS $("radio1-2").checked is false
+
+Detach the from from the document tree:
+PASS radioButtons[0].checked is true
+PASS radioButtons[1].checked is false
+PASS radioButtons[1].checked = true; radioButtons[0].checked is false
+PASS radioButtons[1].checked is true
 
 Changing the type of an input element to radio:
 PASS $("radio1-1").checked is true
index ce7fab9..cdbdb5f 100644 (file)
@@ -35,6 +35,17 @@ shouldBeTrue('$("radio1-2").checked');
 $('radio1-2').name = 'name1';
 shouldBeFalse('$("radio1-1").checked');
 shouldBeTrue('$("radio1-2").checked');
+shouldBeTrue('$("radio1-1").checked = true; $("radio1-1").checked');
+shouldBeFalse('$("radio1-2").checked');
+
+debug('\nDetach the from from the document tree:');
+var orphanForm = parent.firstChild;
+parent.removeChild(orphanForm);
+var radioButtons = orphanForm.getElementsByTagName('input');
+shouldBeTrue('radioButtons[0].checked');
+shouldBeFalse('radioButtons[1].checked');
+shouldBeFalse('radioButtons[1].checked = true; radioButtons[0].checked');
+shouldBeTrue('radioButtons[1].checked');
 
 debug('');
 debug('Changing the type of an input element to radio:');
index 286f1ec..d75d459 100644 (file)
@@ -1,3 +1,49 @@
+2012-06-12  Kent Tamura  <tkent@chromium.org>
+
+        Checking a radio button doesn't uncheck other buttons in the same group in some cases.
+        https://bugs.webkit.org/show_bug.cgi?id=88835
+
+        Reviewed by Ryosuke Niwa.
+
+        This change fixes a bug that checking a radio button in a radio button
+        group in a form detached from a document tree doesn't uncheck another
+        checked radio button in the radio button group.
+
+        A radio button participates in a radio button group in the following
+        conditions:
+        - If it is owned by a form element regardless of the form is in a
+        document tree or not, or
+
+        - If it is not owned by any form elements and it is in a document tree.
+        A radio button group for the radio button is owned by the document.
+
+        For HTMLInputElement::removedFrom():
+        The old code always unregistered the radio button if it was removed from
+        the document tree. It was incorrect because we don't need to unregister
+        it if it has an owner form and the owner form is not changed by
+        removedFrom().
+        If the owner form is cleared by removedFrom(), willChangeForm()
+        unregisters the radio button. So what we should do in removedFrom() is
+        to unregister the radio button only if the radio button group is owned
+        by the document.
+
+        For HTMLInputElement::insertedInto():
+        The old code always registered the radio button if it is inserted into
+        the document tree. It was incorrect because we don't need to register it
+        if it has an owner form and the owner form is not changed by
+        insertedInto().
+        If the owner form is changed by insertedInto(), didChangeForm()
+        registers the radio button. So We should register the radio button only
+        if its radio button group will be owned by the document.
+
+        Test: Add test cases to fast/forms/radio/radio-group.html
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::insertedInto):
+        Register this to CheckedRadioButtons only if new group owner is Document.
+        (WebCore::HTMLInputElement::removedFrom):
+        Unregister this from CheckedRadioButtons only if old group owner was Document.
+
 2012-06-12  James Robinson  <jamesr@chromium.org>
 
         [chromium] REGRESSION(119769): Canvas2DLayerBridge may go away before its TextureLayerChromium
index 333bbdf..3c955c5 100644 (file)
@@ -1352,16 +1352,14 @@ void HTMLInputElement::didChangeForm()
 Node::InsertionNotificationRequest HTMLInputElement::insertedInto(ContainerNode* insertionPoint)
 {
     HTMLTextFormControlElement::insertedInto(insertionPoint);
-    if (!insertionPoint->inDocument())
-        return InsertionDone;
-    ASSERT(inDocument());
-    addToRadioButtonGroup();
+    if (insertionPoint->inDocument() && !form())
+        addToRadioButtonGroup();
     return InsertionDone;
 }
 
 void HTMLInputElement::removedFrom(ContainerNode* insertionPoint)
 {
-    if (insertionPoint->inDocument())
+    if (insertionPoint->inDocument() && !form())
         removeFromRadioButtonGroup();
     HTMLTextFormControlElement::removedFrom(insertionPoint);
 }