Fix PDF viewer jumping immediately after the user presses 'ctrl+f'
This behavior is disruptive and inconsistent with the search behavior of non-PDF sites. After this commit, pressing 'ctrl+f' will not start a search until the user presses additional keys (add characters / press enter). This bug is a side effect of fixing another bug "Chrome PDF Search is not executed in background" (crbug.com/764635). The fix reverts the changes done by the previous bug fix (pdfium_engine.cc) and provides an alternative solution (find_request_manager.cc) which requires to update some tests (pdf_find_request_manager_browsertest.cc, find_request_manager_browsertest.cc, findtext_unittest.cc). Lastly this is my first bug so I added my name to AUTHORS. Bug: 1290824 Change-Id: I8e152087fb2ac019124e477b4d04d73d7b8f2f21 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3455153 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Reviewed-by: Pavel Feldman <pfeldman@chromium.org> Cr-Commit-Position: refs/heads/main@{#1043764}
This commit is contained in:
1
AUTHORS
1
AUTHORS
@ -134,6 +134,7 @@ Arnaud Mandy <arnaud.mandy@intel.com>
|
||||
Arnaud Renevier <a.renevier@samsung.com>
|
||||
Arpita Bahuguna <a.bah@samsung.com>
|
||||
Arthur Lussos <developer0420@gmail.com>
|
||||
Artur Akerberg <artur.aker@gmail.com>
|
||||
Arun Kulkarni <kulkarni.a@samsung.com>
|
||||
Arun Kumar <arun87.kumar@samsung.com>
|
||||
Arun Mankuzhi <arun.m@samsung.com>
|
||||
|
@ -245,8 +245,10 @@ IN_PROC_BROWSER_TEST_F(PdfFindRequestManagerTestWithPdfPartialLoading,
|
||||
// Verify that find-in-page works fine.
|
||||
auto options = blink::mojom::FindOptions::New();
|
||||
Find("FXCMAP_CMap", options.Clone());
|
||||
delegate()->WaitForFinalReply();
|
||||
options->new_session = false;
|
||||
Find("FXCMAP_CMap", options.Clone());
|
||||
delegate()->WaitForFinalReply();
|
||||
Find("FXCMAP_CMap", options.Clone());
|
||||
delegate()->WaitForFinalReply();
|
||||
|
||||
|
@ -374,6 +374,8 @@ void FindRequestManager::EmitFindRequest(int request_id,
|
||||
find_request_queue_.emplace(request_id, search_text, std::move(options));
|
||||
if (find_request_queue_.size() == 1)
|
||||
FindInternal(find_request_queue_.front());
|
||||
if (request_id == current_session_id_)
|
||||
find_request_queue_.pop();
|
||||
}
|
||||
|
||||
void FindRequestManager::ForEachAddedFindInPageRenderFrameHost(
|
||||
|
@ -1475,6 +1475,10 @@ IN_PROC_BROWSER_TEST_F(FindRequestManagerFencedFrameTest,
|
||||
auto options = blink::mojom::FindOptions::New();
|
||||
options->run_synchronously_for_testing = true;
|
||||
Find("result", options.Clone());
|
||||
// Initial find request is pop from the queue immediately so we make a second
|
||||
// find request.
|
||||
options->new_session = false;
|
||||
Find("result", options.Clone());
|
||||
|
||||
// Create a fenced frame.
|
||||
GURL find_test_url =
|
||||
|
@ -73,8 +73,6 @@ void ExpectInitialSearchResults(FindTextTestClient& client, int count) {
|
||||
|
||||
EXPECT_CALL(client,
|
||||
NotifyNumberOfFindResultsChanged(1, /*final_result=*/false));
|
||||
EXPECT_CALL(client,
|
||||
NotifySelectedFindResultChanged(0, /*final_result=*/false));
|
||||
for (int i = 2; i < count + 1; ++i) {
|
||||
EXPECT_CALL(client,
|
||||
NotifyNumberOfFindResultsChanged(i, /*final_result=*/false));
|
||||
@ -184,6 +182,7 @@ TEST_F(FindTextTest, SelectFindResult) {
|
||||
|
||||
ExpectInitialSearchResults(client, 4);
|
||||
engine->StartFind("world", /*case_sensitive=*/true);
|
||||
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true));
|
||||
|
||||
EXPECT_CALL(client, NotifyNumberOfFindResultsChanged(_, _)).Times(0);
|
||||
EXPECT_CALL(client,
|
||||
@ -213,9 +212,9 @@ TEST_F(FindTextTest, SelectFindResultAndSwitchToTwoUpView) {
|
||||
InSequence sequence;
|
||||
|
||||
EXPECT_CALL(client,
|
||||
NotifySelectedFindResultChanged(1, /*final_result=*/true));
|
||||
NotifySelectedFindResultChanged(0, /*final_result=*/true));
|
||||
EXPECT_CALL(client,
|
||||
NotifySelectedFindResultChanged(2, /*final_result=*/true));
|
||||
NotifySelectedFindResultChanged(1, /*final_result=*/true));
|
||||
}
|
||||
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true));
|
||||
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true));
|
||||
@ -236,7 +235,7 @@ TEST_F(FindTextTest, SelectFindResultAndSwitchToTwoUpView) {
|
||||
InSequence sequence;
|
||||
|
||||
EXPECT_CALL(client,
|
||||
NotifySelectedFindResultChanged(2, /*final_result=*/true));
|
||||
NotifySelectedFindResultChanged(1, /*final_result=*/true));
|
||||
}
|
||||
ASSERT_TRUE(engine->SelectFindResult(/*forward=*/true));
|
||||
}
|
||||
|
@ -1987,7 +1987,6 @@ void PDFiumEngine::SearchUsingICU(const std::u16string& term,
|
||||
}
|
||||
|
||||
void PDFiumEngine::AddFindResult(const PDFiumRange& result) {
|
||||
bool first_result = find_results_.empty() && !resume_find_index_.has_value();
|
||||
// Figure out where to insert the new location, since we could have
|
||||
// started searching midway and now we wrapped.
|
||||
size_t result_index;
|
||||
@ -2003,10 +2002,6 @@ void PDFiumEngine::AddFindResult(const PDFiumRange& result) {
|
||||
find_results_.insert(find_results_.begin() + result_index, result);
|
||||
UpdateTickMarks();
|
||||
client_->NotifyNumberOfFindResultsChanged(find_results_.size(), false);
|
||||
if (first_result) {
|
||||
DCHECK(!current_find_index_);
|
||||
SelectFindResult(/*forward=*/true);
|
||||
}
|
||||
}
|
||||
|
||||
bool PDFiumEngine::SelectFindResult(bool forward) {
|
||||
|
Reference in New Issue
Block a user