Fix timing of change events for <select> elements (#37293)

Fixes two silly bugs in the implementation of `<select>` elements.
* We did not fire `input` events
* We fired `change` events *before* updating the selected value

Both of these are only relevant when the embedder selects a value, so
due to our lack of webdriver support we can't test them.

With these changes it is possible to switch the language on
https://toolbox.googleapps.com/apps/main/.

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2025-06-06 16:01:12 +02:00 committed by GitHub
parent 097bd9d87f
commit 32cffbc985
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 61 additions and 20 deletions

View file

@ -1124,7 +1124,13 @@ impl TaskOnce for EventTask {
let target = self.target.root(); let target = self.target.root();
let bubbles = self.bubbles; let bubbles = self.bubbles;
let cancelable = self.cancelable; let cancelable = self.cancelable;
target.fire_event_with_params(self.name, bubbles, cancelable, CanGc::note()); target.fire_event_with_params(
self.name,
bubbles,
cancelable,
EventComposed::NotComposed,
CanGc::note(),
);
} }
} }

View file

@ -59,7 +59,7 @@ use crate::dom::bindings::trace::HashMapTracedValues;
use crate::dom::document::Document; use crate::dom::document::Document;
use crate::dom::element::Element; use crate::dom::element::Element;
use crate::dom::errorevent::ErrorEvent; use crate::dom::errorevent::ErrorEvent;
use crate::dom::event::{Event, EventBubbles, EventCancelable, EventStatus}; use crate::dom::event::{Event, EventBubbles, EventCancelable, EventComposed, EventStatus};
use crate::dom::globalscope::GlobalScope; use crate::dom::globalscope::GlobalScope;
use crate::dom::htmlformelement::FormControlElementHelpers; use crate::dom::htmlformelement::FormControlElementHelpers;
use crate::dom::node::{Node, NodeTraits}; use crate::dom::node::{Node, NodeTraits};
@ -767,6 +767,7 @@ impl EventTarget {
name, name,
EventBubbles::DoesNotBubble, EventBubbles::DoesNotBubble,
EventCancelable::NotCancelable, EventCancelable::NotCancelable,
EventComposed::NotComposed,
can_gc, can_gc,
) )
} }
@ -777,6 +778,7 @@ impl EventTarget {
name, name,
EventBubbles::Bubbles, EventBubbles::Bubbles,
EventCancelable::NotCancelable, EventCancelable::NotCancelable,
EventComposed::NotComposed,
can_gc, can_gc,
) )
} }
@ -787,6 +789,7 @@ impl EventTarget {
name, name,
EventBubbles::DoesNotBubble, EventBubbles::DoesNotBubble,
EventCancelable::Cancelable, EventCancelable::Cancelable,
EventComposed::NotComposed,
can_gc, can_gc,
) )
} }
@ -801,19 +804,22 @@ impl EventTarget {
name, name,
EventBubbles::Bubbles, EventBubbles::Bubbles,
EventCancelable::Cancelable, EventCancelable::Cancelable,
EventComposed::NotComposed,
can_gc, can_gc,
) )
} }
// https://dom.spec.whatwg.org/#concept-event-fire /// <https://dom.spec.whatwg.org/#concept-event-fire>
pub(crate) fn fire_event_with_params( pub(crate) fn fire_event_with_params(
&self, &self,
name: Atom, name: Atom,
bubbles: EventBubbles, bubbles: EventBubbles,
cancelable: EventCancelable, cancelable: EventCancelable,
composed: EventComposed,
can_gc: CanGc, can_gc: CanGc,
) -> DomRoot<Event> { ) -> DomRoot<Event> {
let event = Event::new(&self.global(), name, bubbles, cancelable, can_gc); let event = Event::new(&self.global(), name, bubbles, cancelable, can_gc);
event.set_composed(composed.into());
event.fire(self, can_gc); event.fire(self, can_gc);
event event
} }

View file

@ -16,6 +16,8 @@ use embedder_traits::{SelectElementOptionOrOptgroup, SelectElementOption};
use euclid::{Size2D, Point2D, Rect}; use euclid::{Size2D, Point2D, Rect};
use embedder_traits::{FormControl as EmbedderFormControl, EmbedderMsg}; use embedder_traits::{FormControl as EmbedderFormControl, EmbedderMsg};
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::event::{EventBubbles, EventCancelable, EventComposed};
use crate::dom::bindings::codegen::GenericBindings::HTMLOptGroupElementBinding::HTMLOptGroupElement_Binding::HTMLOptGroupElementMethods; use crate::dom::bindings::codegen::GenericBindings::HTMLOptGroupElementBinding::HTMLOptGroupElement_Binding::HTMLOptGroupElementMethods;
use crate::dom::activation::Activatable; use crate::dom::activation::Activatable;
use crate::dom::attr::Attr; use crate::dom::attr::Attr;
@ -346,13 +348,6 @@ impl HTMLSelectElement {
.SetData(displayed_text.trim().into()); .SetData(displayed_text.trim().into());
} }
pub(crate) fn selection_changed(&self, can_gc: CanGc) {
self.update_shadow_tree(can_gc);
self.upcast::<EventTarget>()
.fire_bubbling_event(atom!("change"), can_gc);
}
pub(crate) fn selected_option(&self) -> Option<DomRoot<HTMLOptionElement>> { pub(crate) fn selected_option(&self) -> Option<DomRoot<HTMLOptionElement>> {
self.list_of_options() self.list_of_options()
.find(|opt_elem| opt_elem.Selected()) .find(|opt_elem| opt_elem.Selected())
@ -417,11 +412,38 @@ impl HTMLSelectElement {
return None; return None;
}; };
if response.is_some() && response != selected_index { response
self.selection_changed(can_gc);
} }
response /// <https://html.spec.whatwg.org/multipage/#send-select-update-notifications>
fn send_update_notifications(&self) {
// > When the user agent is to send select update notifications, queue an element task on the
// > user interaction task source given the select element to run these steps:
let this = Trusted::new(self);
self.owner_global()
.task_manager()
.user_interaction_task_source()
.queue(task!(send_select_update_notification: move || {
let this = this.root();
// TODO: Step 1. Set the select element's user validity to true.
// Step 2. Fire an event named input at the select element, with the bubbles and composed
// attributes initialized to true.
this.upcast::<EventTarget>()
.fire_event_with_params(
atom!("input"),
EventBubbles::Bubbles,
EventCancelable::NotCancelable,
EventComposed::Composed,
CanGc::note(),
);
// Step 3. Fire an event named change at the select element, with the bubbles attribute initialized
// to true.
this.upcast::<EventTarget>()
.fire_bubbling_event(atom!("change"), CanGc::note());
}));
} }
} }
@ -578,22 +600,29 @@ impl HTMLSelectElementMethods<crate::DomTypeHolder> for HTMLSelectElement {
/// <https://html.spec.whatwg.org/multipage/#dom-select-selectedindex> /// <https://html.spec.whatwg.org/multipage/#dom-select-selectedindex>
fn SetSelectedIndex(&self, index: i32, can_gc: CanGc) { fn SetSelectedIndex(&self, index: i32, can_gc: CanGc) {
let mut selection_did_change = false;
let mut opt_iter = self.list_of_options(); let mut opt_iter = self.list_of_options();
for opt in opt_iter.by_ref().take(index as usize) { for opt in opt_iter.by_ref().take(index as usize) {
selection_did_change |= opt.Selected();
opt.set_selectedness(false); opt.set_selectedness(false);
} }
if let Some(opt) = opt_iter.next() { if let Some(selected_option) = opt_iter.next() {
opt.set_selectedness(true); selection_did_change |= !selected_option.Selected();
opt.set_dirtiness(true); selected_option.set_selectedness(true);
selected_option.set_dirtiness(true);
// Reset remaining <option> elements // Reset remaining <option> elements
for opt in opt_iter { for opt in opt_iter {
selection_did_change |= opt.Selected();
opt.set_selectedness(false); opt.set_selectedness(false);
} }
} }
// TODO: Track whether the selected element actually changed if selection_did_change {
self.update_shadow_tree(can_gc); self.update_shadow_tree(can_gc);
} }
}
/// <https://html.spec.whatwg.org/multipage/#dom-cva-willvalidate> /// <https://html.spec.whatwg.org/multipage/#dom-cva-willvalidate>
fn WillValidate(&self) -> bool { fn WillValidate(&self) -> bool {
@ -767,7 +796,6 @@ impl Activatable for HTMLSelectElement {
true true
} }
/// <https://html.spec.whatwg.org/multipage/#input-activation-behavior>
fn activation_behavior(&self, _event: &Event, _target: &EventTarget, can_gc: CanGc) { fn activation_behavior(&self, _event: &Event, _target: &EventTarget, can_gc: CanGc) {
let Some(selected_value) = self.show_menu(can_gc) else { let Some(selected_value) = self.show_menu(can_gc) else {
// The user did not select a value // The user did not select a value
@ -775,6 +803,7 @@ impl Activatable for HTMLSelectElement {
}; };
self.SetSelectedIndex(selected_value as i32, can_gc); self.SetSelectedIndex(selected_value as i32, can_gc);
self.send_update_notifications();
} }
} }