Introduce RadioButtonGroup class to keep track of the group members and required...
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jan 2012 10:02:32 +0000 (10:02 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 24 Jan 2012 10:02:32 +0000 (10:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=74909

Reviewed by Darin Adler.

Source/WebCore:

RadioButtonGroup contains a set of member radio buttons in the group,
and "required" status of the group. This helps implementing correct
radio button validity, and improving performance of updating validity
status of radio buttons.

This change fixes the following bugs:
- A radio button should be "required" if one of a member of the same
  group has the "required" attribute.
  https://bugs.webkit.org/show_bug.cgi?id=76365
- :invalid style is not applied when a checked radio button is removed
  from its radio group
  https://bugs.webkit.org/show_bug.cgi?id=74914
- Loading a page with N radio buttons in a group takes O(N^2) time.

Tests: fast/forms/radio/radio-live-validation-style.html
       perf/adding-radio-buttons.html

* dom/CheckedRadioButtons.cpp:
(WebCore::RadioButtonGroup::isEmpty):
(WebCore::RadioButtonGroup::isRequired):
(WebCore::RadioButtonGroup::checkedButton):
(WebCore::RadioButtonGroup::RadioButtonGroup):
(WebCore::RadioButtonGroup::create):
(WebCore::RadioButtonGroup::isValid):
(WebCore::RadioButtonGroup::setCheckedButton):
(WebCore::RadioButtonGroup::add):
(WebCore::RadioButtonGroup::updateCheckedState):
(WebCore::RadioButtonGroup::requiredAttributeChanged):
(WebCore::RadioButtonGroup::remove):
(WebCore::RadioButtonGroup::setNeedsValidityCheckForAllButtons):
Add RadioButtonGroup class. It keeps track of pointers to member radio
buttons and required status of the group in addition to the checked
radio button pointer.

(WebCore::CheckedRadioButtons::CheckedRadioButtons):
(WebCore::CheckedRadioButtons::~CheckedRadioButtons):
Define empty constructor and destructor in order to avoid exposing
RadioButtonGroup class.

(WebCore::CheckedRadioButtons::addButton):
(WebCore::CheckedRadioButtons::updateCheckedState):
(WebCore::CheckedRadioButtons::requiredAttributeChanged):
(WebCore::CheckedRadioButtons::checkedButtonForGroup):
(WebCore::CheckedRadioButtons::isInRequiredGroup):
(WebCore::CheckedRadioButtons::removeButton):
Change the HashMap member of this class so that it maps a group name to
a RadioButtonGroup object. These functions just get a RadioButtonGroup
object and call a corresponding member function of RadioButtonGroup.

* dom/CheckedRadioButtons.h: Update declarations.

* html/HTMLFormControlElement.cpp:
(WebCore::HTMLFormControlElement::parseMappedAttribute):
(WebCore::HTMLFormControlElement::requiredAttributeChanged):
Move a part of parseMappedAttribute() into requiredAttributeChanged().
* html/HTMLFormControlElement.h: Add requiredAttributeChanged().
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::valueMissing):
Move required check code to InputType::valueMissing implementations.
RadioInputType needs special handling for checking required state.
readOnly() and disabled() are unnecessary because willValidate() checks them.
(WebCore::HTMLInputElement::setChecked):
Call new function CheckedRadioButtons::updateCheckedState() instead of
removeButton() and updateCheckedRadioButtons().
(WebCore::HTMLInputElement::requiredAttributeChanged):
Override this to call CheckedRadioButtons::requiredAttributeChanged().
* html/HTMLInputElement.h: Add requiredAttributeChanged().
* html/RadioInputType.cpp:
(WebCore::RadioInputType::valueMissing):
Check required state by CheckedRadioButtons::isInRequiredGroup().
* html/RadioInputType.h: Remove attach().

* html/CheckboxInputType.cpp:
(WebCore::CheckboxInputType::valueMissing):
  Move required check from HTMLInputElement::valueMissing().
* html/FileInputType.cpp:
(WebCore::FileInputType::valueMissing): ditto.
* html/TextFieldInputType.cpp:
(WebCore::TextFieldInputType::valueMissing): ditto.

LayoutTests:

* fast/forms/radio/radio-live-validation-style-expected.txt: Added.
* fast/forms/radio/radio-live-validation-style.html: Added.

* fast/forms/script-tests/ValidityState-valueMissing-radio.js:
- Update the expectation for the behavior change of
  https://bugs.webkit.org/show_bug.cgi?id=76365
- Add test cases for radio buttons not in a radio button group.
* fast/forms/ValidityState-valueMissing-radio-expected.txt: ditto.

* perf/adding-radio-buttons-expected.txt: Added.
* perf/adding-radio-buttons.html: Added.

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/forms/ValidityState-valueMissing-radio-expected.txt
LayoutTests/fast/forms/radio/radio-live-validation-style-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/radio/radio-live-validation-style.html [new file with mode: 0644]
LayoutTests/fast/forms/script-tests/ValidityState-valueMissing-radio.js
LayoutTests/perf/adding-radio-buttons-expected.txt [new file with mode: 0644]
LayoutTests/perf/adding-radio-buttons.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/CheckedRadioButtons.cpp
Source/WebCore/dom/CheckedRadioButtons.h
Source/WebCore/html/CheckboxInputType.cpp
Source/WebCore/html/FileInputType.cpp
Source/WebCore/html/HTMLFormControlElement.cpp
Source/WebCore/html/HTMLFormControlElement.h
Source/WebCore/html/HTMLInputElement.cpp
Source/WebCore/html/HTMLInputElement.h
Source/WebCore/html/RadioInputType.cpp
Source/WebCore/html/RadioInputType.h
Source/WebCore/html/TextFieldInputType.cpp

index 9b72934..0eaefaf 100644 (file)
@@ -1,3 +1,22 @@
+2012-01-23  Kent Tamura  <tkent@chromium.org>
+
+        Introduce RadioButtonGroup class to keep track of the group members and required state
+        https://bugs.webkit.org/show_bug.cgi?id=74909
+
+        Reviewed by Darin Adler.
+
+        * fast/forms/radio/radio-live-validation-style-expected.txt: Added.
+        * fast/forms/radio/radio-live-validation-style.html: Added.
+
+        * fast/forms/script-tests/ValidityState-valueMissing-radio.js:
+        - Update the expectation for the behavior change of
+          https://bugs.webkit.org/show_bug.cgi?id=76365
+        - Add test cases for radio buttons not in a radio button group.
+        * fast/forms/ValidityState-valueMissing-radio-expected.txt: ditto.
+
+        * perf/adding-radio-buttons-expected.txt: Added.
+        * perf/adding-radio-buttons.html: Added.
+
 2012-01-24  Noel Gordon  <noel.gordon@gmail.com>
 
         [chromium] PNG image with CMYK ICC color profile renders color-inverted and squashed
index f09ddeb..4d90133 100644 (file)
@@ -6,8 +6,8 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 Without form element
 No checked button:
 PASS inputs[0].validity.valueMissing is true
-PASS inputs[1].validity.valueMissing is false
-PASS inputs[2].validity.valueMissing is false
+PASS inputs[1].validity.valueMissing is true
+PASS inputs[2].validity.valueMissing is true
 The second button has been checked:
 PASS inputs[0].validity.valueMissing is false
 PASS inputs[1].validity.valueMissing is false
@@ -20,11 +20,12 @@ The third button has been checked:
 PASS inputs[0].validity.valueMissing is false
 PASS inputs[1].validity.valueMissing is false
 PASS inputs[2].validity.valueMissing is false
+
 With form element
 No checked button:
 PASS inputs[0].validity.valueMissing is true
-PASS inputs[1].validity.valueMissing is false
-PASS inputs[2].validity.valueMissing is false
+PASS inputs[1].validity.valueMissing is true
+PASS inputs[2].validity.valueMissing is true
 The first button has been checked:
 PASS inputs[0].validity.valueMissing is false
 PASS inputs[1].validity.valueMissing is false
@@ -37,6 +38,13 @@ The third button has been checked:
 PASS inputs[0].validity.valueMissing is false
 PASS inputs[1].validity.valueMissing is false
 PASS inputs[2].validity.valueMissing is false
+
+Not in a radio button group
+PASS requiredButton.validity.valueMissing is false
+PASS requiredButton.validity.valueMissing is true
+PASS button.validity.valueMissing is true
+PASS button.validity.valueMissing is false
+PASS requiredButton.validity.valueMissing is false
 PASS successfullyParsed is true
 
 TEST COMPLETE
diff --git a/LayoutTests/fast/forms/radio/radio-live-validation-style-expected.txt b/LayoutTests/fast/forms/radio/radio-live-validation-style-expected.txt
new file mode 100644 (file)
index 0000000..520225b
--- /dev/null
@@ -0,0 +1,45 @@
+Check if :valid/:invalid CSS pseudo selectors are lively applied
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Removing a checked radio button from a required radio button group by a DOM tree mutation:
+PASS backgroundOf($("radio1")) is validColor
+PASS parent.removeChild($("radio2")); backgroundOf($("radio1")) is invalidColor
+
+Removing a checked radio button from a required radio button group by name attribute change:
+PASS $("radio2").name = "group2"; backgroundOf($("radio1")) is invalidColor
+
+Removing a checked radio button from a required radio button group by type change:
+PASS $("radio2").type = "text"; backgroundOf($("radio1")) is invalidColor
+
+Make a radio button group required by required attribute change:
+PASS backgroundOf($("radio1")) is validColor
+PASS backgroundOf($("radio2")) is validColor
+PASS $("radio1").required = true; backgroundOf($("radio1")) is invalidColor
+PASS backgroundOf($("radio2")) is invalidColor
+
+Make a radio button group not required by required attribute change:
+PASS backgroundOf($("radio1")) is invalidColor
+PASS backgroundOf($("radio2")) is invalidColor
+PASS $("radio1").required = false; backgroundOf($("radio1")) is validColor
+PASS backgroundOf($("radio2")) is validColor
+
+Removing one of multiple required attributes:
+PASS backgroundOf($("radio1")) is invalidColor
+PASS backgroundOf($("radio2")) is invalidColor
+PASS $("radio1").required = false; backgroundOf($("radio1")) is invalidColor
+PASS backgroundOf($("radio2")) is invalidColor
+
+Adding a radio button with the required attribute to a radio button group:
+PASS backgroundOf($("radio1")) is validColor
+PASS parent.appendChild(requiredRadioButton); backgroundOf($("radio1")) is invalidColor
+PASS backgroundOf(requiredRadioButton) is invalidColor
+
+Removing a radio button with the required attribute from a radio button group:
+PASS parent.removeChild(requiredRadioButton); backgroundOf($("radio1")) is validColor
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/radio/radio-live-validation-style.html b/LayoutTests/fast/forms/radio/radio-live-validation-style.html
new file mode 100644 (file)
index 0000000..7324447
--- /dev/null
@@ -0,0 +1,94 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../js/resources/js-test-pre.js"></script>
+<style>
+:invalid { background: rgb(255, 0, 0); }
+:valid { background: rgb(0, 0, 255); }
+</style>
+</head>
+<body>
+<script>
+description('Check if :valid/:invalid CSS pseudo selectors are lively applied');
+
+function $(id) {
+    return document.getElementById(id);
+}
+
+function backgroundOf(element) {
+    return document.defaultView.getComputedStyle(element, null).getPropertyValue('background-color');
+}
+
+var invalidColor = 'rgb(255, 0, 0)';
+var validColor = 'rgb(0, 0, 255)';
+
+var parent = document.createElement('div');
+document.body.appendChild(parent);
+
+debug('Removing a checked radio button from a required radio button group by a DOM tree mutation:');
+parent.innerHTML = '<input type=radio name=group1 required id=radio1>' +
+    '<input type=radio name=group1 checked id=radio2>';
+shouldBe('backgroundOf($("radio1"))', 'validColor');
+shouldBe('parent.removeChild($("radio2")); backgroundOf($("radio1"))', 'invalidColor');
+debug('');
+
+debug('Removing a checked radio button from a required radio button group by name attribute change:');
+parent.innerHTML = '<input type=radio name=group1 required id=radio1>' +
+    '<input type=radio name=group1 checked id=radio2>';
+shouldBe('$("radio2").name = "group2"; backgroundOf($("radio1"))', 'invalidColor');
+debug('');
+
+debug('Removing a checked radio button from a required radio button group by type change:');
+parent.innerHTML = '<input type=radio name=group1 required id=radio1>' +
+    '<input type=radio name=group1 checked id=radio2>';
+shouldBe('$("radio2").type = "text"; backgroundOf($("radio1"))', 'invalidColor');
+debug('');
+
+debug('Make a radio button group required by required attribute change:');
+parent.innerHTML = '<input type=radio name=group1 id=radio1>' +
+    '<input type=radio name=group1 id=radio2>';
+shouldBe('backgroundOf($("radio1"))', 'validColor');
+shouldBe('backgroundOf($("radio2"))', 'validColor');
+shouldBe('$("radio1").required = true; backgroundOf($("radio1"))', 'invalidColor');
+shouldBe('backgroundOf($("radio2"))', 'invalidColor');
+debug('');
+
+debug('Make a radio button group not required by required attribute change:');
+parent.innerHTML = '<input type=radio required name=group1 id=radio1>' +
+    '<input type=radio name=group1 id=radio2>';
+shouldBe('backgroundOf($("radio1"))', 'invalidColor');
+shouldBe('backgroundOf($("radio2"))', 'invalidColor');
+shouldBe('$("radio1").required = false; backgroundOf($("radio1"))', 'validColor');
+shouldBe('backgroundOf($("radio2"))', 'validColor');
+debug('');
+
+
+debug('Removing one of multiple required attributes:');
+parent.innerHTML = '<input type=radio required name=group1 id=radio1>' +
+    '<input type=radio required name=group1 id=radio2>';
+shouldBe('backgroundOf($("radio1"))', 'invalidColor');
+shouldBe('backgroundOf($("radio2"))', 'invalidColor');
+shouldBe('$("radio1").required = false; backgroundOf($("radio1"))', 'invalidColor');
+shouldBe('backgroundOf($("radio2"))', 'invalidColor');
+debug('');
+
+debug('Adding a radio button with the required attribute to a radio button group:');
+parent.innerHTML = '<input type=radio name=group1 id=radio1>';
+shouldBe('backgroundOf($("radio1"))', 'validColor');
+var requiredRadioButton = document.createElement('input');
+requiredRadioButton.type = 'radio';
+requiredRadioButton.name = 'group1';
+requiredRadioButton.required = true;
+shouldBe('parent.appendChild(requiredRadioButton); backgroundOf($("radio1"))', 'invalidColor');
+shouldBe('backgroundOf(requiredRadioButton)', 'invalidColor');
+debug('');
+
+debug('Removing a radio button with the required attribute from a radio button group:');
+shouldBe('parent.removeChild(requiredRadioButton); backgroundOf($("radio1"))', 'validColor');
+debug('');
+
+parent.innerHTML = '';
+</script>
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
index 94c490c..4b0bb4a 100644 (file)
@@ -10,11 +10,8 @@ parent.innerHTML = '<input name=victim type=radio required>'
 var inputs = document.getElementsByName('victim');
 debug('No checked button:');
 shouldBeTrue('inputs[0].validity.valueMissing');
-// The following result should be false because the element does not have
-// "required".  It conforms to HTML5, and this behavior has no practical
-// problems.
-shouldBeFalse('inputs[1].validity.valueMissing');
-shouldBeFalse('inputs[2].validity.valueMissing');
+shouldBeTrue('inputs[1].validity.valueMissing');
+shouldBeTrue('inputs[2].validity.valueMissing');
 debug('The second button has been checked:');
 inputs[1].checked = true;
 shouldBeFalse('inputs[0].validity.valueMissing');
@@ -31,6 +28,7 @@ shouldBeFalse('inputs[0].validity.valueMissing');
 shouldBeFalse('inputs[1].validity.valueMissing');
 shouldBeFalse('inputs[2].validity.valueMissing');
 
+debug('');
 debug('With form element');
 parent.innerHTML = '<form>'
     + '<input name=victim type=radio required>'
@@ -40,9 +38,8 @@ parent.innerHTML = '<form>'
 inputs = document.getElementsByName('victim');
 debug('No checked button:');
 shouldBeTrue('inputs[0].validity.valueMissing');
-// The following result should be false.
-shouldBeFalse('inputs[1].validity.valueMissing');
-shouldBeFalse('inputs[2].validity.valueMissing');
+shouldBeTrue('inputs[1].validity.valueMissing');
+shouldBeTrue('inputs[2].validity.valueMissing');
 debug('The first button has been checked:');
 inputs[0].checked = true;
 shouldBeFalse('inputs[0].validity.valueMissing');
@@ -58,3 +55,22 @@ inputs[2].checked = true;
 shouldBeFalse('inputs[0].validity.valueMissing');
 shouldBeFalse('inputs[1].validity.valueMissing');
 shouldBeFalse('inputs[2].validity.valueMissing');
+
+debug('');
+debug('Not in a radio button group');
+var requiredButton = document.createElement('input');
+requiredButton.type = 'radio';
+requiredButton.name = 'victim';
+requiredButton.required = true;
+shouldBeFalse('requiredButton.validity.valueMissing');
+
+parent.innerHTML = '<input name=victim type=radio required><input name=victim type=radio>';
+requiredButton = document.getElementsByName('victim')[0];
+var button = document.getElementsByName('victim')[1];
+shouldBeTrue('requiredButton.validity.valueMissing');
+shouldBeTrue('button.validity.valueMissing');
+parent.removeChild(button);
+shouldBeFalse('button.validity.valueMissing');
+parent.removeChild(requiredButton);
+shouldBeFalse('requiredButton.validity.valueMissing');
+
diff --git a/LayoutTests/perf/adding-radio-buttons-expected.txt b/LayoutTests/perf/adding-radio-buttons-expected.txt
new file mode 100644 (file)
index 0000000..a29132d
--- /dev/null
@@ -0,0 +1,3 @@
+Tests that adding a radio button to a radio button group is constant in the number of radio buttons.
+PASS
+
diff --git a/LayoutTests/perf/adding-radio-buttons.html b/LayoutTests/perf/adding-radio-buttons.html
new file mode 100644 (file)
index 0000000..1abaef4
--- /dev/null
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../resources/magnitude-perf.js"></script>
+<script>
+var parentForm = null;
+
+function setup(magnitude) {
+    if (parentForm)
+        document.body.removeChild(parentForm);
+    parentForm = document.createElement('form');
+    document.body.appendChild(parentForm);
+    for (var i = 0; i < magnitude; ++i) {
+        var radio = document.createElement('input');
+        radio.type = 'radio';
+        radio.name = 'group1';
+        radio.checked = true;
+        parentForm.appendChild(radio);
+    }
+    parentForm.offsetLeft;
+}
+
+function test(magnitude) {
+    var radio = document.createElement('input');
+    radio.type = 'radio';
+    radio.name = 'group1';
+    radio.checked = true;
+    parentForm.appendChild(radio);
+    radio.offsetLeft;
+    parentForm.removeChild(radio);
+}
+
+Magnitude.description("Tests that adding a radio button to a radio button group is constant in the number of radio buttons.");
+Magnitude.run(setup, test, Magnitude.CONSTANT);
+</script>
+</body>
+</html>
index c137072..e9ab5fe 100644 (file)
@@ -1,3 +1,90 @@
+2012-01-23  Kent Tamura  <tkent@chromium.org>
+
+        Introduce RadioButtonGroup class to keep track of the group members and required state
+        https://bugs.webkit.org/show_bug.cgi?id=74909
+
+        Reviewed by Darin Adler.
+
+        RadioButtonGroup contains a set of member radio buttons in the group,
+        and "required" status of the group. This helps implementing correct
+        radio button validity, and improving performance of updating validity
+        status of radio buttons.
+
+        This change fixes the following bugs:
+        - A radio button should be "required" if one of a member of the same
+          group has the "required" attribute.
+          https://bugs.webkit.org/show_bug.cgi?id=76365
+        - :invalid style is not applied when a checked radio button is removed
+          from its radio group
+          https://bugs.webkit.org/show_bug.cgi?id=74914
+        - Loading a page with N radio buttons in a group takes O(N^2) time.
+
+        Tests: fast/forms/radio/radio-live-validation-style.html
+               perf/adding-radio-buttons.html
+
+        * dom/CheckedRadioButtons.cpp:
+        (WebCore::RadioButtonGroup::isEmpty):
+        (WebCore::RadioButtonGroup::isRequired):
+        (WebCore::RadioButtonGroup::checkedButton):
+        (WebCore::RadioButtonGroup::RadioButtonGroup):
+        (WebCore::RadioButtonGroup::create):
+        (WebCore::RadioButtonGroup::isValid):
+        (WebCore::RadioButtonGroup::setCheckedButton):
+        (WebCore::RadioButtonGroup::add):
+        (WebCore::RadioButtonGroup::updateCheckedState):
+        (WebCore::RadioButtonGroup::requiredAttributeChanged):
+        (WebCore::RadioButtonGroup::remove):
+        (WebCore::RadioButtonGroup::setNeedsValidityCheckForAllButtons):
+        Add RadioButtonGroup class. It keeps track of pointers to member radio
+        buttons and required status of the group in addition to the checked
+        radio button pointer.
+
+        (WebCore::CheckedRadioButtons::CheckedRadioButtons):
+        (WebCore::CheckedRadioButtons::~CheckedRadioButtons):
+        Define empty constructor and destructor in order to avoid exposing
+        RadioButtonGroup class.
+
+        (WebCore::CheckedRadioButtons::addButton):
+        (WebCore::CheckedRadioButtons::updateCheckedState):
+        (WebCore::CheckedRadioButtons::requiredAttributeChanged):
+        (WebCore::CheckedRadioButtons::checkedButtonForGroup):
+        (WebCore::CheckedRadioButtons::isInRequiredGroup):
+        (WebCore::CheckedRadioButtons::removeButton):
+        Change the HashMap member of this class so that it maps a group name to
+        a RadioButtonGroup object. These functions just get a RadioButtonGroup
+        object and call a corresponding member function of RadioButtonGroup.
+
+        * dom/CheckedRadioButtons.h: Update declarations.
+
+        * html/HTMLFormControlElement.cpp:
+        (WebCore::HTMLFormControlElement::parseMappedAttribute):
+        (WebCore::HTMLFormControlElement::requiredAttributeChanged):
+        Move a part of parseMappedAttribute() into requiredAttributeChanged().
+        * html/HTMLFormControlElement.h: Add requiredAttributeChanged().
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::valueMissing):
+        Move required check code to InputType::valueMissing implementations.
+        RadioInputType needs special handling for checking required state.
+        readOnly() and disabled() are unnecessary because willValidate() checks them.
+        (WebCore::HTMLInputElement::setChecked):
+        Call new function CheckedRadioButtons::updateCheckedState() instead of
+        removeButton() and updateCheckedRadioButtons().
+        (WebCore::HTMLInputElement::requiredAttributeChanged):
+        Override this to call CheckedRadioButtons::requiredAttributeChanged().
+        * html/HTMLInputElement.h: Add requiredAttributeChanged().
+        * html/RadioInputType.cpp:
+        (WebCore::RadioInputType::valueMissing):
+        Check required state by CheckedRadioButtons::isInRequiredGroup().
+        * html/RadioInputType.h: Remove attach().
+
+        * html/CheckboxInputType.cpp:
+        (WebCore::CheckboxInputType::valueMissing):
+          Move required check from HTMLInputElement::valueMissing().
+        * html/FileInputType.cpp:
+        (WebCore::FileInputType::valueMissing): ditto.
+        * html/TextFieldInputType.cpp:
+        (WebCore::TextFieldInputType::valueMissing): ditto.
+
 2012-01-24  Noel Gordon  <noel.gordon@gmail.com>
 
         [chromium] PNG image with CMYK ICC color profile renders color-inverted and squashed
index 9ab8912..9b5c503 100644 (file)
 #include "CheckedRadioButtons.h"
 
 #include "HTMLInputElement.h"
+#include <wtf/HashSet.h>
 
 namespace WebCore {
 
+class RadioButtonGroup {
+public:
+    static PassOwnPtr<RadioButtonGroup> create();
+    bool isEmpty() const { return m_members.isEmpty(); }
+    bool isRequired() const { return m_requiredCount; }
+    HTMLInputElement* checkedButton() const { return m_checkedButton; }
+    void add(HTMLInputElement*);
+    void updateCheckedState(HTMLInputElement*);
+    void requiredAttributeChanged(HTMLInputElement*);
+    void remove(HTMLInputElement*);
+
+private:
+    RadioButtonGroup();
+    void setNeedsValidityCheckForAllButtons();
+    bool isValid() const;
+    void setCheckedButton(HTMLInputElement*);
+
+    HashSet<HTMLInputElement*> m_members;
+    HTMLInputElement* m_checkedButton;
+    size_t m_requiredCount;
+};
+
+RadioButtonGroup::RadioButtonGroup()
+    : m_checkedButton(0)
+    , m_requiredCount(0)
+{
+}
+
+PassOwnPtr<RadioButtonGroup> RadioButtonGroup::create()
+{
+    return adoptPtr(new RadioButtonGroup);
+}
+
+inline bool RadioButtonGroup::isValid() const
+{
+    return !isRequired() || m_checkedButton;
+}
+
+void RadioButtonGroup::setCheckedButton(HTMLInputElement* button)
+{
+    HTMLInputElement* oldCheckedButton = m_checkedButton;
+    if (oldCheckedButton == button)
+        return;
+    m_checkedButton = button;
+    if (oldCheckedButton)
+        oldCheckedButton->setChecked(false);
+}
+
+void RadioButtonGroup::add(HTMLInputElement* button)
+{
+    ASSERT(button->isRadioButton());
+    if (!m_members.add(button).second)
+        return;
+    bool groupWasValid = isValid();
+    if (button->required())
+        ++m_requiredCount;
+    if (button->checked())
+        setCheckedButton(button);
+
+    bool groupIsValid = isValid();
+    if (groupWasValid != groupIsValid)
+        setNeedsValidityCheckForAllButtons();
+    else if (!groupIsValid) {
+        // A radio button not in a group is always valid. We need to make it
+        // invalid only if the group is invalid.
+        button->setNeedsValidityCheck();
+    }
+}
+
+void RadioButtonGroup::updateCheckedState(HTMLInputElement* button)
+{
+    ASSERT(button->isRadioButton());
+    ASSERT(m_members.contains(button));
+    bool wasValid = isValid();
+    if (button->checked())
+        setCheckedButton(button);
+    else {
+        if (m_checkedButton == button)
+            m_checkedButton = 0;
+    }
+    if (wasValid != isValid())
+        setNeedsValidityCheckForAllButtons();
+}
+
+void RadioButtonGroup::requiredAttributeChanged(HTMLInputElement* button)
+{
+    ASSERT(button->isRadioButton());
+    ASSERT(m_members.contains(button));
+    bool wasValid = isValid();
+    if (button->required())
+        ++m_requiredCount;
+    else {
+        ASSERT(m_requiredCount);
+        --m_requiredCount;
+    }
+    if (wasValid != isValid())
+        setNeedsValidityCheckForAllButtons();
+}
+
+void RadioButtonGroup::remove(HTMLInputElement* button)
+{
+    ASSERT(button->isRadioButton());
+    HashSet<HTMLInputElement*>::iterator it = m_members.find(button);
+    if (it == m_members.end())
+        return;
+    bool wasValid = isValid();
+    m_members.remove(it);
+    if (button->required()) {
+        ASSERT(m_requiredCount);
+        --m_requiredCount;
+    }
+    if (m_checkedButton == button)
+        m_checkedButton = 0;
+
+    if (m_members.isEmpty()) {
+        ASSERT(!m_requiredCount);
+        ASSERT(!m_checkedButton);
+    } else if (wasValid != isValid())
+        setNeedsValidityCheckForAllButtons();
+    if (!wasValid) {
+        // A radio button not in a group is always valid. We need to make it
+        // valid only if the group was invalid.
+        button->setNeedsValidityCheck();
+    }
+}
+
+void RadioButtonGroup::setNeedsValidityCheckForAllButtons()
+{
+    typedef HashSet<HTMLInputElement*>::const_iterator Iterator;
+    Iterator end = m_members.end();
+    for (Iterator it = m_members.begin(); it != end; ++it) {
+        HTMLInputElement* button = *it;
+        ASSERT(button->isRadioButton());
+        button->setNeedsValidityCheck();
+    }
+}
+
+// ----------------------------------------------------------------
+
 static inline bool shouldMakeRadioGroup(HTMLInputElement* element)
 {
     return element->isRadioButton() && !element->name().isEmpty() && element->inDocument();
 }
 
+// Explicity define empty constructor and destructor in order to prevent the
+// compiler from generating them as inlines. So we don't need to to define
+// RadioButtonGroup in the header.
+CheckedRadioButtons::CheckedRadioButtons()
+{
+}
+
+CheckedRadioButtons::~CheckedRadioButtons()
+{
+}
+
 void CheckedRadioButtons::addButton(HTMLInputElement* element)
 {
     if (!shouldMakeRadioGroup(element))
         return;
 
-    // We only track checked buttons.
-    if (!element->checked())
-        return;
+    if (!m_nameToGroupMap)
+        m_nameToGroupMap = adoptPtr(new NameToGroupMap);
 
-    if (!m_nameToCheckedRadioButtonMap)
-        m_nameToCheckedRadioButtonMap = adoptPtr(new NameToInputMap);
+    OwnPtr<RadioButtonGroup>& group = m_nameToGroupMap->add(element->name().impl(), PassOwnPtr<RadioButtonGroup>()).first->second;
+    if (!group)
+        group = RadioButtonGroup::create();
+    group->add(element);
+}
 
-    pair<NameToInputMap::iterator, bool> result = m_nameToCheckedRadioButtonMap->add(element->name().impl(), element);
-    if (result.second)
+void CheckedRadioButtons::updateCheckedState(HTMLInputElement* element)
+{
+    if (!shouldMakeRadioGroup(element))
         return;
-    
-    HTMLInputElement* oldCheckedButton = result.first->second;
-    if (oldCheckedButton == element)
+    ASSERT(m_nameToGroupMap);
+    if (!m_nameToGroupMap)
         return;
+    RadioButtonGroup* group = m_nameToGroupMap->get(element->name().impl());
+    ASSERT(group);
+    group->updateCheckedState(element);
+}
 
-    result.first->second = element;
-    oldCheckedButton->setChecked(false);
+void CheckedRadioButtons::requiredAttributeChanged(HTMLInputElement* element)
+{
+    if (!shouldMakeRadioGroup(element))
+        return;
+    ASSERT(m_nameToGroupMap);
+    if (!m_nameToGroupMap)
+        return;
+    RadioButtonGroup* group = m_nameToGroupMap->get(element->name().impl());
+    ASSERT(group);
+    group->requiredAttributeChanged(element);
 }
 
 HTMLInputElement* CheckedRadioButtons::checkedButtonForGroup(const AtomicString& name) const
 {
-    if (!m_nameToCheckedRadioButtonMap)
+    if (!m_nameToGroupMap)
         return 0;
+    m_nameToGroupMap->checkConsistency();
+    RadioButtonGroup* group = m_nameToGroupMap->get(name.impl());
+    return group ? group->checkedButton() : 0;
+}
 
-    m_nameToCheckedRadioButtonMap->checkConsistency();
-    
-    return m_nameToCheckedRadioButtonMap->get(name.impl());
+bool CheckedRadioButtons::isInRequiredGroup(HTMLInputElement* element) const
+{
+    ASSERT(element->isRadioButton());
+    if (!element->inDocument())
+        return false;
+    if (!m_nameToGroupMap)
+        return false;
+
+    RadioButtonGroup* group = m_nameToGroupMap->get(element->name().impl());
+    return group && group->isRequired();
 }
 
 void CheckedRadioButtons::removeButton(HTMLInputElement* element)
 {
-    if (element->name().isEmpty() || !m_nameToCheckedRadioButtonMap)
+    if (!shouldMakeRadioGroup(element))
         return;
-    
-    m_nameToCheckedRadioButtonMap->checkConsistency();
-
-    NameToInputMap::iterator it = m_nameToCheckedRadioButtonMap->find(element->name().impl());
-    if (it == m_nameToCheckedRadioButtonMap->end() || it->second != element)
+    if (!m_nameToGroupMap)
         return;
-    
-    ASSERT(element->shouldAppearChecked());
-    ASSERT(element->isRadioButton());
 
-    m_nameToCheckedRadioButtonMap->remove(it);
-    if (m_nameToCheckedRadioButtonMap->isEmpty())
-        m_nameToCheckedRadioButtonMap.clear();
+    m_nameToGroupMap->checkConsistency();
+    NameToGroupMap::iterator it = m_nameToGroupMap->find(element->name().impl());
+    if (it == m_nameToGroupMap->end())
+        return;
+    it->second->remove(element);
+    if (it->second->isEmpty()) {
+        // FIXME: We may skip deallocating the empty RadioButtonGroup for
+        // performance improvement. If we do so, we need to change the key type
+        // of m_nameToGroupMap from AtomicStringImpl* to RefPtr<AtomicStringImpl>.
+        m_nameToGroupMap->remove(it);
+        if (m_nameToGroupMap->isEmpty())
+            m_nameToGroupMap.clear();
+    }
 }
 
 } // namespace
index 925b506..f050faa 100644 (file)
 namespace WebCore {
 
 class HTMLInputElement;
+class RadioButtonGroup;
 
+// FIXME: Rename the class. The class was a simple map from a name to a checked
+// radio button. It manages RadioButtonGroup objects now.
 class CheckedRadioButtons {
 public:
+    CheckedRadioButtons();
+    ~CheckedRadioButtons();
     void addButton(HTMLInputElement*);
+    void updateCheckedState(HTMLInputElement*);
+    void requiredAttributeChanged(HTMLInputElement*);
     void removeButton(HTMLInputElement*);
     HTMLInputElement* checkedButtonForGroup(const AtomicString& groupName) const;
+    bool isInRequiredGroup(HTMLInputElement*) const;
 
 private:
-    typedef HashMap<AtomicStringImpl*, HTMLInputElement*> NameToInputMap;
-    OwnPtr<NameToInputMap> m_nameToCheckedRadioButtonMap;
+    typedef HashMap<AtomicStringImpl*, OwnPtr<RadioButtonGroup> > NameToGroupMap;
+    OwnPtr<NameToGroupMap> m_nameToGroupMap;
 };
 
 } // namespace WebCore
index dda172a..ed83517 100644 (file)
@@ -51,7 +51,7 @@ const AtomicString& CheckboxInputType::formControlType() const
 
 bool CheckboxInputType::valueMissing(const String&) const
 {
-    return !element()->checked();
+    return element()->required() && !element()->checked();
 }
 
 String CheckboxInputType::valueMissingText() const
index 655e81e..76efa41 100644 (file)
@@ -129,7 +129,7 @@ bool FileInputType::appendFormData(FormDataList& encoding, bool multipart) const
 
 bool FileInputType::valueMissing(const String& value) const
 {
-    return value.isEmpty();
+    return element()->required() && value.isEmpty();
 }
 
 String FileInputType::valueMissingText() const
index 6e2ed6c..2b06f3d 100644 (file)
@@ -122,15 +122,21 @@ void HTMLFormControlElement::parseMappedAttribute(Attribute* attr)
     } else if (attr->name() == requiredAttr) {
         bool oldRequired = m_required;
         m_required = !attr->isNull();
-        if (oldRequired != m_required) {
-            setNeedsValidityCheck();
-            setNeedsStyleRecalc(); // Updates for :required :optional classes.
-        }
+        if (oldRequired != m_required)
+            requiredAttributeChanged();
     } else
         HTMLElement::parseMappedAttribute(attr);
     setNeedsWillValidateCheck();
 }
 
+void HTMLFormControlElement::requiredAttributeChanged()
+{
+    setNeedsValidityCheck();
+    // Style recalculation is needed because style selectors may include
+    // :required and :optional pseudo-classes.
+    setNeedsStyleRecalc();
+}
+
 static bool shouldAutofocus(HTMLFormControlElement* element)
 {
     if (!element->autofocus())
index f500b6d..edc406f 100644 (file)
@@ -115,6 +115,7 @@ protected:
     HTMLFormControlElement(const QualifiedName& tagName, Document*, HTMLFormElement*);
 
     virtual void parseMappedAttribute(Attribute*);
+    virtual void requiredAttributeChanged();
     virtual void attach();
     virtual void insertedIntoTree(bool deep);
     virtual void removedFromTree(bool deep);
index 7501bc4..ecde0b7 100644 (file)
@@ -178,38 +178,6 @@ bool HTMLInputElement::shouldAutocomplete() const
     return HTMLTextFormControlElement::shouldAutocomplete();
 }
 
-void HTMLInputElement::updateCheckedRadioButtons()
-{
-    checkedRadioButtons().addButton(this);
-
-    if (form()) {
-        const Vector<FormAssociatedElement*>& controls = form()->associatedElements();
-        for (unsigned i = 0; i < controls.size(); ++i) {
-            if (!controls[i]->isFormControlElement())
-                continue;
-            HTMLFormControlElement* control = static_cast<HTMLFormControlElement*>(controls[i]);
-            if (control->name() != name())
-                continue;
-            if (control->type() != type())
-                continue;
-            control->setNeedsValidityCheck();
-        }
-    } else {
-        typedef Document::FormElementListHashSet::const_iterator Iterator;
-        Iterator end = document()->formElements()->end();
-        for (Iterator it = document()->formElements()->begin(); it != end; ++it) {
-            HTMLFormControlElementWithState* control = *it;
-            if (control->formControlName() != name())
-                continue;
-            if (control->formControlType() != type())
-                continue;
-            if (control->form())
-                continue;
-            control->setNeedsValidityCheck();
-        }
-    }
-}
-
 bool HTMLInputElement::isValidValue(const String& value) const
 {
     if (!m_inputType->canSetStringValue()) {
@@ -232,8 +200,6 @@ bool HTMLInputElement::typeMismatch() const
 
 bool HTMLInputElement::valueMissing(const String& value) const
 {
-    if (!isRequiredFormControl() || readOnly() || disabled())
-        return false;
     return m_inputType->valueMissing(value);
 }
 
@@ -963,13 +929,11 @@ void HTMLInputElement::setChecked(bool nowChecked, bool sendChangeEvent)
     if (checked() == nowChecked)
         return;
 
-    checkedRadioButtons().removeButton(this);
-
     m_reflectsCheckedAttribute = false;
     m_isChecked = nowChecked;
     setNeedsStyleRecalc();
     if (isRadioButton())
-        updateCheckedRadioButtons();
+        checkedRadioButtons().updateCheckedState(this);
     if (renderer() && renderer()->style()->hasAppearance())
         renderer()->theme()->stateChanged(renderer(), CheckedState);
     setNeedsValidityCheck();
@@ -1559,6 +1523,12 @@ bool HTMLInputElement::recalcWillValidate() const
     return m_inputType->supportsValidation() && HTMLTextFormControlElement::recalcWillValidate();
 }
 
+void HTMLInputElement::requiredAttributeChanged()
+{
+    HTMLTextFormControlElement::requiredAttributeChanged();
+    checkedRadioButtons().requiredAttributeChanged(this);
+}
+
 #if ENABLE(INPUT_COLOR)
 void HTMLInputElement::selectColorInColorChooser(const Color& color)
 {
index fc6f068..47ac4d3 100644 (file)
@@ -321,6 +321,7 @@ private:
     virtual bool isOptionalFormControl() const { return !isRequiredFormControl(); }
     virtual bool isRequiredFormControl() const;
     virtual bool recalcWillValidate() const;
+    virtual void requiredAttributeChanged() OVERRIDE;
 
     void updateType();
     
index 7e69517..baec9e8 100644 (file)
@@ -48,7 +48,7 @@ const AtomicString& RadioInputType::formControlType() const
 
 bool RadioInputType::valueMissing(const String&) const
 {
-    return !element()->checkedRadioButtons().checkedButtonForGroup(element()->name());
+    return element()->checkedRadioButtons().isInRequiredGroup(element()) && !element()->checkedRadioButtons().checkedButtonForGroup(element()->name());
 }
 
 String RadioInputType::valueMissingText() const
@@ -133,12 +133,6 @@ bool RadioInputType::isKeyboardFocusable() const
     return element()->checked() || !element()->checkedRadioButtons().checkedButtonForGroup(element()->name());
 }
 
-void RadioInputType::attach()
-{
-    InputType::attach();
-    element()->updateCheckedRadioButtons();
-}
-
 bool RadioInputType::shouldSendChangeEventAfterCheckedChanged()
 {
     // Don't send a change event for a radio button that's getting unchecked.
index 525ed69..9cefe51 100644 (file)
@@ -48,7 +48,6 @@ private:
     virtual void handleKeydownEvent(KeyboardEvent*) OVERRIDE;
     virtual void handleKeyupEvent(KeyboardEvent*) OVERRIDE;
     virtual bool isKeyboardFocusable() const OVERRIDE;
-    virtual void attach() OVERRIDE;
     virtual bool shouldSendChangeEventAfterCheckedChanged() OVERRIDE;
     virtual PassOwnPtr<ClickHandlingState> willDispatchClick() OVERRIDE;
     virtual void didDispatchClick(Event*, const ClickHandlingState&) OVERRIDE;
index c9c9083..37e0b9f 100644 (file)
@@ -69,7 +69,7 @@ bool TextFieldInputType::isTextField() const
 
 bool TextFieldInputType::valueMissing(const String& value) const
 {
-    return value.isEmpty();
+    return element()->required() && value.isEmpty();
 }
 
 bool TextFieldInputType::canSetSuggestedValue()