Skip to content
Permalink
Browse files
Introduce RadioButtonGroup class to keep track of the group members a…
…nd required state

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.

Canonical link: https://commits.webkit.org/93742@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@105710 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
tkent-google committed Jan 24, 2012
1 parent 66f84c2 commit 86e7ffe4f9c003b6e0fb568ba9078d474ac2e890
Showing 19 changed files with 561 additions and 94 deletions.
@@ -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
@@ -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
@@ -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

@@ -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>
@@ -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');

@@ -0,0 +1,3 @@
Tests that adding a radio button to a radio button group is constant in the number of radio buttons.
PASS

@@ -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>
@@ -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

0 comments on commit 86e7ffe

Please sign in to comment.