Don't select empty PDF pages when "Select All" is requested.
`PDFiumEngine::SelectAll` selected all pages regardless of having text or not, while `PDFiumEngine::Selection` only selects pages in the given range that include text. To make the behavior consistent, update the former to only select pages with text. Bug: 387387738, 392926460 Change-Id: Iee4833d7b44acc8e5ef7175560f77de0b4a1ed25 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6218255 Commit-Queue: Ramin Halavati <rhalavati@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/main@{#1414767}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
dc6b0516e4
commit
c7e39020fe
@ -1759,7 +1759,9 @@ bool PDFiumEngine::ExtendSelection(int page_index, int char_index) {
|
||||
// First make sure that there are no gaps in selection, i.e. if mousedown on
|
||||
// page one but we only get mousemove over page three, we want page two.
|
||||
for (int i = last_page_index + 1; i < page_index; ++i) {
|
||||
selection_.push_back(PDFiumRange::AllTextOnPage(pages_[i].get()));
|
||||
if (pages_[i]->GetCharCount()) {
|
||||
selection_.push_back(PDFiumRange::AllTextOnPage(pages_[i].get()));
|
||||
}
|
||||
}
|
||||
|
||||
int count = pages_[last_page_index]->GetCharCount();
|
||||
@ -1775,7 +1777,9 @@ bool PDFiumEngine::ExtendSelection(int page_index, int char_index) {
|
||||
// First make sure that there are no gaps in selection, i.e. if mousedown on
|
||||
// page three but we only get mousemove over page one, we want page two.
|
||||
for (int i = last_page_index - 1; i > page_index; --i) {
|
||||
selection_.push_back(PDFiumRange::AllTextOnPage(pages_[i].get()));
|
||||
if (pages_[i]->GetCharCount()) {
|
||||
selection_.push_back(PDFiumRange::AllTextOnPage(pages_[i].get()));
|
||||
}
|
||||
}
|
||||
|
||||
int count = pages_[page_index]->GetCharCount();
|
||||
@ -2433,7 +2437,7 @@ void PDFiumEngine::SelectAll() {
|
||||
|
||||
selection_.clear();
|
||||
for (const auto& page : pages_) {
|
||||
if (page->available()) {
|
||||
if (page->GetCharCount()) {
|
||||
selection_.push_back(PDFiumRange::AllTextOnPage(page.get()));
|
||||
}
|
||||
}
|
||||
|
@ -517,6 +517,40 @@ TEST_P(PDFiumOnDemandSearchifierTest, MetricsCanceledPageWithoutText) {
|
||||
histogram_tester.ExpectTotalCount(kSearchifyAddedTextHistogram, 0);
|
||||
}
|
||||
|
||||
TEST_P(PDFiumOnDemandSearchifierTest, SelectPageBeforeSearchify) {
|
||||
CreateEngine(FILE_PATH_LITERAL("image_alt_text.pdf"));
|
||||
|
||||
PDFiumPage& page = GetPDFiumPageForTest(*engine(), 0);
|
||||
|
||||
// Load the page to trigger searchify checking.
|
||||
page.GetPage();
|
||||
ASSERT_TRUE(engine()->PageNeedsSearchify(0));
|
||||
engine()->SelectAll();
|
||||
ASSERT_TRUE(engine()->GetSelectedText().empty());
|
||||
|
||||
PDFiumOnDemandSearchifier* searchifier = engine()->GetSearchifierForTesting();
|
||||
ASSERT_TRUE(searchifier);
|
||||
|
||||
ASSERT_TRUE(searchifier->IsPageScheduled(0));
|
||||
|
||||
StartSearchify(/*empty_results=*/false);
|
||||
|
||||
base::test::TestFuture<void> future;
|
||||
WaitUntilIdle(searchifier, future.GetCallback());
|
||||
ASSERT_TRUE(future.Wait());
|
||||
|
||||
// Perform SelectAll again to select extracted text.
|
||||
engine()->SelectAll();
|
||||
|
||||
// The page has 2 images, so the text contains 2 fake OCR results.
|
||||
#if BUILDFLAG(IS_WIN)
|
||||
const char kExpectedSelection[] = "OCR Text 0\r\nOCR Text 1";
|
||||
#else
|
||||
const char kExpectedSelection[] = "OCR Text 0\nOCR Text 1";
|
||||
#endif
|
||||
ASSERT_EQ(engine()->GetSelectedText(), kExpectedSelection);
|
||||
}
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(All, PDFiumOnDemandSearchifierTest, testing::Bool());
|
||||
|
||||
} // namespace chrome_pdf
|
||||
|
Reference in New Issue
Block a user