mirror of
https://github.com/servo/servo.git
synced 2025-07-24 15:50:21 +01:00
Fix radio group validity update when removing or selecting an input (#36252)
This PR fixes an issue where radio inputs in the same group failed to correctly update their `validity.valueMissing` state when: - A **checked radio button was removed** from the DOM. - A **different radio button was selected** by user interaction. This behavior caused mismatches with how browsers like Firefox handle radio group validation. --- ### Changes in This PR #### Radio group revalidation on DOM removal - Updated `unbind_from_tree()` to revalidate other radio buttons in the same group when a checked input is removed. - Uses `UnbindContext::parent` as the DOM root to ensure the correct context is used during traversal. #### New helper: `find_related_radios()` - Encapsulates logic for finding other inputs in the same group. - Used during both removal and attribute changes for consistency. #### Validation on `checked`/`value` updates - Introduced `update_related_validity_states()` to revalidate all group members when a radio's `checked` or `value` is changed. #### Web Platform Test (WPT) coverage - Created a new WPT file: `radio-group-valueMissing.html`. - Tests follow recommended `test()` pattern: - **Precondition**: Assert initial `valueMissing`. - **Action**: Remove or select a radio. - **Postcondition**: Assert expected `valueMissing`. #### Manifest updated - The WPT manifest now includes the new test. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #36110 <!-- Either: --> - [X] There are tests for these changes Signed-off-by: Emmanuel Elom <elomemmanuel007@gmail.com>
This commit is contained in:
parent
0caa271176
commit
1f558a0d49
6 changed files with 157 additions and 38 deletions
|
@ -16,12 +16,17 @@ pub(crate) trait Activatable {
|
||||||
fn is_instance_activatable(&self) -> bool;
|
fn is_instance_activatable(&self) -> bool;
|
||||||
|
|
||||||
// https://dom.spec.whatwg.org/#eventtarget-legacy-pre-activation-behavior
|
// https://dom.spec.whatwg.org/#eventtarget-legacy-pre-activation-behavior
|
||||||
fn legacy_pre_activation_behavior(&self) -> Option<InputActivationState> {
|
fn legacy_pre_activation_behavior(&self, _can_gc: CanGc) -> Option<InputActivationState> {
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://dom.spec.whatwg.org/#eventtarget-legacy-canceled-activation-behavior
|
// https://dom.spec.whatwg.org/#eventtarget-legacy-canceled-activation-behavior
|
||||||
fn legacy_canceled_activation_behavior(&self, _state_before: Option<InputActivationState>) {}
|
fn legacy_canceled_activation_behavior(
|
||||||
|
&self,
|
||||||
|
_state_before: Option<InputActivationState>,
|
||||||
|
_can_gc: CanGc,
|
||||||
|
) {
|
||||||
|
}
|
||||||
|
|
||||||
// https://dom.spec.whatwg.org/#eventtarget-activation-behavior
|
// https://dom.spec.whatwg.org/#eventtarget-activation-behavior
|
||||||
// event and target are used only by HTMLAnchorElement, in the case
|
// event and target are used only by HTMLAnchorElement, in the case
|
||||||
|
|
|
@ -507,7 +507,7 @@ impl Event {
|
||||||
// corresponding pre-activation behavior.
|
// corresponding pre-activation behavior.
|
||||||
pre_activation_result = activation_target
|
pre_activation_result = activation_target
|
||||||
.as_maybe_activatable()
|
.as_maybe_activatable()
|
||||||
.and_then(|activatable| activatable.legacy_pre_activation_behavior());
|
.and_then(|activatable| activatable.legacy_pre_activation_behavior(can_gc));
|
||||||
}
|
}
|
||||||
|
|
||||||
let timeline_window = DomRoot::downcast::<Window>(target.global())
|
let timeline_window = DomRoot::downcast::<Window>(target.global())
|
||||||
|
@ -623,7 +623,7 @@ impl Event {
|
||||||
// Step 11.2 Otherwise, if activationTarget has legacy-canceled-activation behavior, then run
|
// Step 11.2 Otherwise, if activationTarget has legacy-canceled-activation behavior, then run
|
||||||
// activationTarget’s legacy-canceled-activation behavior.
|
// activationTarget’s legacy-canceled-activation behavior.
|
||||||
else {
|
else {
|
||||||
activatable.legacy_canceled_activation_behavior(pre_activation_result);
|
activatable.legacy_canceled_activation_behavior(pre_activation_result, can_gc);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -1276,7 +1276,7 @@ impl HTMLFormElement {
|
||||||
NodeTypeId::Element(ElementTypeId::HTMLElement(
|
NodeTypeId::Element(ElementTypeId::HTMLElement(
|
||||||
HTMLElementTypeId::HTMLInputElement,
|
HTMLElementTypeId::HTMLInputElement,
|
||||||
)) => {
|
)) => {
|
||||||
child.downcast::<HTMLInputElement>().unwrap().reset();
|
child.downcast::<HTMLInputElement>().unwrap().reset(can_gc);
|
||||||
},
|
},
|
||||||
NodeTypeId::Element(ElementTypeId::HTMLElement(
|
NodeTypeId::Element(ElementTypeId::HTMLElement(
|
||||||
HTMLElementTypeId::HTMLSelectElement,
|
HTMLElementTypeId::HTMLSelectElement,
|
||||||
|
|
|
@ -815,7 +815,16 @@ impl HTMLInputElement {
|
||||||
}
|
}
|
||||||
let mut is_required = self.Required();
|
let mut is_required = self.Required();
|
||||||
let mut is_checked = self.Checked();
|
let mut is_checked = self.Checked();
|
||||||
for other in radio_group_iter(self, self.radio_group_name().as_ref()) {
|
let root = self
|
||||||
|
.upcast::<Node>()
|
||||||
|
.GetRootNode(&GetRootNodeOptions::empty());
|
||||||
|
let form = self.form_owner();
|
||||||
|
for other in radio_group_iter(
|
||||||
|
self,
|
||||||
|
self.radio_group_name().as_ref(),
|
||||||
|
form.as_deref(),
|
||||||
|
&root,
|
||||||
|
) {
|
||||||
is_required = is_required || other.Required();
|
is_required = is_required || other.Required();
|
||||||
is_checked = is_checked || other.Checked();
|
is_checked = is_checked || other.Checked();
|
||||||
}
|
}
|
||||||
|
@ -1339,8 +1348,7 @@ impl HTMLInputElementMethods<crate::DomTypeHolder> for HTMLInputElement {
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
self.validity_state()
|
update_related_validity_states(self, can_gc);
|
||||||
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
|
||||||
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
|
self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
@ -1684,26 +1692,49 @@ impl HTMLInputElementMethods<crate::DomTypeHolder> for HTMLInputElement {
|
||||||
fn radio_group_iter<'a>(
|
fn radio_group_iter<'a>(
|
||||||
elem: &'a HTMLInputElement,
|
elem: &'a HTMLInputElement,
|
||||||
group: Option<&'a Atom>,
|
group: Option<&'a Atom>,
|
||||||
|
form: Option<&'a HTMLFormElement>,
|
||||||
|
root: &'a Node,
|
||||||
) -> impl Iterator<Item = DomRoot<HTMLInputElement>> + 'a {
|
) -> impl Iterator<Item = DomRoot<HTMLInputElement>> + 'a {
|
||||||
let owner = elem.form_owner();
|
|
||||||
let root = elem
|
|
||||||
.upcast::<Node>()
|
|
||||||
.GetRootNode(&GetRootNodeOptions::empty());
|
|
||||||
|
|
||||||
// If group is None, in_same_group always fails, but we need to always return elem.
|
|
||||||
root.traverse_preorder(ShadowIncluding::No)
|
root.traverse_preorder(ShadowIncluding::No)
|
||||||
.filter_map(DomRoot::downcast::<HTMLInputElement>)
|
.filter_map(DomRoot::downcast::<HTMLInputElement>)
|
||||||
.filter(move |r| &**r == elem || in_same_group(r, owner.as_deref(), group, None))
|
.filter(move |r| &**r == elem || in_same_group(r, form, group, None))
|
||||||
}
|
}
|
||||||
|
|
||||||
fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&Atom>) {
|
fn broadcast_radio_checked(broadcaster: &HTMLInputElement, group: Option<&Atom>) {
|
||||||
for r in radio_group_iter(broadcaster, group) {
|
let root = broadcaster
|
||||||
|
.upcast::<Node>()
|
||||||
|
.GetRootNode(&GetRootNodeOptions::empty());
|
||||||
|
let form = broadcaster.form_owner();
|
||||||
|
for r in radio_group_iter(broadcaster, group, form.as_deref(), &root) {
|
||||||
if broadcaster != &*r && r.Checked() {
|
if broadcaster != &*r && r.Checked() {
|
||||||
r.SetChecked(false);
|
r.SetChecked(false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn perform_radio_group_validation(elem: &HTMLInputElement, group: Option<&Atom>, can_gc: CanGc) {
|
||||||
|
let root = elem
|
||||||
|
.upcast::<Node>()
|
||||||
|
.GetRootNode(&GetRootNodeOptions::empty());
|
||||||
|
let form = elem.form_owner();
|
||||||
|
for r in radio_group_iter(elem, group, form.as_deref(), &root) {
|
||||||
|
r.validity_state()
|
||||||
|
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn update_related_validity_states(elem: &HTMLInputElement, can_gc: CanGc) {
|
||||||
|
match elem.input_type() {
|
||||||
|
InputType::Radio => {
|
||||||
|
perform_radio_group_validation(elem, elem.radio_group_name().as_ref(), can_gc)
|
||||||
|
},
|
||||||
|
_ => {
|
||||||
|
elem.validity_state()
|
||||||
|
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// https://html.spec.whatwg.org/multipage/#radio-button-group
|
// https://html.spec.whatwg.org/multipage/#radio-button-group
|
||||||
fn in_same_group(
|
fn in_same_group(
|
||||||
other: &HTMLInputElement,
|
other: &HTMLInputElement,
|
||||||
|
@ -1868,11 +1899,12 @@ impl HTMLInputElement {
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://html.spec.whatwg.org/multipage/#the-input-element:concept-form-reset-control
|
// https://html.spec.whatwg.org/multipage/#the-input-element:concept-form-reset-control
|
||||||
pub(crate) fn reset(&self) {
|
pub(crate) fn reset(&self, can_gc: CanGc) {
|
||||||
match self.input_type() {
|
match self.input_type() {
|
||||||
InputType::Radio | InputType::Checkbox => {
|
InputType::Radio | InputType::Checkbox => {
|
||||||
self.update_checked_state(self.DefaultChecked(), false);
|
self.update_checked_state(self.DefaultChecked(), false);
|
||||||
self.checked_changed.set(false);
|
self.checked_changed.set(false);
|
||||||
|
update_related_validity_states(self, can_gc);
|
||||||
},
|
},
|
||||||
InputType::Image => (),
|
InputType::Image => (),
|
||||||
_ => (),
|
_ => (),
|
||||||
|
@ -2520,8 +2552,7 @@ impl VirtualMethods for HTMLInputElement {
|
||||||
_ => {},
|
_ => {},
|
||||||
}
|
}
|
||||||
|
|
||||||
self.validity_state()
|
update_related_validity_states(self, can_gc);
|
||||||
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
|
fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
|
||||||
|
@ -2549,13 +2580,12 @@ impl VirtualMethods for HTMLInputElement {
|
||||||
self.upcast::<Element>()
|
self.upcast::<Element>()
|
||||||
.check_ancestors_disabled_state_for_form_control();
|
.check_ancestors_disabled_state_for_form_control();
|
||||||
|
|
||||||
for r in radio_group_iter(self, self.radio_group_name().as_ref()) {
|
update_related_validity_states(self, can_gc);
|
||||||
r.validity_state()
|
|
||||||
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn unbind_from_tree(&self, context: &UnbindContext, can_gc: CanGc) {
|
fn unbind_from_tree(&self, context: &UnbindContext, can_gc: CanGc) {
|
||||||
|
let form_owner = self.form_owner();
|
||||||
|
|
||||||
self.super_type().unwrap().unbind_from_tree(context, can_gc);
|
self.super_type().unwrap().unbind_from_tree(context, can_gc);
|
||||||
|
|
||||||
let node = self.upcast::<Node>();
|
let node = self.upcast::<Node>();
|
||||||
|
@ -2569,6 +2599,19 @@ impl VirtualMethods for HTMLInputElement {
|
||||||
el.check_disabled_attribute();
|
el.check_disabled_attribute();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if self.input_type() == InputType::Radio {
|
||||||
|
let root = context.parent.GetRootNode(&GetRootNodeOptions::empty());
|
||||||
|
for r in radio_group_iter(
|
||||||
|
self,
|
||||||
|
self.radio_group_name().as_ref(),
|
||||||
|
form_owner.as_deref(),
|
||||||
|
&root,
|
||||||
|
) {
|
||||||
|
r.validity_state()
|
||||||
|
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
self.validity_state()
|
self.validity_state()
|
||||||
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
||||||
}
|
}
|
||||||
|
@ -2680,8 +2723,7 @@ impl VirtualMethods for HTMLInputElement {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
self.validity_state()
|
update_related_validity_states(self, can_gc);
|
||||||
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://html.spec.whatwg.org/multipage/#the-input-element%3Aconcept-node-clone-ext
|
// https://html.spec.whatwg.org/multipage/#the-input-element%3Aconcept-node-clone-ext
|
||||||
|
@ -2703,8 +2745,7 @@ impl VirtualMethods for HTMLInputElement {
|
||||||
elem.textinput
|
elem.textinput
|
||||||
.borrow_mut()
|
.borrow_mut()
|
||||||
.set_content(self.textinput.borrow().get_content());
|
.set_content(self.textinput.borrow().get_content());
|
||||||
elem.validity_state()
|
update_related_validity_states(self, can_gc);
|
||||||
.perform_validation_and_update(ValidationFlags::all(), can_gc);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2819,40 +2860,58 @@ impl Activatable for HTMLInputElement {
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://dom.spec.whatwg.org/#eventtarget-legacy-pre-activation-behavior
|
// https://dom.spec.whatwg.org/#eventtarget-legacy-pre-activation-behavior
|
||||||
fn legacy_pre_activation_behavior(&self) -> Option<InputActivationState> {
|
fn legacy_pre_activation_behavior(&self, can_gc: CanGc) -> Option<InputActivationState> {
|
||||||
let ty = self.input_type();
|
let ty = self.input_type();
|
||||||
match ty {
|
let activation_state = match ty {
|
||||||
InputType::Checkbox => {
|
InputType::Checkbox => {
|
||||||
let was_checked = self.Checked();
|
let was_checked = self.Checked();
|
||||||
let was_indeterminate = self.Indeterminate();
|
let was_indeterminate = self.Indeterminate();
|
||||||
self.SetIndeterminate(false);
|
self.SetIndeterminate(false);
|
||||||
self.SetChecked(!was_checked);
|
self.SetChecked(!was_checked);
|
||||||
return Some(InputActivationState {
|
Some(InputActivationState {
|
||||||
checked: was_checked,
|
checked: was_checked,
|
||||||
indeterminate: was_indeterminate,
|
indeterminate: was_indeterminate,
|
||||||
checked_radio: None,
|
checked_radio: None,
|
||||||
old_type: InputType::Checkbox,
|
old_type: InputType::Checkbox,
|
||||||
});
|
})
|
||||||
},
|
},
|
||||||
InputType::Radio => {
|
InputType::Radio => {
|
||||||
let checked_member =
|
let root = self
|
||||||
radio_group_iter(self, self.radio_group_name().as_ref()).find(|r| r.Checked());
|
.upcast::<Node>()
|
||||||
|
.GetRootNode(&GetRootNodeOptions::empty());
|
||||||
|
let form_owner = self.form_owner();
|
||||||
|
let checked_member = radio_group_iter(
|
||||||
|
self,
|
||||||
|
self.radio_group_name().as_ref(),
|
||||||
|
form_owner.as_deref(),
|
||||||
|
&root,
|
||||||
|
)
|
||||||
|
.find(|r| r.Checked());
|
||||||
let was_checked = self.Checked();
|
let was_checked = self.Checked();
|
||||||
self.SetChecked(true);
|
self.SetChecked(true);
|
||||||
return Some(InputActivationState {
|
Some(InputActivationState {
|
||||||
checked: was_checked,
|
checked: was_checked,
|
||||||
indeterminate: false,
|
indeterminate: false,
|
||||||
checked_radio: checked_member.as_deref().map(DomRoot::from_ref),
|
checked_radio: checked_member.as_deref().map(DomRoot::from_ref),
|
||||||
old_type: InputType::Radio,
|
old_type: InputType::Radio,
|
||||||
});
|
})
|
||||||
},
|
},
|
||||||
_ => (),
|
_ => None,
|
||||||
|
};
|
||||||
|
|
||||||
|
if activation_state.is_some() {
|
||||||
|
update_related_validity_states(self, can_gc);
|
||||||
}
|
}
|
||||||
None
|
|
||||||
|
activation_state
|
||||||
}
|
}
|
||||||
|
|
||||||
// https://dom.spec.whatwg.org/#eventtarget-legacy-canceled-activation-behavior
|
// https://dom.spec.whatwg.org/#eventtarget-legacy-canceled-activation-behavior
|
||||||
fn legacy_canceled_activation_behavior(&self, cache: Option<InputActivationState>) {
|
fn legacy_canceled_activation_behavior(
|
||||||
|
&self,
|
||||||
|
cache: Option<InputActivationState>,
|
||||||
|
can_gc: CanGc,
|
||||||
|
) {
|
||||||
// Step 1
|
// Step 1
|
||||||
let ty = self.input_type();
|
let ty = self.input_type();
|
||||||
let cache = match cache {
|
let cache = match cache {
|
||||||
|
@ -2899,6 +2958,8 @@ impl Activatable for HTMLInputElement {
|
||||||
},
|
},
|
||||||
_ => (),
|
_ => (),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
update_related_validity_states(self, can_gc);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <https://html.spec.whatwg.org/multipage/#input-activation-behavior>
|
/// <https://html.spec.whatwg.org/multipage/#input-activation-behavior>
|
||||||
|
|
7
tests/wpt/meta/MANIFEST.json
vendored
7
tests/wpt/meta/MANIFEST.json
vendored
|
@ -728363,6 +728363,13 @@
|
||||||
{}
|
{}
|
||||||
]
|
]
|
||||||
],
|
],
|
||||||
|
"radio-group-valueMissing.html": [
|
||||||
|
"5f14c0b82e15c2e1ad259d47b970e4b19e7b3f0e",
|
||||||
|
[
|
||||||
|
null,
|
||||||
|
{}
|
||||||
|
]
|
||||||
|
],
|
||||||
"radio-valueMissing.html": [
|
"radio-valueMissing.html": [
|
||||||
"a02b6b96454f28c7ebd6ebfbedd604b67e8b1d77",
|
"a02b6b96454f28c7ebd6ebfbedd604b67e8b1d77",
|
||||||
[
|
[
|
||||||
|
|
46
tests/wpt/tests/html/semantics/forms/constraints/radio-group-valueMissing.html
vendored
Normal file
46
tests/wpt/tests/html/semantics/forms/constraints/radio-group-valueMissing.html
vendored
Normal file
|
@ -0,0 +1,46 @@
|
||||||
|
<!DOCTYPE html>
|
||||||
|
<meta charset="utf-8" />
|
||||||
|
<title>
|
||||||
|
The constraint validation API Test: element.validity.valueMissing for radio
|
||||||
|
group
|
||||||
|
</title>
|
||||||
|
<link rel="author" title="Emmanuel Elom" href="mailto:elomemmanuel007@gmail.com">
|
||||||
|
<link
|
||||||
|
rel="help"
|
||||||
|
href="https://html.spec.whatwg.org/multipage/#dom-validitystate-valuemissing"
|
||||||
|
/>
|
||||||
|
<link
|
||||||
|
rel="help"
|
||||||
|
href="https://html.spec.whatwg.org/multipage/#the-constraint-validation-api"
|
||||||
|
/>
|
||||||
|
<script src="/resources/testharness.js"></script>
|
||||||
|
<script src="/resources/testharnessreport.js"></script>
|
||||||
|
<script src="support/validator.js"></script>
|
||||||
|
<div id="log"></div>
|
||||||
|
|
||||||
|
<input type="radio" id="first" required name="group" />
|
||||||
|
<input type="radio" id="second" checked name="group" />
|
||||||
|
<input type="radio" id="third" required name="group1" />
|
||||||
|
<input type="radio" id="fourth" name="group1" />
|
||||||
|
|
||||||
|
<script>
|
||||||
|
const first = document.getElementById("first");
|
||||||
|
const second = document.getElementById("second");
|
||||||
|
const third = document.getElementById("third");
|
||||||
|
const fourth = document.getElementById("fourth");
|
||||||
|
|
||||||
|
test(() => {
|
||||||
|
assert_equals(first.validity.valueMissing, false);
|
||||||
|
assert_equals(second.validity.valueMissing, false);
|
||||||
|
second.remove();
|
||||||
|
assert_equals(first.validity.valueMissing, true);
|
||||||
|
}, "valueMissing is true for all group members when checked group member is removed");
|
||||||
|
|
||||||
|
test(() => {
|
||||||
|
assert_equals(third.validity.valueMissing, true);
|
||||||
|
assert_equals(fourth.validity.valueMissing, true);
|
||||||
|
fourth.click();
|
||||||
|
assert_equals(third.validity.valueMissing, false);
|
||||||
|
assert_equals(fourth.validity.valueMissing, false);
|
||||||
|
}, "valueMissing is false for all group members when any group member is checked");
|
||||||
|
</script>
|
Loading…
Add table
Add a link
Reference in a new issue