From b11ba4cc903a76532bdea43b75ff1e85f49e5a9f Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sat, 1 Mar 2025 18:31:59 -0500 Subject: [PATCH] LibWeb: Clear the document's page's focused navigable upon destruction We set the page's focused navigable upon mouse-down events from the UI. However, we neglected to ever clear that focused navigable upon events such as subsequent page navigations. This left the page with a stale reference to a no-longer-active navigable. The effect was that any key events from the UI would not be sent to the new page until either the reference was collected by GC, or another mouse-down event occurred. In the test added here, without this fix, the text sent to the input element would not be received, and the change event would not fire. --- Libraries/LibWeb/DOM/Document.cpp | 7 ++++- Libraries/LibWeb/Page/Page.cpp | 6 ++++ Libraries/LibWeb/Page/Page.h | 1 + .../reset-focused-naviable-on-navigation.txt | 1 + .../reset-focused-naviable-on-navigation.html | 29 +++++++++++++++++++ 5 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 Tests/LibWeb/Text/expected/UIEvents/reset-focused-naviable-on-navigation.txt create mode 100644 Tests/LibWeb/Text/input/UIEvents/reset-focused-naviable-on-navigation.html diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index da40421c493..c6987c8b555 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -3971,9 +3971,14 @@ void Document::destroy() } // 9. Set document's node navigable's active session history entry's document state's document to null. - if (auto navigable = this->navigable()) + if (auto navigable = this->navigable()) { navigable->active_session_history_entry()->document_state()->set_document(nullptr); + // AD-HOC: We set the page's focused navigable during mouse-down events. If that navigable is this document's + // navigable, we must be sure to reset the page's focused navigable. + page().navigable_document_destroyed({}, *navigable); + } + // FIXME: 10. Remove document from the owner set of each WorkerGlobalScope object whose set contains document. // FIXME: 11. For each workletGlobalScope in document's worklet global scopes, terminate workletGlobalScope. } diff --git a/Libraries/LibWeb/Page/Page.cpp b/Libraries/LibWeb/Page/Page.cpp index 61fec2ad7e5..1ff61b4c5d7 100644 --- a/Libraries/LibWeb/Page/Page.cpp +++ b/Libraries/LibWeb/Page/Page.cpp @@ -63,6 +63,12 @@ void Page::set_focused_navigable(Badge, HTML::Navigable& navigable m_focused_navigable = navigable; } +void Page::navigable_document_destroyed(Badge, HTML::Navigable& navigable) +{ + if (&navigable == m_focused_navigable.ptr()) + m_focused_navigable.clear(); +} + void Page::load(URL::URL const& url) { (void)top_level_traversable()->navigate({ .url = url, .source_document = *top_level_traversable()->active_document(), .user_involvement = HTML::UserNavigationInvolvement::BrowserUI }); diff --git a/Libraries/LibWeb/Page/Page.h b/Libraries/LibWeb/Page/Page.h index f776a01c9e3..112b0a4225f 100644 --- a/Libraries/LibWeb/Page/Page.h +++ b/Libraries/LibWeb/Page/Page.h @@ -75,6 +75,7 @@ public: HTML::Navigable const& focused_navigable() const { return const_cast(this)->focused_navigable(); } void set_focused_navigable(Badge, HTML::Navigable&); + void navigable_document_destroyed(Badge, HTML::Navigable&); void load(URL::URL const&); diff --git a/Tests/LibWeb/Text/expected/UIEvents/reset-focused-naviable-on-navigation.txt b/Tests/LibWeb/Text/expected/UIEvents/reset-focused-naviable-on-navigation.txt new file mode 100644 index 00000000000..91b0e99c802 --- /dev/null +++ b/Tests/LibWeb/Text/expected/UIEvents/reset-focused-naviable-on-navigation.txt @@ -0,0 +1 @@ +wfh :^) diff --git a/Tests/LibWeb/Text/input/UIEvents/reset-focused-naviable-on-navigation.html b/Tests/LibWeb/Text/input/UIEvents/reset-focused-naviable-on-navigation.html new file mode 100644 index 00000000000..1278c300663 --- /dev/null +++ b/Tests/LibWeb/Text/input/UIEvents/reset-focused-naviable-on-navigation.html @@ -0,0 +1,29 @@ + + + + +