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

@@ -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;
 }
 
 }

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<DOM::Node>, size_t offset) override;
     virtual void set_selection_focus(GC::Ref<DOM::Node>, size_t offset) override;

Libraries/LibWeb/DOM/InputEventsTarget.h

@@ -8,6 +8,7 @@
 
 #include <LibGC/Ptr.h>
 #include <LibWeb/Forward.h>
+#include <LibWeb/Page/EventResult.h>
 
 namespace Web {
 
@@ -18,7 +19,7 @@ public:
     virtual GC::Ref<JS::Cell> 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,

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<HTMLInputElement>(html_element)) {
-        auto& input_element = static_cast<HTMLInputElement&>(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<HTMLInputElement>(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)

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<DOM::Node>, size_t offset) override;
     virtual void set_selection_focus(GC::Ref<DOM::Node>, size_t offset) override;

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)

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.

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();
     })
 </script>