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 af586a9d29000467e1227c9a4470aedf707cf37b..4610c160b5f203a8ae0b24e8aefa6de928f2d8ee 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 056ba7a7f22e515f894127c0f2e9a787244ce070..89ac4211d289dbf8883c38c9876cb2770c395b08 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 453674360798e09edd04ccbbd2d76528d93d07dd..b2d8fa32d5625a0c7885079f030b882afb6038d7 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 bd111551762f259d35285c391505afc1fe2fc841..9b367bbc3ab6b994de10a81202df90a9bedb055b 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();