Title: [120118] trunk
Revision
120118
Author
[email protected]
Date
2012-06-12 14:04:05 -0700 (Tue, 12 Jun 2012)

Log Message

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.

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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (120117 => 120118)


--- trunk/LayoutTests/ChangeLog	2012-06-12 20:54:41 UTC (rev 120117)
+++ trunk/LayoutTests/ChangeLog	2012-06-12 21:04:05 UTC (rev 120118)
@@ -1,3 +1,13 @@
+2012-06-12  Kent Tamura  <[email protected]>
+
+        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  <[email protected]>
 
         IndexedDB: Error codes, phase two

Modified: trunk/LayoutTests/fast/forms/radio/radio-group-expected.txt (120117 => 120118)


--- trunk/LayoutTests/fast/forms/radio/radio-group-expected.txt	2012-06-12 20:54:41 UTC (rev 120117)
+++ trunk/LayoutTests/fast/forms/radio/radio-group-expected.txt	2012-06-12 21:04:05 UTC (rev 120118)
@@ -8,7 +8,15 @@
 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
 PASS $("radio1-1").checked is false

Modified: trunk/LayoutTests/fast/forms/radio/radio-group.html (120117 => 120118)


--- trunk/LayoutTests/fast/forms/radio/radio-group.html	2012-06-12 20:54:41 UTC (rev 120117)
+++ trunk/LayoutTests/fast/forms/radio/radio-group.html	2012-06-12 21:04:05 UTC (rev 120118)
@@ -35,7 +35,18 @@
 $('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:');
 parent.innerHTML = '<form>' +

Modified: trunk/Source/WebCore/ChangeLog (120117 => 120118)


--- trunk/Source/WebCore/ChangeLog	2012-06-12 20:54:41 UTC (rev 120117)
+++ trunk/Source/WebCore/ChangeLog	2012-06-12 21:04:05 UTC (rev 120118)
@@ -1,3 +1,49 @@
+2012-06-12  Kent Tamura  <[email protected]>
+
+        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  <[email protected]>
 
         [chromium] REGRESSION(119769): Canvas2DLayerBridge may go away before its TextureLayerChromium

Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (120117 => 120118)


--- trunk/Source/WebCore/html/HTMLInputElement.cpp	2012-06-12 20:54:41 UTC (rev 120117)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp	2012-06-12 21:04:05 UTC (rev 120118)
@@ -1352,16 +1352,14 @@
 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);
 }
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to