PDF Viewer - Links should open on mouse up

pdf/pdfium/pdfium_engine.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "pdf/pdfium/pdfium_engine.h"
 
 #include <math.h>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ skipping 512 matching lines @@
 PDFiumEngine::PDFiumEngine(PDFEngine::Client* client)
     : client_(client),
       current_zoom_(1.0),
       current_rotation_(0),
       doc_loader_(this),
       password_tries_remaining_(0),
       doc_(NULL),
       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),
       current_find_index_(-1),
       permissions_(0),
       fpdf_availability_(NULL),
       next_timer_id_(0),
       last_page_mouse_down_(-1),
       first_visible_page_(-1),
       most_visible_page_(-1),
@@ skipping 738 matching lines @@
 
   SelectionChangeInvalidator selection_invalidator(this);
   selection_.clear();
 
   int page_index = -1;
   int char_index = -1;
   PDFiumPage::LinkTarget target;
   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;
   }
 
   if (area == PDFiumPage::DOCLINK_AREA) {
     client_->ScrollToPage(target.page);
     client_->FormTextFieldFocusChange(false);
     return true;
   }
 
   if (page_index != -1) {
@@ skipping 59 matching lines @@
   selection_.push_back(PDFiumRange(
       pages_[page_index], start_index, end_index - start_index));
 }
 
 bool PDFiumEngine::OnMouseUp(const pp::MouseInputEvent& event) {
   if (event.GetButton() != PP_INPUTEVENT_MOUSEBUTTON_LEFT)
     return false;
 
   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();
     DeviceToPage(page_index, point.x(), point.y(), &page_x, &page_y);
     FORM_OnLButtonUp(
         form_, pages_[page_index]->GetPage(), 0, page_x, page_y);
   }
 
   if (!selecting_)
     return false;
 
   selecting_ = false;
   return true;
 }
 
 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) {
       case PDFiumPage::TEXT_AREA:
         cursor = PP_CURSORTYPE_IBEAM;
         break;
       case PDFiumPage::WEBLINK_AREA:
       case PDFiumPage::DOCLINK_AREA:
         cursor = PP_CURSORTYPE_HAND;
         break;
@@ skipping 2019 matching lines @@
     double* height) {
   FPDF_DOCUMENT doc = FPDF_LoadMemDocument(pdf_buffer, pdf_buffer_size, NULL);
   if (!doc)
     return false;
   bool success = FPDF_GetPageSizeByIndex(doc, page_number, width, height) != 0;
   FPDF_CloseDocument(doc);
   return success;
 }
 
 }  // namespace chrome_pdf