From fd289deb44e0fc26f54a133c637c136bf0716cd5 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 27 Aug 2024 10:46:34 -0400 Subject: [PATCH] LibWeb: Update the document cursor position when the selection changes Otherwise, it looks a bit awkward where the cursor position does not update while the selection is elsewhere. Note that this requires passing along the raw selection positions from `set the selection range` to the elements. Otherwise, consider what will happen if we set the selection start and end to the same value. By going through the API accessor, we hit the case where the start and end are the same value, and return the document cursor position. This would mean the cursor position would not be updated. The test changes here more closely match what Firefox produces now. It is not a 100% match; the `select event fired` test case isn't right. The problem is the event fires for the input element, but we most recently focused the textarea element. Thus, when we retrieve the selection from the input element, we return the document's cursor position, which is actually in the textarea element. The fix will ultimately be to fully implement the following: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-textarea/input-cursor That is, each input / textarea element should separately track its own text cursor position. --- .../Text/expected/DOM/FormAssociatedElement-selection.txt | 6 +++--- Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp | 2 +- Userland/Libraries/LibWeb/HTML/FormAssociatedElement.h | 4 ++-- Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp | 6 ++++-- Userland/Libraries/LibWeb/HTML/HTMLInputElement.h | 2 +- Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp | 6 ++++-- Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.h | 2 +- 7 files changed, 16 insertions(+), 12 deletions(-) diff --git a/Tests/LibWeb/Text/expected/DOM/FormAssociatedElement-selection.txt b/Tests/LibWeb/Text/expected/DOM/FormAssociatedElement-selection.txt index 32ea89b6839..e163b3c7546 100644 --- a/Tests/LibWeb/Text/expected/DOM/FormAssociatedElement-selection.txt +++ b/Tests/LibWeb/Text/expected/DOM/FormAssociatedElement-selection.txt @@ -8,7 +8,7 @@ text-input selectionStart: 0 selectionEnd: 18 selectionDirection: none text-input selectionStart: 2 selectionEnd: 4 selectionDirection: forward text-input selectionStart: 1 selectionEnd: 4 selectionDirection: forward text-input selectionStart: 1 selectionEnd: 5 selectionDirection: forward -text-input selectionStart: 18 selectionEnd: 18 selectionDirection: forward -text-input selectionStart: 18 selectionEnd: 18 selectionDirection: backward +text-input selectionStart: 6 selectionEnd: 6 selectionDirection: forward +text-input selectionStart: 6 selectionEnd: 6 selectionDirection: backward textarea selectionStart: 0 selectionEnd: 9 selectionDirection: none -select event fired: 18 18 +select event fired: 9 9 diff --git a/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp b/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp index f59b480cdd9..41ba8929b59 100644 --- a/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.cpp @@ -541,7 +541,7 @@ void FormAssociatedTextControlElement::set_the_selection_range(Optional const&); - virtual void selection_was_changed() { } - private: void reset_form_owner(); @@ -159,6 +157,8 @@ protected: // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-textarea/input-relevant-value void relevant_value_was_changed(JS::GCPtr); + virtual void selection_was_changed([[maybe_unused]] size_t selection_start, [[maybe_unused]] size_t selection_end) { } + private: // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-textarea/input-selection WebIDL::UnsignedLong m_selection_start { 0 }; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index 393952a9875..7a68d4c434e 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -2362,13 +2362,15 @@ HTMLInputElement::ValueAttributeMode HTMLInputElement::value_attribute_mode() co VERIFY_NOT_REACHED(); } -void HTMLInputElement::selection_was_changed() +void HTMLInputElement::selection_was_changed(size_t selection_start, size_t selection_end) { + document().set_cursor_position(DOM::Position::create(realm(), *m_text_node, selection_end)); + auto selection = document().get_selection(); if (!selection || selection->range_count() == 0) return; - MUST(selection->set_base_and_extent(*m_text_node, selection_start().value(), *m_text_node, selection_end().value())); + MUST(selection->set_base_and_extent(*m_text_node, selection_start, *m_text_node, selection_end)); } } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h index 336de21e4c4..911513f3aeb 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h @@ -206,7 +206,7 @@ public: bool selection_or_range_applies() const; protected: - void selection_was_changed() override; + void selection_was_changed(size_t selection_start, size_t selection_end) override; private: HTMLInputElement(DOM::Document&, DOM::QualifiedName); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp index ee9706395b2..c2a3b05e400 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.cpp @@ -460,13 +460,15 @@ void HTMLTextAreaElement::queue_firing_input_event() }); } -void HTMLTextAreaElement::selection_was_changed() +void HTMLTextAreaElement::selection_was_changed(size_t selection_start, size_t selection_end) { + document().set_cursor_position(DOM::Position::create(realm(), *m_text_node, selection_end)); + auto selection = document().get_selection(); if (!selection || selection->range_count() == 0) return; - MUST(selection->set_base_and_extent(*m_text_node, selection_start().value(), *m_text_node, selection_end().value())); + MUST(selection->set_base_and_extent(*m_text_node, selection_start, *m_text_node, selection_end)); } } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.h b/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.h index 89f55ba3c83..ab0c300ff09 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLTextAreaElement.h @@ -123,7 +123,7 @@ public: void set_dirty_value_flag(Badge, bool flag) { m_dirty_value = flag; } protected: - void selection_was_changed() override; + void selection_was_changed(size_t selection_start, size_t selection_end) override; private: HTMLTextAreaElement(DOM::Document&, DOM::QualifiedName);