WebCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2007 00:53:21 +0000 (00:53 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Nov 2007 00:53:21 +0000 (00:53 +0000)
        Reviewed by Adele.

        - fix http://bugs.webkit.org/show_bug.cgi?id=16191
          REGRESSION: cannot tab to radio input after setting checked to false

        Test: fast/forms/input-radio-checked-tab.html

        * html/HTMLFormElement.cpp:
        (WebCore::HTMLFormElement::CheckedRadioButtons::addButton): Fix this code
        so that it doesn't call setChecked(false) until after the map has been updated.
        Otherwise, we can end up deallocating the map before manipulating it. As long
        as I was changing the function, I decided to make it do only a single hash
        table lookup.

        * html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::setChecked):
        Remove the button from the radio buttons set before changing the checked
        state. This matches the idiom used elsewhere and fixes the problem where
        setting checked to false would not remove it from the set.

LayoutTests:

        Reviewed by Adele.

        - test for http://bugs.webkit.org/show_bug.cgi?id=16191
          REGRESSION: cannot tab to radio input after setting checked to false

        * fast/forms/input-radio-checked-tab-expected.txt: Added.
        * fast/forms/input-radio-checked-tab.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/input-radio-checked-tab-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/input-radio-checked-tab.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/html/HTMLFormElement.cpp
WebCore/html/HTMLInputElement.cpp

index af586a9..4610c16 100644 (file)
@@ -1,3 +1,13 @@
+2007-11-29  Darin Adler  <darin@apple.com>
+
+        Reviewed by Adele.
+
+        - test for http://bugs.webkit.org/show_bug.cgi?id=16191
+          REGRESSION: cannot tab to radio input after setting checked to false
+
+        * fast/forms/input-radio-checked-tab-expected.txt: Added.
+        * fast/forms/input-radio-checked-tab.html: Added.
+
 2007-11-29  Alice Liu  <alice.liu@apple.com>
 
         Move Mac-specific results of fast/dom/wrapper-classes.html into platform/mac and replace them with non-Mac results
diff --git a/LayoutTests/fast/forms/input-radio-checked-tab-expected.txt b/LayoutTests/fast/forms/input-radio-checked-tab-expected.txt
new file mode 100644 (file)
index 0000000..fa897f1
--- /dev/null
@@ -0,0 +1,4 @@
+
+This tests that you can still focus a radio button after unchecking it by setting its checked property to false.
+
+SUCCESS
diff --git a/LayoutTests/fast/forms/input-radio-checked-tab.html b/LayoutTests/fast/forms/input-radio-checked-tab.html
new file mode 100644 (file)
index 0000000..20775f0
--- /dev/null
@@ -0,0 +1,22 @@
+<html>
+<head>
+    <script>
+    function runTest() {
+        if (window.layoutTestController)
+            layoutTestController.dumpAsText();
+
+        document.getElementById('tom').checked = false;
+        document.getElementById('result').innerHTML = 'FAILURE';
+
+        // Simulate an option-tab, which can tab to a radio button with default preference settings. 
+        if (window.eventSender)
+            eventSender.keyDown('\t', ["altKey"]);
+    }
+    </script>
+</head>
+<body onload="runTest()">
+    <input id="tom" type="radio" onfocus="document.getElementById('result').innerHTML = 'SUCCESS'" name="joe" checked>
+    <p>This tests that you can still focus a radio button after unchecking it by setting its checked property to false.</p>
+    <div id="result">TEST NOT RUN</div>
+</body>
+</html>
index 056ba7a..89ac421 100644 (file)
@@ -1,3 +1,24 @@
+2007-11-29  Darin Adler  <darin@apple.com>
+
+        Reviewed by Adele.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=16191
+          REGRESSION: cannot tab to radio input after setting checked to false
+
+        Test: fast/forms/input-radio-checked-tab.html
+
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::CheckedRadioButtons::addButton): Fix this code
+        so that it doesn't call setChecked(false) until after the map has been updated.
+        Otherwise, we can end up deallocating the map before manipulating it. As long
+        as I was changing the function, I decided to make it do only a single hash
+        table lookup.
+
+        * html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::setChecked):
+        Remove the button from the radio buttons set before changing the checked
+        state. This matches the idiom used elsewhere and fixes the problem where
+        setting checked to false would not remove it from the set.
+
 2007-11-29  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by Darin Adler.
index 4536743..b2d8fa3 100644 (file)
@@ -721,21 +721,26 @@ void HTMLFormElement::CheckedRadioButtons::addButton(HTMLGenericFormElement* ele
         return;
 
     HTMLInputElement* inputElement = static_cast<HTMLInputElement*>(element);
+
     // We only track checked buttons.
     if (!inputElement->checked())
         return;
 
     if (!m_nameToCheckedRadioButtonMap)
         m_nameToCheckedRadioButtonMap.set(new NameToInputMap);
-    else {
-        HTMLInputElement* currentCheckedRadio = m_nameToCheckedRadioButtonMap->get(element->name().impl());
-        if (currentCheckedRadio && currentCheckedRadio != element)
-            currentCheckedRadio->setChecked(false);
-    }
 
-    m_nameToCheckedRadioButtonMap->set(element->name().impl(), inputElement);    
-}
+    pair<NameToInputMap::iterator, bool> result = m_nameToCheckedRadioButtonMap->add(element->name().impl(), inputElement);
+    if (result.second)
+        return;
     
+    HTMLInputElement* oldCheckedButton = result.first->second;
+    if (oldCheckedButton == inputElement)
+        return;
+
+    result.first->second = inputElement;
+    oldCheckedButton->setChecked(false);
+}
+
 HTMLInputElement* HTMLFormElement::CheckedRadioButtons::checkedButtonForGroup(const AtomicString& name) const
 {
     if (!m_nameToCheckedRadioButtonMap)
index bd11155..9b367bb 100644 (file)
@@ -864,6 +864,8 @@ void HTMLInputElement::setChecked(bool nowChecked, bool sendChangeEvent)
     if (checked() == nowChecked)
         return;
 
+    checkedRadioButtons(this).removeButton(this);
+
     m_useDefaultChecked = false;
     m_checked = nowChecked;
     setChanged();