diff --git a/pdf/pdf_ink_module.cc b/pdf/pdf_ink_module.cc
index 28bb181225319..2af8a96904298 100644
--- a/pdf/pdf_ink_module.cc
+++ b/pdf/pdf_ink_module.cc
@@ -54,6 +54,7 @@
#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/base/cursor/cursor.h"
+#include "ui/base/cursor/mojom/cursor_type.mojom.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/point_conversions.h"
#include "ui/gfx/geometry/point_f.h"
@@ -370,6 +371,10 @@ bool PdfInkModule::OnMessage(const base::Value::Dict& message) {
}
void PdfInkModule::OnGeometryChanged() {
+ // If the highlighter tool is selected, and zooming moves the cursor onto
+ // text, the cursor should be an I-beam, but it will instead be the drawing
+ // cursor until a mousemove event occurs. There is not a way to get the new
+ // mouse position on geometry change.
MaybeSetCursor();
}
@@ -470,11 +475,11 @@ bool PdfInkModule::OnMouseUp(const blink::WebMouseEvent& event) {
return false;
}
+ gfx::PointF position = event.PositionInWidget();
if (features::kPdfInk2TextHighlighting.Get() && is_text_highlighting()) {
- return FinishTextHighlight();
+ return FinishTextHighlight(position);
}
- gfx::PointF position = event.PositionInWidget();
return is_drawing_stroke()
? FinishStroke(position, event.TimeStamp(),
ink::StrokeInput::ToolType::kMouse)
@@ -484,10 +489,8 @@ bool PdfInkModule::OnMouseUp(const blink::WebMouseEvent& event) {
bool PdfInkModule::OnMouseMove(const blink::WebMouseEvent& event) {
CHECK(enabled());
- // TODO(crbug.com/342445982): Set the cursor for hovering over text with the
- // highlighter brush while not drawing.
-
gfx::PointF position = event.PositionInWidget();
+
bool still_interacting_with_ink =
event.GetModifiers() & blink::WebInputEvent::kLeftButtonDown;
if (still_interacting_with_ink) {
@@ -507,6 +510,7 @@ bool PdfInkModule::OnMouseMove(const blink::WebMouseEvent& event) {
// that now, and compensate by synthesizing a mouse-up input event at the
// last known input position. Intentionally do not use `position`.
if (is_drawing_stroke()) {
+ MaybeSetCursorOnMouseMove(position);
DrawingStrokeState& state = drawing_stroke_state();
if (!state.input_last_event.has_value()) {
// Ignore when not drawing.
@@ -772,7 +776,12 @@ bool PdfInkModule::FinishStroke(const gfx::PointF& position,
state.page_index = -1;
state.input_last_event.reset();
- if (MaybeSetDrawingBrush()) {
+ bool set_drawing_brush = MaybeSetDrawingBrush();
+ if (features::kPdfInk2TextHighlighting.Get() &&
+ state.brush_type == PdfInkBrush::Type::kHighlighter &&
+ client_->IsSelectableTextOrLinkArea(position)) {
+ client_->UpdateInkCursor(ui::mojom::CursorType::kIBeam);
+ } else if (set_drawing_brush) {
MaybeSetCursor();
}
@@ -984,7 +993,7 @@ bool PdfInkModule::ContinueTextHighlight(const gfx::PointF& position) {
return true;
}
-bool PdfInkModule::FinishTextHighlight() {
+bool PdfInkModule::FinishTextHighlight(const gfx::PointF& position) {
CHECK(is_text_highlighting());
auto& highlight_strokes = text_highlight_state().highlight_strokes;
@@ -1018,6 +1027,10 @@ bool PdfInkModule::FinishTextHighlight() {
drawing_stroke_state().brush_type = PdfInkBrush::Type::kHighlighter;
client_->ClearSelection();
+
+ if (!client_->IsSelectableTextOrLinkArea(position)) {
+ MaybeSetCursor();
+ }
return true;
}
@@ -1609,7 +1622,6 @@ void PdfInkModule::MaybeSetCursor() {
}
if (features::kPdfInk2TextHighlighting.Get() && is_text_highlighting()) {
- // TODO(crbug.com/342445982): Set the cursor for text highlighting.
return;
}
@@ -1633,6 +1645,25 @@ void PdfInkModule::MaybeSetCursor() {
ui::Cursor::NewCustom(std::move(bitmap), std::move(hotspot)));
}
+void PdfInkModule::MaybeSetCursorOnMouseMove(const gfx::PointF& position) {
+ if (!features::kPdfInk2TextHighlighting.Get()) {
+ return;
+ }
+
+ CHECK(is_drawing_stroke());
+ if (drawing_stroke_state().brush_type != PdfInkBrush::Type::kHighlighter ||
+ !client_->IsSelectableTextOrLinkArea(position)) {
+ if (client_->GetCursor().type() == ui::mojom::CursorType::kIBeam) {
+ MaybeSetCursor();
+ }
+ return;
+ }
+
+ if (client_->GetCursor().type() != ui::mojom::CursorType::kIBeam) {
+ client_->UpdateInkCursor(ui::mojom::CursorType::kIBeam);
+ }
+}
+
PdfInkModule::DrawingStrokeState::DrawingStrokeState() = default;
PdfInkModule::DrawingStrokeState::~DrawingStrokeState() = default;
diff --git a/pdf/pdf_ink_module.h b/pdf/pdf_ink_module.h
index 14ff977dd265e..45adf9d766e4d 100644
--- a/pdf/pdf_ink_module.h
+++ b/pdf/pdf_ink_module.h
@@ -356,7 +356,7 @@ class PdfInkModule {
int click_count,
base::TimeTicks timestamp);
bool ContinueTextHighlight(const gfx::PointF& position);
- bool FinishTextHighlight();
+ bool FinishTextHighlight(const gfx::PointF& position);
// Returns a highlighter stroke that matches the position and size of
// `selection_rect`. `selection_rect` must be in screen coordinates.
@@ -455,8 +455,14 @@ class PdfInkModule {
void ApplyUndoRedoDiscards(
const PdfInkUndoRedoModel::DiscardedDrawCommands& discards);
+ // Sets the cursor to a drawing/erasing brush cursor when necessary.
void MaybeSetCursor();
+ // Handles setting the cursor only for mousemove events at `position`. This
+ // differs from `MaybeSetCursor()` in that it may also set the cursor to an
+ // I-beam for text highlighting.
+ void MaybeSetCursorOnMouseMove(const gfx::PointF& position);
+
// Returns whether the drawing brush was set or not.
bool MaybeSetDrawingBrush();
diff --git a/pdf/pdf_ink_module_client.h b/pdf/pdf_ink_module_client.h
index 731e5b2feb4eb..a4a280766f796 100644
--- a/pdf/pdf_ink_module_client.h
+++ b/pdf/pdf_ink_module_client.h
@@ -12,6 +12,7 @@
#include "pdf/pdf_ink_ids.h"
#include "pdf/ui/thumbnail.h"
#include "third_party/ink/src/ink/geometry/partitioned_mesh.h"
+#include "ui/base/cursor/cursor.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/vector2d.h"
@@ -55,6 +56,9 @@ class PdfInkModuleClient {
// `point`. `point` must be in device coordinates.
virtual void ExtendSelectionByPoint(const gfx::PointF& point) {}
+ // Returns the current cursor.
+ virtual ui::Cursor GetCursor() = 0;
+
// Gets the current page orientation.
virtual PageOrientation GetOrientation() const = 0;
diff --git a/pdf/pdf_ink_module_unittest.cc b/pdf/pdf_ink_module_unittest.cc
index 5a3225d339f5a..434cdba603060 100644
--- a/pdf/pdf_ink_module_unittest.cc
+++ b/pdf/pdf_ink_module_unittest.cc
@@ -199,6 +199,13 @@ std::map<int, std::vector<raw_ref<const ink::Stroke>>> CollectVisibleStrokes(
return visible_stroke_shapes;
}
+blink::WebMouseEvent CreateMouseMoveEventAtPoint(const gfx::PointF& point) {
+ return MouseEventBuilder()
+ .SetType(blink::WebInputEvent::Type::kMouseMove)
+ .SetPosition(point)
+ .Build();
+}
+
blink::WebMouseEvent CreateMouseMoveWithLeftButtonEventAtPoint(
const gfx::PointF& point) {
return MouseEventBuilder()
@@ -261,6 +268,8 @@ class FakeClient : public PdfInkModuleClient {
(const gfx::PointF& point),
(override));
+ MOCK_METHOD(ui::Cursor, GetCursor, (), (override));
+
PageOrientation GetOrientation() const override { return orientation_; }
MOCK_METHOD(std::vector<gfx::Rect>, GetSelectionRects, (), (override));
@@ -3098,6 +3107,8 @@ class PdfInkModuleTextHighlightTest : public PdfInkModuleStrokeTest {
.WillRepeatedly(Return(selection_rects));
EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kStartPointInsidePage0))
.WillRepeatedly(Return(true));
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kEndPointInsidePage0))
+ .WillRepeatedly(Return(true));
EXPECT_CALL(client(), OnTextOrLinkAreaClick(kStartPointInsidePage0,
/*click_count=*/1));
@@ -3330,10 +3341,12 @@ TEST_P(PdfInkModuleTextHighlightTest, MultipleSelection) {
.WillRepeatedly(Return(selection_rects));
EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kStartPointInsidePage0))
.WillRepeatedly(Return(true));
+ constexpr gfx::PointF kEndPoint2InsidePage0{25.0, 30.0};
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kEndPoint2InsidePage0))
+ .WillRepeatedly(Return(true));
EXPECT_CALL(client(), OnTextOrLinkAreaClick(kStartPointInsidePage0,
/*click_count=*/1));
- constexpr gfx::PointF kEndPoint2InsidePage0{25.0, 30.0};
EXPECT_CALL(client(), ExtendSelectionByPoint(kEndPoint2InsidePage0));
// Apply a text highlight stroke at the given points.
@@ -3663,7 +3676,7 @@ TEST_P(PdfInkModuleTextHighlightTest,
std::vector<gfx::Rect> selection_rects{gfx::Rect(9, 14, 5, 10)};
EXPECT_CALL(client(), GetSelectionRects())
.WillRepeatedly(Return(selection_rects));
- EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kMouseDownPoint))
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(_))
.WillRepeatedly(Return(true));
// There should not be any text selection extension after the miss, as the
@@ -3673,6 +3686,173 @@ TEST_P(PdfInkModuleTextHighlightTest,
RunStrokeMissedEndEventThenMouseMoveTest();
}
+TEST_P(PdfInkModuleTextHighlightTest, CursorOnMouseMove) {
+ EnableAnnotationMode();
+ InitializeSimpleSinglePageBasicLayout();
+
+ // Select the pen tool with a "Light Red" color. The cursor should be the
+ // custom pen cursor.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(8, 8))));
+
+ TestAnnotationBrushMessageParams params = {/*color_r=*/0xF2, /*color_g=*/0x8B,
+ /*color_b=*/0x82, /*size=*/6.0f};
+ SelectBrushTool(PdfInkBrush::Type::kPen, params);
+
+ // `kStartPointInsidePage0` will be the selectable text area position, while
+ // all other positions will be non-text areas.
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(_))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kStartPointInsidePage0))
+ .WillRepeatedly(Return(true));
+
+ // Move to a text position. The cursor should remain as the custom pen cursor.
+ blink::WebMouseEvent mouse_move_event =
+ CreateMouseMoveEventAtPoint(kEndPointInsidePage0);
+ // The event will be considered not handled, but the cursor will still update.
+ EXPECT_FALSE(ink_module().HandleInputEvent(mouse_move_event));
+
+ VerifyAndClearExpectations();
+
+ // Select the highlighter tool. The cursor should be the custom highlighter
+ // cursor.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(10, 10))));
+ params.size = 8.0f;
+ SelectBrushTool(PdfInkBrush::Type::kHighlighter, params);
+
+ VerifyAndClearExpectations();
+
+ // Move to a text position. The cursor should be an I-beam.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(ui::Cursor(ui::mojom::CursorType::kIBeam)));
+ mouse_move_event = CreateMouseMoveEventAtPoint(kStartPointInsidePage0);
+ EXPECT_FALSE(ink_module().HandleInputEvent(mouse_move_event));
+
+ VerifyAndClearExpectations();
+
+ // Move to a non-text position. The cursor should restore to the custom
+ // highlighter cursor.
+ {
+ InSequence seq;
+ EXPECT_CALL(client(), GetCursor())
+ .WillOnce(Return(ui::mojom::CursorType::kIBeam));
+ EXPECT_CALL(client(),
+ UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(10, 10))));
+ }
+ mouse_move_event = CreateMouseMoveEventAtPoint(kEndPointInsidePage0);
+ EXPECT_FALSE(ink_module().HandleInputEvent(mouse_move_event));
+}
+
+TEST_P(PdfInkModuleTextHighlightTest, CursorOnMouseMoveWhileTextSelecting) {
+ EnableAnnotationMode();
+ InitializeSimpleSinglePageBasicLayout();
+
+ // Select the highlighter tool. The cursor should be the custom highlighter
+ // cursor.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(10, 10))));
+ TestAnnotationBrushMessageParams params = {/*color_r=*/0xF2, /*color_g=*/0x8B,
+ /*color_b=*/0x82, /*size=*/8.0f};
+ SelectBrushTool(PdfInkBrush::Type::kHighlighter, params);
+
+ VerifyAndClearExpectations();
+
+ // `kStartPointInsidePage0` will be the selectable text area position, while
+ // all other positions will be non-text areas.
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(_))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kStartPointInsidePage0))
+ .WillRepeatedly(Return(true));
+
+ // Move to a text position. The cursor should be an I-beam.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(ui::Cursor(ui::mojom::CursorType::kIBeam)));
+ blink::WebMouseEvent mouse_move_event =
+ CreateMouseMoveEventAtPoint(kStartPointInsidePage0);
+ EXPECT_FALSE(ink_module().HandleInputEvent(mouse_move_event));
+
+ VerifyAndClearExpectations();
+
+ // Start text highlighting and move to a non-text position. The cursor should
+ // remain as an I-beam.
+ EXPECT_CALL(client(), UpdateInkCursor(_)).Times(0);
+ blink::WebMouseEvent mouse_down_event =
+ MouseEventBuilder()
+ .CreateLeftClickAtPosition(kStartPointInsidePage0)
+ .Build();
+ EXPECT_TRUE(ink_module().HandleInputEvent(mouse_down_event));
+ mouse_move_event =
+ CreateMouseMoveWithLeftButtonEventAtPoint(kEndPointInsidePage0);
+ EXPECT_TRUE(ink_module().HandleInputEvent(mouse_move_event));
+
+ VerifyAndClearExpectations();
+
+ // End text highlighting. The cursor should restore to the custom highlighter
+ // cursor.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(10, 10))));
+ blink::WebMouseEvent mouse_up_event =
+ MouseEventBuilder()
+ .CreateLeftMouseUpAtPosition(kEndPointInsidePage0)
+ .Build();
+ EXPECT_TRUE(ink_module().HandleInputEvent(mouse_up_event));
+}
+
+TEST_P(PdfInkModuleTextHighlightTest, CursorOnMouseMoveWhileBrushDrawing) {
+ EnableAnnotationMode();
+ InitializeSimpleSinglePageBasicLayout();
+
+ // Select the highlighter tool. The cursor should be the custom highlighter
+ // cursor.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(10, 10))));
+ TestAnnotationBrushMessageParams params = {/*color_r=*/0xF2, /*color_g=*/0x8B,
+ /*color_b=*/0x82, /*size=*/8.0f};
+ SelectBrushTool(PdfInkBrush::Type::kHighlighter, params);
+
+ VerifyAndClearExpectations();
+
+ // `kEndPointInsidePage0` will be the selectable text area position, while
+ // all other positions will be non-text areas.
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(_))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(client(), IsSelectableTextOrLinkArea(kEndPointInsidePage0))
+ .WillRepeatedly(Return(true));
+
+ // Move to a non-text position. The cursor should remain as the custom
+ // highlighter cursor.
+ EXPECT_CALL(client(), UpdateInkCursor(_)).Times(0);
+ blink::WebMouseEvent mouse_move_event =
+ CreateMouseMoveEventAtPoint(kStartPointInsidePage0);
+ EXPECT_FALSE(ink_module().HandleInputEvent(mouse_move_event));
+
+ VerifyAndClearExpectations();
+
+ // Start drawing and move to a text position. The cursor should remain as the
+ // custom highlighter cursor.
+ EXPECT_CALL(client(), UpdateInkCursor(_)).Times(0);
+ blink::WebMouseEvent mouse_down_event =
+ MouseEventBuilder()
+ .CreateLeftClickAtPosition(kStartPointInsidePage0)
+ .Build();
+ EXPECT_TRUE(ink_module().HandleInputEvent(mouse_down_event));
+ mouse_move_event =
+ CreateMouseMoveWithLeftButtonEventAtPoint(kEndPointInsidePage0);
+ EXPECT_TRUE(ink_module().HandleInputEvent(mouse_move_event));
+
+ VerifyAndClearExpectations();
+
+ // End drawing. The cursor should be an I-beam.
+ EXPECT_CALL(client(),
+ UpdateInkCursor(ui::Cursor(ui::mojom::CursorType::kIBeam)));
+ blink::WebMouseEvent mouse_up_event =
+ MouseEventBuilder()
+ .CreateLeftMouseUpAtPosition(kEndPointInsidePage0)
+ .Build();
+ EXPECT_TRUE(ink_module().HandleInputEvent(mouse_up_event));
+}
+
INSTANTIATE_TEST_SUITE_P(All,
PdfInkModuleTest,
testing::ValuesIn(GetAllInkTestVariations()));
diff --git a/pdf/pdf_view_web_plugin.cc b/pdf/pdf_view_web_plugin.cc
index 7eed5a29b126b..bcb7097e3252b 100644
--- a/pdf/pdf_view_web_plugin.cc
+++ b/pdf/pdf_view_web_plugin.cc
@@ -300,6 +300,8 @@ class PdfViewWebPlugin::PdfInkModuleClientImpl : public PdfInkModuleClient {
plugin_->engine_->ExtendSelectionByPoint(point);
}
+ ui::Cursor GetCursor() override { return plugin_->cursor_; }
+
PageOrientation GetOrientation() const override {
return plugin_->engine_->GetCurrentOrientation();
}
diff --git a/pdf/pdf_view_web_plugin_unittest.cc b/pdf/pdf_view_web_plugin_unittest.cc
index 9d6d226d0a8df..87ade8d6e05b0 100644
--- a/pdf/pdf_view_web_plugin_unittest.cc
+++ b/pdf/pdf_view_web_plugin_unittest.cc
@@ -3049,6 +3049,16 @@ TEST_P(PdfViewWebPluginInkTest, SendThumbnailWithNoStrokes) {
SendThumbnail(/*message_id=*/"foo", /*page_size=*/{50, 25});
}
+TEST_P(PdfViewWebPluginInkTest, GetCursor) {
+ plugin_->set_cursor_type_for_testing(ui::mojom::CursorType::kPointer);
+ EXPECT_EQ(ui::mojom::CursorType::kPointer,
+ plugin_->ink_module_client_for_testing()->GetCursor().type());
+
+ plugin_->set_cursor_type_for_testing(ui::mojom::CursorType::kIBeam);
+ EXPECT_EQ(ui::mojom::CursorType::kIBeam,
+ plugin_->ink_module_client_for_testing()->GetCursor().type());
+}
+
TEST_P(PdfViewWebPluginInkTest, UpdateCursor) {
UpdatePluginGeometryWithoutWaiting(2.0f, {0, 0, 20, 20});