From ccd3a19944039ce5bd0536e5b7f58efdf5ae75df Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Sat, 18 May 2024 05:45:13 +0100 Subject: [PATCH 1/4] LibWeb: Deduplicate the firing of input events in HTMLInputElement Input elements without a defined user-interaction behavior need to fire an input event when the user changes the element's value in some way. This change moves the code to do this into its own function and adds some spec text to explain what is being done. --- .../LibWeb/HTML/HTMLInputElement.cpp | 63 ++++++++----------- .../Libraries/LibWeb/HTML/HTMLInputElement.h | 2 + 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index a2378476d0a819..b7fe7bbe9f1412 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -391,22 +391,11 @@ void HTMLInputElement::did_edit_text_node(Badge) update_placeholder_visibility(); - // NOTE: This is a bit ad-hoc, but basically implements part of "4.10.5.5 Common event behaviors" - // https://html.spec.whatwg.org/multipage/input.html#common-input-element-events - queue_an_element_task(HTML::Task::Source::UserInteraction, [this] { - auto input_event = DOM::Event::create(realm(), HTML::EventNames::input); - input_event->set_bubbles(true); - input_event->set_composed(true); - dispatch_event(*input_event); - }); + user_interaction_did_change_input_value(); } void HTMLInputElement::did_pick_color(Optional picked_color, ColorPickerUpdateState state) { - // https://html.spec.whatwg.org/multipage/input.html#common-input-element-events - // For input elements without a defined input activation behavior, but to which these events apply - // and for which the user interface involves both interactive manipulation and an explicit commit action - if (type_state() == TypeAttributeState::Color && picked_color.has_value()) { // then when the user changes the element's value m_value = value_sanitization_algorithm(picked_color.value().to_string_without_alpha()); @@ -415,15 +404,10 @@ void HTMLInputElement::did_pick_color(Optional picked_color, ColorPickerU update_color_well_element(); // the user agent must queue an element task on the user interaction task source - queue_an_element_task(HTML::Task::Source::UserInteraction, [this] { - // given the input element to fire an event named input at the input element, with the bubbles and composed attributes initialized to true - auto input_event = DOM::Event::create(realm(), HTML::EventNames::input); - input_event->set_bubbles(true); - input_event->set_composed(true); - dispatch_event(*input_event); - }); + user_interaction_did_change_input_value(); - // and any time the user commits the change, the user agent must queue an element task on the user interaction task source + // https://html.spec.whatwg.org/multipage/input.html#common-input-element-events + // [...] any time the user commits the change, the user agent must queue an element task on the user interaction task source if (state == ColorPickerUpdateState::Closed) { queue_an_element_task(HTML::Task::Source::UserInteraction, [this] { // given the input element @@ -951,18 +935,8 @@ void HTMLInputElement::create_range_input_shadow_tree() MUST(slider_runnable_track->append_child(*m_slider_thumb)); update_slider_thumb_element(); - //  User event listeners - auto dispatch_input_event = [this]() { - queue_an_element_task(HTML::Task::Source::UserInteraction, [&] { - auto input_event = DOM::Event::create(realm(), HTML::EventNames::input); - input_event->set_bubbles(true); - input_event->set_composed(true); - dispatch_event(*input_event); - }); - }; - auto keydown_callback_function = JS::NativeFunction::create( - realm(), [this, dispatch_input_event](JS::VM& vm) { + realm(), [this](JS::VM& vm) { auto key = MUST(vm.argument(0).get(vm, "key")).as_string().utf8_string(); if (key == "ArrowLeft" || key == "ArrowDown") @@ -975,7 +949,7 @@ void HTMLInputElement::create_range_input_shadow_tree() if (key == "PageUp") MUST(step_up(10)); - dispatch_input_event(); + user_interaction_did_change_input_value(); return JS::js_undefined(); }, 0, "", &realm()); @@ -983,28 +957,28 @@ void HTMLInputElement::create_range_input_shadow_tree() add_event_listener_without_options(UIEvents::EventNames::keydown, DOM::IDLEventListener::create(realm(), keydown_callback)); auto wheel_callback_function = JS::NativeFunction::create( - realm(), [this, dispatch_input_event](JS::VM& vm) { + realm(), [this](JS::VM& vm) { auto deltaY = MUST(vm.argument(0).get(vm, "deltaY")).as_i32(); if (deltaY > 0) { MUST(step_down()); } else { MUST(step_up()); } - dispatch_input_event(); + user_interaction_did_change_input_value(); return JS::js_undefined(); }, 0, "", &realm()); auto wheel_callback = realm().heap().allocate_without_realm(*wheel_callback_function, Bindings::host_defined_environment_settings_object(realm())); add_event_listener_without_options(UIEvents::EventNames::wheel, DOM::IDLEventListener::create(realm(), wheel_callback)); - auto update_slider_by_mouse = [this, dispatch_input_event](JS::VM& vm) { + auto update_slider_by_mouse = [this](JS::VM& vm) { auto client_x = MUST(vm.argument(0).get(vm, "clientX")).as_double(); auto rect = get_bounding_client_rect(); double minimum = *min(); double maximum = *max(); // FIXME: Snap new value to input steps MUST(set_value_as_number(clamp(round(((client_x - rect->left()) / rect->width()) * (maximum - minimum) + minimum), minimum, maximum))); - dispatch_input_event(); + user_interaction_did_change_input_value(); }; auto mousedown_callback_function = JS::NativeFunction::create( @@ -1041,6 +1015,23 @@ void HTMLInputElement::create_range_input_shadow_tree() add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), mousedown_callback)); } +void HTMLInputElement::user_interaction_did_change_input_value() +{ + // https://html.spec.whatwg.org/multipage/input.html#common-input-element-events + // For input elements without a defined input activation behavior, but to which these events apply, + // and for which the user interface involves both interactive manipulation and an explicit commit action, + // then when the user changes the element's value, the user agent must queue an element task on the user interaction task source + // given the input element to fire an event named input at the input element, with the bubbles and composed attributes initialized to true, + // and any time the user commits the change, the user agent must queue an element task on the user interaction task source given the input + // element to set its user validity to true and fire an event named change at the input element, with the bubbles attribute initialized to true. + queue_an_element_task(HTML::Task::Source::UserInteraction, [this] { + auto input_event = DOM::Event::create(realm(), HTML::EventNames::input); + input_event->set_bubbles(true); + input_event->set_composed(true); + dispatch_event(*input_event); + }); +} + void HTMLInputElement::update_slider_thumb_element() { if (!m_slider_thumb) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h index a9dca6c56c33d3..c17beb0580e24b 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h @@ -252,6 +252,8 @@ class HTMLInputElement final void handle_readonly_attribute(Optional const& value); WebIDL::ExceptionOr handle_src_attribute(String const& value); + void user_interaction_did_change_input_value(); + // https://html.spec.whatwg.org/multipage/input.html#value-sanitization-algorithm String value_sanitization_algorithm(String const&) const; From d737a6cfea2cb984789f7fdf4627023918cb2ce0 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Sat, 18 May 2024 05:45:13 +0100 Subject: [PATCH 2/4] LibWeb: Fire input event on user interaction with input element buttons An input event is now fired when the step up or step down button of an input element of type number is clicked. This ensures that any associated element is updated when these buttons are clicked. --- Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index b7fe7bbe9f1412..a2edc46502bd4d 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -811,6 +811,7 @@ void HTMLInputElement::create_text_input_shadow_tree() auto up_callback_function = JS::NativeFunction::create( realm(), [this](JS::VM&) { MUST(step_up()); + user_interaction_did_change_input_value(); return JS::js_undefined(); }, 0, "", &realm()); @@ -829,6 +830,7 @@ void HTMLInputElement::create_text_input_shadow_tree() auto down_callback_function = JS::NativeFunction::create( realm(), [this](JS::VM&) { MUST(step_down()); + user_interaction_did_change_input_value(); return JS::js_undefined(); }, 0, "", &realm()); From c6ed79d26032b565a37a82b59eff9eda69c9f286 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Wed, 22 May 2024 21:36:30 +0100 Subject: [PATCH 3/4] LibWeb: Update number input on mousedown of number input buttons This matches the behavior of other browsers. Previously, a click event was used, so the value was only updated when the mouse was released. --- Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index a2edc46502bd4d..df1b776180673d 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -816,7 +816,7 @@ void HTMLInputElement::create_text_input_shadow_tree() }, 0, "", &realm()); auto up_callback = realm().heap().allocate_without_realm(*up_callback_function, Bindings::host_defined_environment_settings_object(realm())); - up_button->add_event_listener_without_options(UIEvents::EventNames::click, DOM::IDLEventListener::create(realm(), up_callback)); + up_button->add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), up_callback)); // Down button auto down_button = MUST(DOM::create_element(document(), HTML::TagNames::button, Namespace::HTML)); @@ -835,7 +835,7 @@ void HTMLInputElement::create_text_input_shadow_tree() }, 0, "", &realm()); auto down_callback = realm().heap().allocate_without_realm(*down_callback_function, Bindings::host_defined_environment_settings_object(realm())); - down_button->add_event_listener_without_options(UIEvents::EventNames::click, DOM::IDLEventListener::create(realm(), down_callback)); + down_button->add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), down_callback)); } } From 3b1d83ab9a450600961b30241e3e5157239261e8 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Wed, 22 May 2024 21:52:10 +0100 Subject: [PATCH 4/4] LibWeb: Fire a change event on mouseup of number input buttons This matches the behavior of other browsers. --- .../LibWeb/HTML/HTMLInputElement.cpp | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index df1b776180673d..259199fe2adc02 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -808,6 +808,16 @@ void HTMLInputElement::create_text_input_shadow_tree() MUST(up_button->set_inner_html(""sv)); MUST(element->append_child(up_button)); + auto mouseup_callback_function = JS::NativeFunction::create( + realm(), [this](JS::VM&) { + commit_pending_changes(); + return JS::js_undefined(); + }, + 0, "", &realm()); + auto mouseup_callback = realm().heap().allocate_without_realm(*mouseup_callback_function, Bindings::host_defined_environment_settings_object(realm())); + DOM::AddEventListenerOptions mouseup_listener_options; + mouseup_listener_options.once = true; + auto up_callback_function = JS::NativeFunction::create( realm(), [this](JS::VM&) { MUST(step_up()); @@ -815,8 +825,9 @@ void HTMLInputElement::create_text_input_shadow_tree() return JS::js_undefined(); }, 0, "", &realm()); - auto up_callback = realm().heap().allocate_without_realm(*up_callback_function, Bindings::host_defined_environment_settings_object(realm())); - up_button->add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), up_callback)); + auto step_up_callback = realm().heap().allocate_without_realm(*up_callback_function, Bindings::host_defined_environment_settings_object(realm())); + up_button->add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), step_up_callback)); + up_button->add_event_listener_without_options(UIEvents::EventNames::mouseup, DOM::IDLEventListener::create(realm(), mouseup_callback)); // Down button auto down_button = MUST(DOM::create_element(document(), HTML::TagNames::button, Namespace::HTML)); @@ -834,8 +845,9 @@ void HTMLInputElement::create_text_input_shadow_tree() return JS::js_undefined(); }, 0, "", &realm()); - auto down_callback = realm().heap().allocate_without_realm(*down_callback_function, Bindings::host_defined_environment_settings_object(realm())); - down_button->add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), down_callback)); + auto step_down_callback = realm().heap().allocate_without_realm(*down_callback_function, Bindings::host_defined_environment_settings_object(realm())); + down_button->add_event_listener_without_options(UIEvents::EventNames::mousedown, DOM::IDLEventListener::create(realm(), step_down_callback)); + down_button->add_event_listener_without_options(UIEvents::EventNames::mouseup, DOM::IDLEventListener::create(realm(), mouseup_callback)); } }