Show handles even when dragging from outside selectable region

content/browser/renderer_host/input/gesture_text_selector.cc

 // Copyright 2014 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 "content/browser/renderer_host/input/gesture_text_selector.h"
 
 #include "ui/events/event_constants.h"
 #include "ui/events/gesture_detection/gesture_event_data.h"
 #include "ui/events/gesture_detection/motion_event.h"
 
@@ skipping 31 matching lines @@
       client_->Unselect();
       // TODO(changwan): check if we can show handles on ET_GESTURE_SCROLL_END
       // instead. Currently it is not possible as ShowSelectionHandles should
       // be called before we change the selection.
       client_->ShowSelectionHandlesAutomatically();
       anchor_x_ = gesture.x;
       anchor_y_ = gesture.y;
       break;
     }
     case ui::ET_GESTURE_SCROLL_UPDATE: {
+      client_->ShowSelectionHandlesAutomatically();

[jdduke (slow), 2014/07/25 14:07:27]

Any idea why the call to |client_->ShowSelectionHandlesAutomatically();| from
ET_GESTURE_SCROLL_BEGIN is insufficient?

[jdduke (slow), 2014/07/25 14:25:03]

I wonder if the |Unselect()| call is triggering a new selection update which
"consumes" the automatic showing.  In any case, with this change I would remove
the |client_->ShowSelectionHandlesAutomatically()| call entirely from
ET_GESTURE_SCROLL_BEGIN.  With that, lgtm (although it would still be good to
verify the exact order of events making the previous solution insufficient), and
please validate the callback behavior in an existing test (or new one). 

I'm still wondering if it would be better to show the handles *after* the
dragging is finished, but that's a discussion for another day.

[Changwan Ryu, 2014/07/25 22:12:47]

Removed ShowSelectionHandlesAutomatically() from ET_GESTURE_SCROLL_BEGIN.

This only occurs when dragging starts in unselectable region, so I suspect that
we hide selection handles when it is not applicable (selected region is null). I
agree that showing handles on ET_GESTURE_SCROLL_END is much neater, but I'd like
to address that in the future. (Seems that refactoring is being done. :) )

On 2014/07/25 14:25:03, jdduke wrote:

       client_->SelectRange(anchor_x_, anchor_y_, gesture.x, gesture.y);
       break;
     }
     default:
       // Suppress all other gestures when we are selecting text.
       break;
   }
   return true;
 }
 
 // static
 bool GestureTextSelector::ShouldStartTextSelection(
     const ui::MotionEvent& event) {
   DCHECK_GT(event.GetPointerCount(), 0u);
   // Currently we are supporting stylus-only cases.
   const bool is_stylus =
       event.GetToolType(0) == ui::MotionEvent::TOOL_TYPE_STYLUS;
   const bool is_only_secondary_button_pressed =
       event.GetButtonState() == ui::MotionEvent::BUTTON_SECONDARY;
   return is_stylus && is_only_secondary_button_pressed;
 }
 
 }  // namespace content