Skip to content

Commit

Permalink
Preserving Text Selection during Page Rotation and 2-Up View Toggling
Browse files Browse the repository at this point in the history
When viewing a PDF, if text is selected, it will remain highlighted
after rotating the page or toggling the 2-up view to ensure that the
text remains selected in the completed page layout.

Bug: 1420684
Change-Id: Ic3de70e543c7efd2370c00fc5cf4d59fc807ba11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4940462
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1210558}
  • Loading branch information
shaochenguang authored and Chromium LUCI CQ committed Oct 17, 2023
1 parent 737cb02 commit b5d1a6b
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 0 deletions.
21 changes: 21 additions & 0 deletions pdf/pdfium/pdfium_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2147,11 +2147,13 @@ void PDFiumEngine::ZoomUpdated(double new_zoom_level) {
}

void PDFiumEngine::RotateClockwise() {
SaveSelection();
desired_layout_options_.RotatePagesClockwise();
ProposeNextDocumentLayout();
}

void PDFiumEngine::RotateCounterclockwise() {
SaveSelection();
desired_layout_options_.RotatePagesCounterclockwise();
ProposeNextDocumentLayout();
}
Expand Down Expand Up @@ -2179,6 +2181,7 @@ void PDFiumEngine::SetReadOnly(bool enable) {
}

void PDFiumEngine::SetDocumentLayout(DocumentLayout::PageSpread page_spread) {
SaveSelection();
desired_layout_options_.set_page_spread(page_spread);
ProposeNextDocumentLayout();
}
Expand Down Expand Up @@ -3808,6 +3811,8 @@ gfx::Size PDFiumEngine::ApplyDocumentLayout(
// TODO(thestig): It would be better to also restore the position on the page.
client_->ScrollToPage(most_visible_page);

RestoreSelection();

return layout_.size();
}

Expand Down Expand Up @@ -3922,6 +3927,22 @@ void PDFiumEngine::SetSelection(const PageCharacterIndex& selection_start_index,
}
}

void PDFiumEngine::SaveSelection() {
// The PDFiumRange in the `selection_` has stored some previously cached
// information. The `saved_selection_` needs a fresh PDFiumRange for future
// use.
for (const auto& item : selection_) {
saved_selection_.push_back(PDFiumRange(
pages_[item.page_index()].get(), item.char_index(), item.char_count()));
}
}

void PDFiumEngine::RestoreSelection() {
if (!saved_selection_.empty()) {
selection_ = std::move(saved_selection_);
}
}

void PDFiumEngine::ScrollFocusedAnnotationIntoView() {
FPDF_ANNOTATION annot;
int page_index;
Expand Down
8 changes: 8 additions & 0 deletions pdf/pdfium/pdfium_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,9 @@ class PDFiumEngine : public PDFEngine,
void SetSelection(const PageCharacterIndex& selection_start_index,
const PageCharacterIndex& selection_end_index);

void SaveSelection();
void RestoreSelection();

// Scroll the current focused annotation into view if not already in view.
void ScrollFocusedAnnotationIntoView();

Expand Down Expand Up @@ -708,6 +711,11 @@ class PDFiumEngine : public PDFEngine,
// There could be more than one range if selection spans more than one page.
std::vector<PDFiumRange> selection_;

// When rotating the page or updating the PageSpread mode, used to store the
// contents of `selection_`. After the page change is completed, the contents
// of `selection_` are restored.
std::vector<PDFiumRange> saved_selection_;

// True if we're in the middle of text selection.
bool selecting_ = false;

Expand Down
63 changes: 63 additions & 0 deletions pdf/pdfium/pdfium_engine_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,69 @@ TEST_P(PDFiumEngineTest, SelectTextWithNonPrintableCharacter) {
EXPECT_EQ("Hello, world!", engine->GetSelectedText());
}

TEST_P(PDFiumEngineTest, RotateAfterSelectedText) {
NiceMock<MockTestClient> client;
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
ASSERT_TRUE(engine);

// Plugin size chosen so all pages of the document are visible.
engine->PluginSizeUpdated({1024, 4096});

EXPECT_THAT(engine->GetSelectedText(), IsEmpty());

constexpr gfx::PointF kPosition(100, 120);
EXPECT_TRUE(engine->HandleInputEvent(
CreateLeftClickWebMouseEventAtPositionWithClickCount(kPosition, 2)));
EXPECT_EQ("Goodbye", engine->GetSelectedText());

DocumentLayout::Options options;
EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(276, 556)))
.WillOnce(Return());
engine->RotateClockwise();
options.RotatePagesClockwise();
engine->ApplyDocumentLayout(options);
EXPECT_EQ("Goodbye", engine->GetSelectedText());

EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithSize(276, 556)))
.WillOnce(Return());
engine->RotateCounterclockwise();
options.RotatePagesCounterclockwise();
engine->ApplyDocumentLayout(options);
EXPECT_EQ("Goodbye", engine->GetSelectedText());
}

TEST_P(PDFiumEngineTest, MultiPagesPdfInTwoUpViewAfterSelectedText) {
NiceMock<MockTestClient> client;
std::unique_ptr<PDFiumEngine> engine =
InitializeEngine(&client, FILE_PATH_LITERAL("hello_world2.pdf"));
ASSERT_TRUE(engine);
// Plugin size chosen so all pages of the document are visible.
engine->PluginSizeUpdated({1024, 4096});

EXPECT_THAT(engine->GetSelectedText(), IsEmpty());

constexpr gfx::PointF kPosition(100, 120);
EXPECT_TRUE(engine->HandleInputEvent(
CreateLeftClickWebMouseEventAtPositionWithClickCount(kPosition, 2)));
EXPECT_EQ("Goodbye", engine->GetSelectedText());

DocumentLayout::Options options;
options.set_page_spread(DocumentLayout::PageSpread::kTwoUpOdd);
EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithOptions(options)))
.WillOnce(Return());
engine->SetDocumentLayout(DocumentLayout::PageSpread::kTwoUpOdd);
engine->ApplyDocumentLayout(options);
EXPECT_EQ("Goodbye", engine->GetSelectedText());

options.set_page_spread(DocumentLayout::PageSpread::kOneUp);
EXPECT_CALL(client, ProposeDocumentLayout(LayoutWithOptions(options)))
.WillOnce(Return());
engine->SetDocumentLayout(DocumentLayout::PageSpread::kOneUp);
engine->ApplyDocumentLayout(options);
EXPECT_EQ("Goodbye", engine->GetSelectedText());
}

INSTANTIATE_TEST_SUITE_P(All, PDFiumEngineTest, testing::Bool());

using PDFiumEngineDeathTest = PDFiumEngineTest;
Expand Down

0 comments on commit b5d1a6b

Please sign in to comment.