From 9b5d1382bf3648b1aa3e981b80d4d1eb04937e4b Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Fri, 14 Jun 2024 16:56:38 +0100 Subject: [PATCH] LibWeb: Rerun find in page query when navigating between URLs Previously, the "Find Next Match" and "Find Previous Match" actions simply updated the match index of the last query to be performed. This led to incorrect results if the page had been modified after the last query had been run. `Page::find_in_page_next_match()` and `Page::find_in_page_previous_match()` both now rerun the last query to ensure the results are up to date before updating the match index. The match index is also reset if the URL of the active document has changed since the last query. The current match index is maintained if only the URL fragment changes. --- Userland/Libraries/LibWeb/Page/Page.cpp | 65 +++++++++++-------- Userland/Libraries/LibWeb/Page/Page.h | 9 ++- .../WebContent/ConnectionFromClient.cpp | 2 +- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/Userland/Libraries/LibWeb/Page/Page.cpp b/Userland/Libraries/LibWeb/Page/Page.cpp index 7f2728ac577..37cc4723410 100644 --- a/Userland/Libraries/LibWeb/Page/Page.cpp +++ b/Userland/Libraries/LibWeb/Page/Page.cpp @@ -563,19 +563,13 @@ void Page::clear_selection() } } -Page::FindInPageResult Page::find_in_page(String const& query, CaseSensitivity case_sensitivity) +Page::FindInPageResult Page::perform_find_in_page_query(FindInPageQuery const& query) { - m_find_in_page_match_index = 0; - - if (query.is_empty()) { - m_find_in_page_matches = {}; - update_find_in_page_selection(); - return {}; - } + VERIFY(top_level_traversable_is_initialized()); Vector> all_matches; for (auto const& document : documents_in_active_window()) { - auto matches = document->find_matching_text(query, case_sensitivity); + auto matches = document->find_matching_text(query.string, query.case_sensitivity); all_matches.extend(move(matches)); } @@ -583,6 +577,15 @@ Page::FindInPageResult Page::find_in_page(String const& query, CaseSensitivity c for (auto& match : all_matches) m_find_in_page_matches.append(*match); + if (auto active_document = top_level_traversable()->active_document()) { + if (m_last_find_in_page_url.serialize(URL::ExcludeFragment::Yes) != active_document->url().serialize(URL::ExcludeFragment::Yes)) { + m_last_find_in_page_url = top_level_traversable()->active_document()->url(); + m_find_in_page_match_index = 0; + } + } else if (m_find_in_page_match_index >= m_find_in_page_matches.size()) { + m_find_in_page_match_index = 0; + } + update_find_in_page_selection(); return Page::FindInPageResult { @@ -591,42 +594,52 @@ Page::FindInPageResult Page::find_in_page(String const& query, CaseSensitivity c }; } -Page::FindInPageResult Page::find_in_page_next_match() +Page::FindInPageResult Page::find_in_page(FindInPageQuery const& query) { - if (m_find_in_page_matches.is_empty()) + if (!top_level_traversable_is_initialized()) return {}; - if (m_find_in_page_match_index == m_find_in_page_matches.size() - 1) { + m_find_in_page_match_index = 0; + m_last_find_in_page_query = query; + m_last_find_in_page_url = top_level_traversable()->active_document()->url(); + + if (query.string.is_empty()) { + m_last_find_in_page_query = {}; + update_find_in_page_selection(); + return {}; + } + + return perform_find_in_page_query(query); +} + +Page::FindInPageResult Page::find_in_page_next_match() +{ + if (!(m_last_find_in_page_query.has_value() && top_level_traversable_is_initialized())) + return {}; + + auto result = perform_find_in_page_query(*m_last_find_in_page_query); + if (m_find_in_page_match_index == *result.total_match_count - 1) { m_find_in_page_match_index = 0; } else { m_find_in_page_match_index++; } - update_find_in_page_selection(); - - return Page::FindInPageResult { - .current_match_index = m_find_in_page_match_index, - .total_match_count = m_find_in_page_matches.size(), - }; + return result; } Page::FindInPageResult Page::find_in_page_previous_match() { - if (m_find_in_page_matches.is_empty()) + if (!(m_last_find_in_page_query.has_value() && top_level_traversable_is_initialized())) return {}; + auto result = perform_find_in_page_query(*m_last_find_in_page_query); if (m_find_in_page_match_index == 0) { - m_find_in_page_match_index = m_find_in_page_matches.size() - 1; + m_find_in_page_match_index = *result.total_match_count - 1; } else { m_find_in_page_match_index--; } - update_find_in_page_selection(); - - return Page::FindInPageResult { - .current_match_index = m_find_in_page_match_index, - .total_match_count = m_find_in_page_matches.size(), - }; + return result; } void Page::update_find_in_page_selection() diff --git a/Userland/Libraries/LibWeb/Page/Page.h b/Userland/Libraries/LibWeb/Page/Page.h index 25dbbdf4c92..061f0149998 100644 --- a/Userland/Libraries/LibWeb/Page/Page.h +++ b/Userland/Libraries/LibWeb/Page/Page.h @@ -187,11 +187,15 @@ public: void clear_selection(); + struct FindInPageQuery { + String string {}; + CaseSensitivity case_sensitivity { CaseSensitivity::CaseInsensitive }; + }; struct FindInPageResult { size_t current_match_index { 0 }; Optional total_match_count {}; }; - FindInPageResult find_in_page(String const& query, CaseSensitivity); + FindInPageResult find_in_page(FindInPageQuery const&); FindInPageResult find_in_page_next_match(); FindInPageResult find_in_page_previous_match(); @@ -203,6 +207,7 @@ private: Vector> documents_in_active_window() const; + FindInPageResult perform_find_in_page_query(FindInPageQuery const&); void update_find_in_page_selection(); JS::NonnullGCPtr m_client; @@ -248,6 +253,8 @@ private: bool m_pdf_viewer_supported { false }; size_t m_find_in_page_match_index { 0 }; Vector> m_find_in_page_matches; + Optional m_last_find_in_page_query; + URL::URL m_last_find_in_page_url; }; struct PaintOptions { diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 71b71cf6c42..4fcfa6cdf03 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -812,7 +812,7 @@ void ConnectionFromClient::find_in_page(u64 page_id, String const& query, CaseSe if (!page.has_value()) return; - auto result = page->page().find_in_page(query, case_sensitivity); + auto result = page->page().find_in_page({ .string = query, .case_sensitivity = case_sensitivity }); async_did_find_in_page(page_id, result.current_match_index, result.total_match_count); }