From 906ac71eca5369a29152b41962629bea9e0eeb32 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sat, 24 Feb 2024 02:53:35 +0100 Subject: [PATCH] LibWeb: Fix crashing after input into empty contenteditable Change `EventHandler::handle_keydown()` to no longer assume the cursor position's node is always a `DOM::Text`. While this assumption holds for `HTMLInputElement` that has a shadow DOM with a text node, an empty `contenteditable` might not have any children. With this change, `handle_keydown()` creates a new text node if the cursor position's node is not a text node. --- .../input-into-empty-contenteditable.txt | 1 + .../input-into-empty-contenteditable.html | 15 ++++++++++ Userland/Libraries/LibWeb/DOM/Position.h | 1 + .../LibWeb/Page/EditEventHandler.cpp | 9 ++++++ .../Libraries/LibWeb/Page/EventHandler.cpp | 30 +++++++++++++------ 5 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/Editing/input-into-empty-contenteditable.txt create mode 100644 Tests/LibWeb/Text/input/Editing/input-into-empty-contenteditable.html diff --git a/Tests/LibWeb/Text/expected/Editing/input-into-empty-contenteditable.txt b/Tests/LibWeb/Text/expected/Editing/input-into-empty-contenteditable.txt new file mode 100644 index 0000000000..c867320453 --- /dev/null +++ b/Tests/LibWeb/Text/expected/Editing/input-into-empty-contenteditable.txt @@ -0,0 +1 @@ +hello diff --git a/Tests/LibWeb/Text/input/Editing/input-into-empty-contenteditable.html b/Tests/LibWeb/Text/input/Editing/input-into-empty-contenteditable.html new file mode 100644 index 0000000000..c630401ea1 --- /dev/null +++ b/Tests/LibWeb/Text/input/Editing/input-into-empty-contenteditable.html @@ -0,0 +1,15 @@ + + +
+ diff --git a/Userland/Libraries/LibWeb/DOM/Position.h b/Userland/Libraries/LibWeb/DOM/Position.h index c7b021decd..e1466d0d76 100644 --- a/Userland/Libraries/LibWeb/DOM/Position.h +++ b/Userland/Libraries/LibWeb/DOM/Position.h @@ -28,6 +28,7 @@ public: JS::GCPtr node() { return m_node; } JS::GCPtr node() const { return m_node; } + void set_node(JS::NonnullGCPtr node) { m_node = node; } unsigned offset() const { return m_offset; } bool offset_is_at_end_of_node() const; diff --git a/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp b/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp index 2d4acc1421..9dfe950c00 100644 --- a/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp @@ -114,6 +114,15 @@ void EditEventHandler::handle_insert(JS::NonnullGCPtr position, u node.set_data(MUST(builder.to_string())); node.invalidate_style(); + } else { + auto& node = *position->node(); + auto& realm = node.realm(); + StringBuilder builder; + builder.append_code_point(code_point); + auto text = realm.heap().allocate(realm, node.document(), MUST(builder.to_string())); + MUST(node.append_child(*text)); + position->set_node(text); + position->set_offset(1); } // FIXME: When nodes are removed from the DOM, the associated layout nodes become stale and still diff --git a/Userland/Libraries/LibWeb/Page/EventHandler.cpp b/Userland/Libraries/LibWeb/Page/EventHandler.cpp index 884eaef94b..87bba02d69 100644 --- a/Userland/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EventHandler.cpp @@ -787,8 +787,6 @@ bool EventHandler::handle_keydown(KeyCode key, u32 modifiers, u32 code_point) return false; if (m_browsing_context->cursor_position() && m_browsing_context->cursor_position()->node()->is_editable()) { - auto& node = verify_cast(*m_browsing_context->cursor_position()->node()); - if (key == KeyCode::Key_Backspace) { if (!m_browsing_context->decrement_cursor_position_offset()) { // FIXME: Move to the previous node and delete the last character there. @@ -819,23 +817,37 @@ bool EventHandler::handle_keydown(KeyCode key, u32 modifiers, u32 code_point) return true; } if (key == KeyCode::Key_Home) { - m_browsing_context->set_cursor_position(DOM::Position::create(realm, node, 0)); + auto& cursor_position_node = *m_browsing_context->cursor_position()->node(); + if (cursor_position_node.is_text()) + m_browsing_context->set_cursor_position(DOM::Position::create(realm, cursor_position_node, 0)); return true; } if (key == KeyCode::Key_End) { - m_browsing_context->set_cursor_position(DOM::Position::create(realm, node, (unsigned)node.data().bytes().size())); + auto& cursor_position_node = *m_browsing_context->cursor_position()->node(); + if (cursor_position_node.is_text()) { + auto& text_node = static_cast(cursor_position_node); + m_browsing_context->set_cursor_position(DOM::Position::create(realm, text_node, (unsigned)text_node.data().bytes().size())); + } return true; } if (key == KeyCode::Key_Return) { - if (is(node.editable_text_node_owner())) { - auto& input_element = static_cast(*node.editable_text_node_owner()); - - if (auto* form = input_element.form()) { + HTML::HTMLInputElement* input_element = nullptr; + if (auto node = m_browsing_context->cursor_position()->node()) { + if (node->is_text()) { + auto& text_node = static_cast(*node); + if (is(text_node.editable_text_node_owner())) + input_element = static_cast(text_node.editable_text_node_owner()); + } else if (node->is_html_input_element()) { + input_element = static_cast(node.ptr()); + } + } + if (input_element) { + if (auto* form = input_element->form()) { form->implicitly_submit_form().release_value_but_fixme_should_propagate_errors(); return true; } - input_element.commit_pending_changes(); + input_element->commit_pending_changes(); return true; } }