Fix discrepancies in form text selection between PDF and HTML forms.

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 <stddef.h>
 #include <stdint.h>
 
@@ skipping 1620 matching lines @@
 }
 
 void PDFiumEngine::SetFormSelectedText(FPDF_FORMHANDLE form_handle,
                                        FPDF_PAGE page) {
   unsigned long form_sel_text_len =
       FORM_GetSelectedText(form_handle, page, nullptr, 0);
 
   // Check to see if there is selected text in the form. When
   // |form_sel_text_len| is 2, that represents a wide string with just a
   // NUL-terminator.
-  if (form_sel_text_len <= 2)
+  if (form_sel_text_len <= 2 && old_selected_form_text_.empty())
     return;
 
   base::string16 selected_form_text16;
   PDFiumAPIStringBufferSizeInBytesAdapter<base::string16> string_adapter(
       &selected_form_text16, form_sel_text_len, false);
   string_adapter.Close(FORM_GetSelectedText(
       form_handle, page, string_adapter.GetData(), form_sel_text_len));
 
-  std::string selected_form_text = base::UTF16ToUTF8(selected_form_text16);
-  if (!selected_form_text.empty()) {
-    pp::PDF::SetSelectedText(GetPluginInstance(), selected_form_text.c_str());
-  }
+  // Update previous and current selections, then compare them to check if

[Lei Zhang, 2017/06/29 02:52:46]

If this method does not have |old_selected_form_text_|, and this code just
became:

  std::string selected_form_text = base::UTF16ToUTF8(selected_form_text16);
  if (selected_form_text != selected_form_text_) {
    selected_form_text_ = selected_form_text;
    pp::PDF::SetSelectedText(GetPluginInstance(), selected_form_text_.c_str());
  }

Would that work? Can you explain what purpose |old_selected_form_text_| serves?

[drgage, 2017/06/29 17:49:59]

Yes, something like that would work. That's actually what I had first, but I
changed it so that |old_selected_text_| was a member variable because I was
thinking it was necessary for making the check on line 1641 and exiting early if
possible. Looking at it again today, I see that it's not necessary and the check
can be made without it.

+  // selection has changed. If so, set plugin text selection.
+  old_selected_form_text_ = selected_form_text_;
+  selected_form_text_ = base::UTF16ToUTF8(selected_form_text16);
+  if (old_selected_form_text_ != selected_form_text_)
+    pp::PDF::SetSelectedText(GetPluginInstance(), selected_form_text_.c_str());
 }
 
 void PDFiumEngine::PrintEnd() {
   FORM_DoDocumentAAction(form_, FPDFDOC_AACTION_DP);
 }
 
 PDFiumPage::Area PDFiumEngine::GetCharIndex(const pp::MouseInputEvent& event,
                                             int* page_index,
                                             int* char_index,
                                             int* form_type,
@@ skipping 58 matching lines @@
   SelectionChangeInvalidator selection_invalidator(this);
   selection_.clear();
 
   int page_index = -1;
   int char_index = -1;
   int form_type = FPDF_FORMFIELD_UNKNOWN;
   PDFiumPage::LinkTarget target;
   PDFiumPage::Area area =
       GetCharIndex(event, &page_index, &char_index, &form_type, &target);
   mouse_down_state_.Set(area, target);
+  SetMouseHeldDown(true);
 
   // Decide whether to open link or not based on user action in mouse up and
   // mouse move events.
   if (area == PDFiumPage::WEBLINK_AREA || area == PDFiumPage::DOCLINK_AREA)
     return true;
 
   // Prevent middle mouse button from selecting texts.
   if (event.GetButton() == PP_INPUTEVENT_MOUSEBUTTON_MIDDLE)
     return false;
 
@@ skipping 71 matching lines @@
       event.GetButton() != PP_INPUTEVENT_MOUSEBUTTON_MIDDLE) {
     return false;
   }
 
   int page_index = -1;
   int char_index = -1;
   int form_type = FPDF_FORMFIELD_UNKNOWN;
   PDFiumPage::LinkTarget target;
   PDFiumPage::Area area =
       GetCharIndex(event, &page_index, &char_index, &form_type, &target);
+  SetMouseHeldDown(false);
 
   // Open link on mouse up for same link for which mouse down happened earlier.
   if (mouse_down_state_.Matches(area, target)) {
     if (area == PDFiumPage::WEBLINK_AREA) {
       uint32_t modifiers = event.GetModifiers();
       bool middle_button =
           !!(modifiers & PP_INPUTEVENT_MODIFIER_MIDDLEBUTTONDOWN);
       bool alt_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_ALTKEY);
       bool ctrl_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_CONTROLKEY);
       bool meta_key = !!(modifiers & PP_INPUTEVENT_MODIFIER_METAKEY);
@@ skipping 17 matching lines @@
   if (event.GetButton() == PP_INPUTEVENT_MOUSEBUTTON_MIDDLE)
     return false;
 
   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 (area == PDFiumPage::FORM_TEXT_AREA)

[Lei Zhang, 2017/06/29 02:52:46]

This change is a bit out of date and won't apply anymore.

[drgage, 2017/06/29 17:49:58]

I'm not sure what you mean. I removed this check because it doesn't apply
anymore.

[Lei Zhang, 2017/06/29 22:38:04]

Didn't you already change this block of code to also check:
last_page_mouse_down_ != -1 ? That change needs to be reflected in the diff, for
it to apply correctly. i.e. your local git branch associated with this CL likely
does not include https://crbug.com/483133 .

[drgage, 2017/06/29 23:00:50]

Oh yeah, at the time that I started working on this I had checked out this
branch from master, not the branch where I fixed that. But now since master
includes that change, I'll rebase and upload a new patchset.

-    SetFormSelectedText(form_, pages_[last_page_mouse_down_]->GetPage());
-
   if (!selecting_)
     return false;
 
   SetSelecting(false);
   return true;
 }
 
 bool PDFiumEngine::OnMouseMove(const pp::MouseInputEvent& event) {
   int page_index = -1;
   int char_index = -1;
@@ skipping 44 matching lines @@
       DeviceToPage(page_index, point.x(), point.y(), &page_x, &page_y);
       FORM_OnMouseMove(form_, pages_[page_index]->GetPage(), 0, page_x, page_y);
     }
 
     client_->UpdateCursor(cursor);
     std::string url = GetLinkAtPosition(event.GetPosition());
     if (url != link_under_cursor_) {
       link_under_cursor_ = url;
       pp::PDF::SetLinkUnderCursor(GetPluginInstance(), url.c_str());
     }
+
+    // If in form text area while left mouse button is held down, check if form
+    // text selection needs to be updated.
+    if (mouse_held_down_ && area == PDFiumPage::FORM_TEXT_AREA &&
+        last_page_mouse_down_ != -1) {
+      SetFormSelectedText(form_, pages_[last_page_mouse_down_]->GetPage());
+    }
+
     // No need to swallow the event, since this might interfere with the
     // scrollbars if the user is dragging them.
     return false;
   }
 
   // We're selecting but right now we're not over text, so don't change the
   // current selection.
   if (area != PDFiumPage::TEXT_AREA && area != PDFiumPage::WEBLINK_AREA &&
       area != PDFiumPage::DOCLINK_AREA) {
     return false;
@@ skipping 74 matching lines @@
     // http://chrome-corpsvn.mtv.corp.google.com/viewvc?view=rev&root=chrome&revision=31805
     // for more information.  So just fake one since PDFium uses it.
     std::string str;
     str.push_back(event.GetKeyCode());
     pp::KeyboardInputEvent synthesized(pp::KeyboardInputEvent(
         client_->GetPluginInstance(), PP_INPUTEVENT_TYPE_CHAR,
         event.GetTimeStamp(), event.GetModifiers(), event.GetKeyCode(), str));
     OnChar(synthesized);
   }
 
-  // If form selected text is empty and key pressed within form text area,
-  // plugin text selection should be cleared.
-  if (in_form_text_area_ &&
-      FORM_GetSelectedText(form_, pages_[last_page_mouse_down_]->GetPage(),
-                           nullptr, 0) <= 2) {
-    pp::PDF::SetSelectedText(GetPluginInstance(), "");
-  }
-
   return rv;
 }
 
 bool PDFiumEngine::OnKeyUp(const pp::KeyboardInputEvent& event) {
   if (last_page_mouse_down_ == -1)
     return false;
 
+  // Check if form text selection needs to be updated.
   if (in_form_text_area_) {
-    if (event.GetKeyCode() == ui::VKEY_SHIFT)
-      SetFormSelectedText(form_, pages_[last_page_mouse_down_]->GetPage());
+    SetFormSelectedText(form_, pages_[last_page_mouse_down_]->GetPage());
   }
 
   return !!FORM_OnKeyUp(form_, pages_[last_page_mouse_down_]->GetPage(),
                         event.GetKeyCode(), event.GetModifiers());
 }
 
 bool PDFiumEngine::OnChar(const pp::KeyboardInputEvent& event) {
   if (last_page_mouse_down_ == -1)
     return false;
 
@@ skipping 1554 matching lines @@
   selecting_ = selecting;
   if (selecting_ != was_selecting)
     client_->IsSelectingChanged(selecting);
 }
 
 void PDFiumEngine::SetInFormTextArea(bool in_form_text_area) {
   client_->FormTextFieldFocusChange(in_form_text_area);
   in_form_text_area_ = in_form_text_area;
 }
 
+void PDFiumEngine::SetMouseHeldDown(bool mouse_held_down) {
+  mouse_held_down_ = mouse_held_down;
+}
+
 void PDFiumEngine::ScheduleTouchTimer(const pp::TouchInputEvent& evt) {
   touch_timers_[++next_touch_timer_id_] = evt;
   client_->ScheduleTouchTimerCallback(next_touch_timer_id_,
                                       kTouchLongPressTimeoutMs);
 }
 
 void PDFiumEngine::KillTouchTimer(int timer_id) {
   touch_timers_.erase(timer_id);
 }
 
@@ skipping 569 matching lines @@
   FPDF_DOCUMENT doc =
       FPDF_LoadMemDocument(pdf_buffer, pdf_buffer_size, nullptr);
   if (!doc)
     return false;
   bool success = FPDF_GetPageSizeByIndex(doc, page_number, width, height) != 0;
   FPDF_CloseDocument(doc);
   return success;
 }
 
 }  // namespace chrome_pdf