Commit 5addd957 authored by Kent Tamura's avatar Kent Tamura Committed by Commit Bot

Radio buttons should make a group even if they are disconnected and not owned by a form

If a radio button is connected or owned by a form, radio button
groups are managed by RadioButtonGroupScope in order to complete
various operations for a group in O(1).
Otherwise, we did not assume a radio button belonged to any
groups.  It didn't conform to the HTML standard, and was not
interoperable with Firefox.

After this CL, such radio buttons are grouped without
RadioButtonGroupScope's help. We iterate over the tree including
the target radio buttons to find a checked button in the group or
|required| attribute in the group.

Bug: 883723
Change-Id: I56185559592dff6b0254655aeb499ed6ac29df64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1988087
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729577}
parent 15180acf
......@@ -131,6 +131,13 @@ class Traversal {
}
template <typename MatchFunc>
static ElementType* FirstWithin(const ContainerNode&, MatchFunc);
static ElementType* InclusiveFirstWithin(Node& current) {
if (IsElementOfType<const ElementType>(current))
return To<ElementType>(&current);
return FirstWithin(current);
}
static ElementType* LastWithin(const ContainerNode& current) {
return LastWithinTemplate(current);
}
......
......@@ -982,6 +982,7 @@ void HTMLInputElement::setChecked(bool now_checked,
if (checked() == now_checked)
return;
input_type_->WillUpdateCheckedness(now_checked);
is_checked_ = now_checked;
if (RadioButtonGroupScope* scope = GetRadioButtonGroupScope())
......
......@@ -253,6 +253,8 @@ void InputType::SetValueAsDecimal(const Decimal& new_value,
void InputType::ReadingChecked() const {}
void InputType::WillUpdateCheckedness(bool) {}
bool InputType::SupportsValidation() const {
return true;
}
......
......@@ -104,7 +104,12 @@ class CORE_EXPORT InputType : public GarbageCollected<InputType> {
virtual void SetValueAsDecimal(const Decimal&,
TextFieldEventBehavior,
ExceptionState&) const;
// Functions related to 'checked'
virtual void ReadingChecked() const;
// The function is called just before updating checkedness.
virtual void WillUpdateCheckedness(bool new_checked);
// Validation functions
......
......@@ -59,9 +59,30 @@ bool RadioInputType::ValueMissing(const String&) const {
HTMLInputElement& input = GetElement();
if (auto* scope = input.GetRadioButtonGroupScope())
return scope->IsInRequiredGroup(&input) && !CheckedRadioButtonForGroup();
// TODO(crbug.com/883723): This function should work even if this radio
// button doesn't belong to any RadioButtonGroupScope.
return false;
// This element is not managed by a RadioButtonGroupScope. We need to traverse
// the tree from TreeRoot.
DCHECK(!input.isConnected());
DCHECK(!input.formOwner());
const AtomicString& name = input.GetName();
if (name.IsEmpty())
return false;
bool is_required = false;
bool is_checked = false;
Node& root = input.TreeRoot();
for (auto* another = Traversal<HTMLInputElement>::InclusiveFirstWithin(root);
another; another = Traversal<HTMLInputElement>::Next(*another, &root)) {
if (another->type() != input_type_names::kRadio ||
another->GetName() != name || another->formOwner())
continue;
if (another->checked())
is_checked = true;
if (another->FastHasAttribute(html_names::kRequiredAttr))
is_required = true;
if (is_checked && is_required)
return false;
}
return is_required && !is_checked;
}
String RadioInputType::ValueMissingText() const {
......@@ -257,7 +278,36 @@ HTMLInputElement* RadioInputType::CheckedRadioButtonForGroup() const {
return &input;
if (auto* scope = input.GetRadioButtonGroupScope())
return scope->CheckedButtonForGroup(input.GetName());
// This element is not managed by a RadioButtonGroupScope. We need to traverse
// the tree from TreeRoot.
DCHECK(!input.isConnected());
DCHECK(!input.formOwner());
const AtomicString& name = input.GetName();
if (name.IsEmpty())
return nullptr;
Node& root = input.TreeRoot();
for (auto* another = Traversal<HTMLInputElement>::InclusiveFirstWithin(root);
another; another = Traversal<HTMLInputElement>::Next(*another, &root)) {
if (another->type() != input_type_names::kRadio ||
another->GetName() != name || another->formOwner())
continue;
if (another->checked())
return another;
}
return nullptr;
}
void RadioInputType::WillUpdateCheckedness(bool new_checked) {
if (!new_checked)
return;
if (GetElement().GetRadioButtonGroupScope()) {
// Buttons in RadioButtonGroupScope are handled in
// HTMLInputElement::setChecked().
return;
}
if (auto* input = CheckedRadioButtonForGroup())
input->setChecked(false);
}
} // namespace blink
......@@ -46,6 +46,7 @@ class RadioInputType final : public BaseCheckableInputType {
private:
void CountUsage() override;
const AtomicString& FormControlType() const override;
void WillUpdateCheckedness(bool new_checked) override;
bool ValueMissing(const String&) const override;
String ValueMissingText() const override;
void HandleClickEvent(MouseEvent&) override;
......
......@@ -38,7 +38,7 @@ PASS [INPUT in NUMBER status] validity.valid must be false if validity.rangeUnde
PASS [INPUT in NUMBER status] validity.valid must be false if validity.stepMismatch is true
FAIL [INPUT in NUMBER status] validity.valid must be false if validity.valueMissing is true assert_true: The validity.valid should be true, when control is disabled. expected true got false
PASS [INPUT in CHECKBOX status] validity.valid must be false if validity.valueMissing is true
FAIL [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true assert_false: The validity.valid should be false. expected false got true
PASS [INPUT in RADIO status] validity.valid must be false if validity.valueMissing is true
PASS [INPUT in FILE status] validity.valid must be false if validity.valueMissing is true
PASS [select] validity.valid must be false if validity.valueMissing is true
FAIL [textarea] validity.valid must be false if validity.valueMissing is true assert_true: The validity.valid should be true, when control is disabled. expected true got false
......
This is a testharness.js-based test.
Found 94 tests; 48 PASS, 46 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 95 tests; 50 PASS, 45 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS [INPUT in TEXT status] The required attribute is not set
PASS [INPUT in TEXT status] The value is not empty and required is true
FAIL [INPUT in TEXT status] The value is empty and required is true assert_false: The validity.valueMissing should be false, when control is disabled. expected false got true
......@@ -85,7 +85,8 @@ PASS [INPUT in CHECKBOX status] The checked attribute is true
PASS [INPUT in CHECKBOX status] The checked attribute is false
PASS [INPUT in RADIO status] The required attribute is not set
PASS [INPUT in RADIO status] The checked attribute is true
FAIL [INPUT in RADIO status] The checked attribute is false assert_true: The validity.valueMissing should be true. expected true got false
PASS [INPUT in RADIO status] The checked attribute is false
PASS [INPUT in RADIO status] The checked attribute is false and the name attribute is empty
PASS [INPUT in FILE status] The required attribute is not set
PASS [INPUT in FILE status] The Files attribute is null
PASS [select] The required attribute is not set
......
......@@ -132,7 +132,8 @@
testData: [
{conditions: {required: false, checked: false, name: "test4"}, expected: false, name: "[target] The required attribute is not set"},
{conditions: {required: true, checked: true, name: "test5"}, expected: false, name: "[target] The checked attribute is true"},
{conditions: {required: true, checked: false, name: "test6"}, expected: true, name: "[target] The checked attribute is false"}
{conditions: {required: true, checked: false, name: "test6"}, expected: true, name: "[target] The checked attribute is false"},
{conditions: {required: true, checked: false, name: ""}, expected: false, name: "[target] The checked attribute is false and the name attribute is empty"}
]
},
{
......
......@@ -174,4 +174,34 @@
assert_true(radio71.checked, "canceled click event on radio should leave the previously-checked radio checked");
assert_false(radio72.checked, "canceled click event on previously-unchecked radio should leave that radio unchecked");
});
test(() => {
const container = document.createElement('div');
container.innerHTML =
'<input type=radio name=n1><span><input type=radio name=n1 checked></span>' +
'<form><input type=radio name=n1 checked></form>';
const radios = container.querySelectorAll('input');
assert_false(radios[0].checked, 'Sanity check: The first radio should be unchecked');
assert_true(radios[1].checked, 'Sanity check: The second radio should be checked');
assert_true(radios[2].checked, 'Sanity check: The third radio should be checked');
radios[0].checked = true;
assert_true(radios[0].checked, 'The first radio should be checked after setting checked');
assert_false(radios[1].checked, 'The second radio should be unchecked after setting checked');
assert_true(radios[2].checked, 'The third radio should be checked after setting checked');
radios[1].required = true;
assert_false(radios[0].validity.valueMissing, 'The first radio should be valid');
assert_false(radios[1].validity.valueMissing, 'The second radio should be valid');
assert_false(radios[2].validity.valueMissing, 'The third radio should be valid');
radios[0].remove();
assert_false(radios[0].validity.valueMissing, 'The first radio should be valid because of no required');
assert_true(radios[1].validity.valueMissing, 'The second radio should be invalid***');
assert_false(radios[2].validity.valueMissing, 'The third radio should be valid');
radios[0].required = true;
radios[0].checked = false;
assert_true(radios[0].validity.valueMissing, 'The first radio should be invalid because of required');
}, 'Radio buttons in an orphan tree should make a group');
</script>
......@@ -38,14 +38,6 @@ 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
......
......@@ -64,25 +64,6 @@ 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');
</script>
</body>
</html>
......@@ -6,7 +6,7 @@ Removing a checked radio button from a required radio button group by a DOM tree
PASS backgroundOf($("radio1")) is validColor
PASS parent.removeChild($("radio2")); backgroundOf($("radio1")) is invalidColor
PASS $("radio1").remove(); radio2.webkitMatchesSelector(":valid") is false
PASS radio2.remove(); radio2.webkitMatchesSelector(":valid") is true
PASS radio2.remove(); radio2.webkitMatchesSelector(":valid") is false
Removing a checked radio button from a required radio button group by name attribute change:
......
......@@ -35,7 +35,7 @@ parent.innerHTML = '<input type=radio name=group1 required checked id=radio1>' +
'<input type=radio name=group1 required id=radio3>';
var radio2 = $('radio2');
shouldBeFalse('$("radio1").remove(); radio2.webkitMatchesSelector(":valid")');
shouldBeTrue('radio2.remove(); radio2.webkitMatchesSelector(":valid")');
shouldBeFalse('radio2.remove(); radio2.webkitMatchesSelector(":valid")');
debug('');
debug('Removing a checked radio button from a required radio button group by name attribute change:');
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment