PDF Viewer - Links should open on mouse up

pdf/pdfium/pdfium_engine.cc

Index: pdf/pdfium/pdfium_engine.cc
diff --git a/pdf/pdfium/pdfium_engine.cc b/pdf/pdfium/pdfium_engine.cc
index 83c83f4fe439c59c8d971de0d1fbe68ba1d1fda5..8ad0f4b5bfb66741cb1d0c7f9383212783d116dd 100644
--- a/pdf/pdfium/pdfium_engine.cc
+++ b/pdf/pdfium/pdfium_engine.cc
@@ -530,6 +530,7 @@ PDFiumEngine::PDFiumEngine(PDFEngine::Client* client)
       form_(NULL),
       defer_page_unload_(false),
       selecting_(false),
+      open_link_(false),
       next_page_to_search_(-1),
       last_page_to_search_(-1),
       last_character_index_to_search_(-1),
@@ -1288,9 +1289,7 @@ bool PDFiumEngine::OnMouseDown(const pp::MouseInputEvent& event) {
   PDFiumPage::Area area = GetCharIndex(event, &page_index,
                                        &char_index, &target);
   if (area == PDFiumPage::WEBLINK_AREA) {
-    bool open_in_new_tab = !!(event.GetModifiers() & kDefaultKeyModifier);
-    client_->NavigateTo(target.url, open_in_new_tab);
-    client_->FormTextFieldFocusChange(false);

[raymes, 2014/09/08 01:52:57]

Would it be better to store the information related to the mouse down in a
struct:

mouse_down_state_ = MouseDownState(area, target);

(this could be done outside of this if-statement)

Then in OnMouseUp, you could say:

if (mouse_down_state_ == MouseDownState(area, target)) {
  if (area == PDFiumPage::WEBLINK_AREA) {
    // open the link
  }
  // Possibly handle the scroll case too.
}

Then in OnMouseMove:

if (mouse_down_state_ != MouseDownState(area, target)) {
  mouse_down_state_ = MouseDownState();
}

What do you think?

[Nikhil, 2014/09/08 09:44:53]

Thanks for your suggestion, I agree that it could be useful to store mouse down
information. I've updated this CL accordingly. PTAL.

+    open_link_ = true;
     return true;
   }
 
@@ -1370,7 +1369,18 @@ bool PDFiumEngine::OnMouseUp(const pp::MouseInputEvent& event) {
 
   int page_index = -1;
   int char_index = -1;
-  GetCharIndex(event, &page_index, &char_index, NULL);
+  PDFiumPage::LinkTarget target;
+  PDFiumPage::Area area =
+      GetCharIndex(event, &page_index, &char_index, &target);
+
+  if (open_link_ && area == PDFiumPage::WEBLINK_AREA) {
+    bool open_in_new_tab = !!(event.GetModifiers() & kDefaultKeyModifier);
+    client_->NavigateTo(target.url, open_in_new_tab);
+    client_->FormTextFieldFocusChange(false);
+    open_link_ = false;
+    return true;
+  }
+
   if (page_index != -1) {
     double page_x, page_y;
     pp::Point point = event.GetPosition();
@@ -1390,6 +1400,10 @@ bool PDFiumEngine::OnMouseMove(const pp::MouseInputEvent& event) {
   int page_index = -1;
   int char_index = -1;
   PDFiumPage::Area area = GetCharIndex(event, &page_index, &char_index, NULL);
+
+  if (open_link_ && area != PDFiumPage::WEBLINK_AREA)
+    open_link_ = false;
+
   if (!selecting_) {
     PP_CursorType_Dev cursor;
     switch (area) {