From 5f0f97b3cc3daa7181dc6bad55c32af1eef82ae0 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 21 Mar 2025 11:35:42 -0400 Subject: [PATCH] LibWeb: Do not insert "return" key presses into input element values When the return key is pressed, we try to handle it as a commit action for input elements. However, we would then go on to actually insert the return key's code point (U+000D) into the input element. This would be sanitized out, but would leave the input element in a state where it thinks it has text to commit. This would result in a change event being fired when the return key is pressed multiple times in a row. We were also firing the beforeinput/input events twice for all return key presses. To fix this, this patch changes the input event target to signify if it actually handled the return key. If not (i.e. for textarea elements), only then do we insert the code point. We also must not fall through to the generic key handler, to avoid the repeated input events. --- Libraries/LibWeb/DOM/EditingHostManager.cpp | 3 ++- Libraries/LibWeb/DOM/EditingHostManager.h | 2 +- Libraries/LibWeb/DOM/InputEventsTarget.h | 3 ++- .../LibWeb/HTML/FormAssociatedElement.cpp | 21 ++++++++++--------- Libraries/LibWeb/HTML/FormAssociatedElement.h | 2 +- Libraries/LibWeb/Internals/Internals.cpp | 2 +- Libraries/LibWeb/Page/EventHandler.cpp | 6 +++++- Tests/LibWeb/Text/input/input-commit.html | 3 +++ 8 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Libraries/LibWeb/DOM/EditingHostManager.cpp b/Libraries/LibWeb/DOM/EditingHostManager.cpp index 0e1f20e7c9d..6e2e341753a 100644 --- a/Libraries/LibWeb/DOM/EditingHostManager.cpp +++ b/Libraries/LibWeb/DOM/EditingHostManager.cpp @@ -190,9 +190,10 @@ void EditingHostManager::handle_delete(DeleteDirection direction) MUST(selection_range->delete_contents()); } -void EditingHostManager::handle_return_key() +EventResult EditingHostManager::handle_return_key() { dbgln("FIXME: Implement EditingHostManager::handle_return_key()"); + return EventResult::Dropped; } } diff --git a/Libraries/LibWeb/DOM/EditingHostManager.h b/Libraries/LibWeb/DOM/EditingHostManager.h index dd6dc179bf5..fe629dcb60c 100644 --- a/Libraries/LibWeb/DOM/EditingHostManager.h +++ b/Libraries/LibWeb/DOM/EditingHostManager.h @@ -25,7 +25,7 @@ public: virtual void handle_insert(String const&) override; virtual void handle_delete(DeleteDirection) override; - virtual void handle_return_key() override; + virtual EventResult handle_return_key() override; virtual void select_all() override; virtual void set_selection_anchor(GC::Ref, size_t offset) override; virtual void set_selection_focus(GC::Ref, size_t offset) override; diff --git a/Libraries/LibWeb/DOM/InputEventsTarget.h b/Libraries/LibWeb/DOM/InputEventsTarget.h index 58b007fb5ce..668699fbcba 100644 --- a/Libraries/LibWeb/DOM/InputEventsTarget.h +++ b/Libraries/LibWeb/DOM/InputEventsTarget.h @@ -8,6 +8,7 @@ #include #include +#include namespace Web { @@ -18,7 +19,7 @@ public: virtual GC::Ref as_cell() = 0; virtual void handle_insert(String const&) = 0; - virtual void handle_return_key() = 0; + virtual EventResult handle_return_key() = 0; enum class DeleteDirection { Backward, diff --git a/Libraries/LibWeb/HTML/FormAssociatedElement.cpp b/Libraries/LibWeb/HTML/FormAssociatedElement.cpp index 3bc1cd8f39d..4f597becb14 100644 --- a/Libraries/LibWeb/HTML/FormAssociatedElement.cpp +++ b/Libraries/LibWeb/HTML/FormAssociatedElement.cpp @@ -781,17 +781,18 @@ void FormAssociatedTextControlElement::handle_delete(DeleteDirection direction) MUST(set_range_text(String {}, selection_start, selection_end, Bindings::SelectionMode::End)); } -void FormAssociatedTextControlElement::handle_return_key() +EventResult FormAssociatedTextControlElement::handle_return_key() { - auto& html_element = form_associated_element_to_html_element(); - if (is(html_element)) { - auto& input_element = static_cast(html_element); - if (auto* form = input_element.form()) { - form->implicitly_submit_form().release_value_but_fixme_should_propagate_errors(); - return; - } - input_element.commit_pending_changes(); - } + auto* input_element = as_if(form_associated_element_to_html_element()); + if (!input_element) + return EventResult::Dropped; + + if (auto* form = input_element->form()) + form->implicitly_submit_form().release_value_but_fixme_should_propagate_errors(); + else + input_element->commit_pending_changes(); + + return EventResult::Handled; } void FormAssociatedTextControlElement::collapse_selection_to_offset(size_t position) diff --git a/Libraries/LibWeb/HTML/FormAssociatedElement.h b/Libraries/LibWeb/HTML/FormAssociatedElement.h index b9975bc42c6..36a98fc9596 100644 --- a/Libraries/LibWeb/HTML/FormAssociatedElement.h +++ b/Libraries/LibWeb/HTML/FormAssociatedElement.h @@ -214,7 +214,7 @@ public: virtual void handle_insert(String const&) override; virtual void handle_delete(DeleteDirection) override; - virtual void handle_return_key() override; + virtual EventResult handle_return_key() override; virtual void select_all() override; virtual void set_selection_anchor(GC::Ref, size_t offset) override; virtual void set_selection_focus(GC::Ref, size_t offset) override; diff --git a/Libraries/LibWeb/Internals/Internals.cpp b/Libraries/LibWeb/Internals/Internals.cpp index d0f0fe46eb5..e717f496cd7 100644 --- a/Libraries/LibWeb/Internals/Internals.cpp +++ b/Libraries/LibWeb/Internals/Internals.cpp @@ -89,7 +89,7 @@ void Internals::send_key(HTML::HTMLElement& target, String const& key_name, WebI void Internals::commit_text() { - page().handle_keydown(UIEvents::Key_Return, 0, 0, false); + page().handle_keydown(UIEvents::Key_Return, 0, 0x0d, false); } void Internals::click(double x, double y) diff --git a/Libraries/LibWeb/Page/EventHandler.cpp b/Libraries/LibWeb/Page/EventHandler.cpp index 57a5739fd3b..1ab63958712 100644 --- a/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Libraries/LibWeb/Page/EventHandler.cpp @@ -1236,8 +1236,12 @@ EventResult EventHandler::handle_keydown(UIEvents::KeyCode key, u32 modifiers, u if (key == UIEvents::KeyCode::Key_Return) { FIRE(input_event(UIEvents::EventNames::beforeinput, UIEvents::InputTypes::insertParagraph, m_navigable, code_point)); - target->handle_return_key(); + + if (target->handle_return_key() != EventResult::Handled) + target->handle_insert(String::from_code_point(code_point)); + FIRE(input_event(UIEvents::EventNames::input, UIEvents::InputTypes::insertParagraph, m_navigable, code_point)); + return EventResult::Handled; } // FIXME: Text editing shortcut keys (copy/paste etc.) should be handled here. diff --git a/Tests/LibWeb/Text/input/input-commit.html b/Tests/LibWeb/Text/input/input-commit.html index 341b7d86de6..0f57206a15e 100644 --- a/Tests/LibWeb/Text/input/input-commit.html +++ b/Tests/LibWeb/Text/input/input-commit.html @@ -11,5 +11,8 @@ internals.sendText(input, "wfh :^)"); internals.commitText(); + + // A second commit should not result in a change event. + internals.commitText(); })