From 9440a3c280fbdfb32b6311a3a880384dca207579 Mon Sep 17 00:00:00 2001 From: Max Wipfli Date: Tue, 18 May 2021 22:09:15 +0200 Subject: [PATCH] LibWeb: Improve Unicode compatibility of HTML contenteditable This patch updates the Page::keydown_event event handler to implement crude Unicode support. It implements new method in EditEventHandler to more easily handle deleting a single character after the cursor. Furthermore, it makes use of the previously implemented methods to increment and decrement the cursor position, which take into account that Unicode codepoint may be multiple bytes wide. This means it is now possible to mostly edit Unicode in editable DOM nodes without any crashes. :^) --- .../LibWeb/Page/EditEventHandler.cpp | 24 ++++++++- .../Libraries/LibWeb/Page/EditEventHandler.h | 3 +- .../Libraries/LibWeb/Page/EventHandler.cpp | 50 +++---------------- 3 files changed, 32 insertions(+), 45 deletions(-) diff --git a/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp b/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp index 3c34885ed4..e3d6691cda 100644 --- a/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EditEventHandler.cpp @@ -1,21 +1,41 @@ /* - * Copyright (c) 2020, the SerenityOS developers. + * Copyright (c) 2020-2021, the SerenityOS developers. * * SPDX-License-Identifier: BSD-2-Clause */ -#include "EditEventHandler.h" #include +#include #include #include #include #include #include #include +#include #include namespace Web { +void EditEventHandler::handle_delete_character_after(const DOM::Position& cursor_position) +{ + if (cursor_position.offset_is_at_end_of_node()) { + // FIXME: Move to the next node and delete the first character there. + return; + } + + auto& node = *static_cast(const_cast(cursor_position.node())); + auto& text = node.data(); + auto codepoint_length = Utf8View(text).iterator_at_byte_offset(cursor_position.offset()).code_point_length_in_bytes(); + + StringBuilder builder; + builder.append(text.substring_view(0, cursor_position.offset())); + builder.append(text.substring_view(cursor_position.offset() + codepoint_length)); + node.set_data(builder.to_string()); + + m_frame.did_edit({}); +} + // This method is quite convoluted but this is necessary to make editing feel intuitive. void EditEventHandler::handle_delete(DOM::Range& range) { diff --git a/Userland/Libraries/LibWeb/Page/EditEventHandler.h b/Userland/Libraries/LibWeb/Page/EditEventHandler.h index 91d202ad2a..30fe21f30a 100644 --- a/Userland/Libraries/LibWeb/Page/EditEventHandler.h +++ b/Userland/Libraries/LibWeb/Page/EditEventHandler.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, the SerenityOS developers. + * Copyright (c) 2020-2021, the SerenityOS developers. * * SPDX-License-Identifier: BSD-2-Clause */ @@ -20,6 +20,7 @@ public: virtual ~EditEventHandler() = default; + virtual void handle_delete_character_after(const DOM::Position&); virtual void handle_delete(DOM::Range&); virtual void handle_insert(DOM::Position, u32 code_point); diff --git a/Userland/Libraries/LibWeb/Page/EventHandler.cpp b/Userland/Libraries/LibWeb/Page/EventHandler.cpp index 4b4649278d..c5c491dc43 100644 --- a/Userland/Libraries/LibWeb/Page/EventHandler.cpp +++ b/Userland/Libraries/LibWeb/Page/EventHandler.cpp @@ -408,11 +408,7 @@ bool EventHandler::handle_keydown(KeyCode key, unsigned modifiers, u32 code_poin } else if (!should_ignore_keydown_event(code_point)) { m_edit_event_handler->handle_delete(range); m_edit_event_handler->handle_insert(m_frame.cursor_position(), code_point); - - auto new_position = m_frame.cursor_position(); - new_position.set_offset(new_position.offset() + 1); - m_frame.set_cursor_position(move(new_position)); - + m_frame.increment_cursor_position_offset(); return true; } } @@ -420,63 +416,33 @@ bool EventHandler::handle_keydown(KeyCode key, unsigned modifiers, u32 code_poin if (m_frame.cursor_position().is_valid() && m_frame.cursor_position().node()->is_editable()) { if (key == KeyCode::Key_Backspace) { - auto position = m_frame.cursor_position(); - - if (position.offset() == 0) { + if (!m_frame.decrement_cursor_position_offset()) { // FIXME: Move to the previous node and delete the last character there. return true; } - auto new_position = m_frame.cursor_position(); - new_position.set_offset(position.offset() - 1); - m_frame.set_cursor_position(move(new_position)); - - m_edit_event_handler->handle_delete(DOM::Range::create(*position.node(), position.offset() - 1, *position.node(), position.offset())); - + m_edit_event_handler->handle_delete_character_after(m_frame.cursor_position()); return true; } else if (key == KeyCode::Key_Delete) { - auto position = m_frame.cursor_position(); - - if (position.offset() >= downcast(position.node())->data().length()) { + if (m_frame.cursor_position().offset_is_at_end_of_node()) { // FIXME: Move to the next node and delete the first character there. return true; } - - m_edit_event_handler->handle_delete(DOM::Range::create(*position.node(), position.offset(), *position.node(), position.offset() + 1)); - + m_edit_event_handler->handle_delete_character_after(m_frame.cursor_position()); return true; } else if (key == KeyCode::Key_Right) { - auto position = m_frame.cursor_position(); - - if (position.offset() >= downcast(position.node())->data().length()) { + if (!m_frame.increment_cursor_position_offset()) { // FIXME: Move to the next node. - return true; } - - auto new_position = m_frame.cursor_position(); - new_position.set_offset(position.offset() + 1); - m_frame.set_cursor_position(move(new_position)); - return true; } else if (key == KeyCode::Key_Left) { - auto position = m_frame.cursor_position(); - - if (position.offset() == 0) { + if (!m_frame.decrement_cursor_position_offset()) { // FIXME: Move to the previous node. - return true; } - - auto new_position = m_frame.cursor_position(); - new_position.set_offset(new_position.offset() - 1); - m_frame.set_cursor_position(move(new_position)); - return true; } else if (!should_ignore_keydown_event(code_point)) { m_edit_event_handler->handle_insert(m_frame.cursor_position(), code_point); - - auto new_position = m_frame.cursor_position(); - new_position.set_offset(new_position.offset() + 1); - m_frame.set_cursor_position(move(new_position)); + m_frame.increment_cursor_position_offset(); return true; } else { // NOTE: Because modifier keys should be ignored, we need to return true.