From 824102c0a5b9150698703f11ee1500d84eb89eb1 Mon Sep 17 00:00:00 2001 From: mohsen Date: Thu, 5 Nov 2015 14:27:20 -0800 Subject: Show quick menu for insertion handle on long-press In order to match Chrome on Android behavior, made a few more changes to insertion handle behavior: - Tapping on an empty textfield does not show the insertion handle. - When an insertion handle is shown as a result of a long-press, if there is any action available to show in the quick menu, the quick menu is shown; otherise, the context menu is shown as usual. BUG=520928 Review URL: https://codereview.chromium.org/1421073003 Cr-Commit-Position: refs/heads/master@{#358148} --- .../touch_selection_controller_client_aura.cc | 44 +++++++++-------- .../input/touch_selection_controller_client_aura.h | 14 +++++- ...selection_controller_client_aura_browsertest.cc | 52 ++++++++++++++++++++ .../renderer_host/render_widget_host_view_aura.cc | 2 +- .../browser/web_contents/web_contents_view_aura.cc | 9 ++-- content/test/data/touch_selection.html | 6 ++- ui/touch_selection/touch_selection_controller.h | 2 + ui/touch_selection/touch_selection_menu_runner.h | 4 ++ .../touchui/touch_selection_menu_runner_views.cc | 56 ++++++++++------------ .../touchui/touch_selection_menu_runner_views.h | 2 + 10 files changed, 134 insertions(+), 57 deletions(-) diff --git a/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc b/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc index be8f2b0..2cbc8a1 100644 --- a/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc +++ b/content/browser/renderer_host/input/touch_selection_controller_client_aura.cc @@ -9,6 +9,7 @@ #include "content/browser/renderer_host/render_widget_host_view_aura.h" #include "content/common/view_messages.h" #include "content/public/browser/render_view_host.h" +#include "content/public/common/context_menu_params.h" #include "ui/aura/client/cursor_client.h" #include "ui/aura/client/screen_position_client.h" #include "ui/aura/env.h" @@ -116,10 +117,10 @@ TouchSelectionControllerClientAura::TouchSelectionControllerClientAura( base::Bind(&TouchSelectionControllerClientAura::ShowQuickMenu, base::Unretained(this)), false), + quick_menu_requested_(false), touch_down_(false), scroll_in_progress_(false), - handle_drag_in_progress_(false), - insertion_quick_menu_allowed_(true) { + handle_drag_in_progress_(false) { DCHECK(rwhva_); } @@ -152,23 +153,23 @@ void TouchSelectionControllerClientAura::OnScrollCompleted() { UpdateQuickMenu(); } -bool TouchSelectionControllerClientAura::IsQuickMenuAllowed() const { - if (touch_down_ || scroll_in_progress_ || handle_drag_in_progress_) - return false; - - switch (rwhva_->selection_controller()->active_status()) { - case ui::TouchSelectionController::INACTIVE: - return false; - case ui::TouchSelectionController::INSERTION_ACTIVE: - return insertion_quick_menu_allowed_; - case ui::TouchSelectionController::SELECTION_ACTIVE: - return true; +bool TouchSelectionControllerClientAura::HandleContextMenu( + const ContextMenuParams& params) { + if (params.source_type == ui::MENU_SOURCE_TOUCH && params.is_editable && + params.selection_text.empty() && IsQuickMenuAvailable()) { + quick_menu_requested_ = true; + UpdateQuickMenu(); + return true; } - - NOTREACHED(); + rwhva_->selection_controller()->HideAndDisallowShowingAutomatically(); return false; } +bool TouchSelectionControllerClientAura::IsQuickMenuAvailable() const { + return ui::TouchSelectionMenuRunner::GetInstance() && + ui::TouchSelectionMenuRunner::GetInstance()->IsMenuAvailable(this); +} + void TouchSelectionControllerClientAura::ShowQuickMenu() { if (!ui::TouchSelectionMenuRunner::GetInstance()) return; @@ -213,8 +214,12 @@ void TouchSelectionControllerClientAura::UpdateQuickMenu() { else quick_menu_timer_.Stop(); + bool should_show_menu = quick_menu_requested_ && !touch_down_ && + !scroll_in_progress_ && !handle_drag_in_progress_ && + IsQuickMenuAvailable(); + // Start timer to show quick menu if necessary. - if (IsQuickMenuAllowed()) { + if (should_show_menu) { if (show_quick_menu_immediately_for_test_) ShowQuickMenu(); else @@ -260,8 +265,9 @@ void TouchSelectionControllerClientAura::OnSelectionEvent( ui::SelectionEventType event) { switch (event) { case ui::SELECTION_HANDLES_SHOWN: + quick_menu_requested_ = true; + // Fall through. case ui::INSERTION_HANDLE_SHOWN: - insertion_quick_menu_allowed_ = false; UpdateQuickMenu(); env_pre_target_handler_.reset(new EnvPreTargetHandler( rwhva_->selection_controller(), rwhva_->GetNativeView())); @@ -269,6 +275,7 @@ void TouchSelectionControllerClientAura::OnSelectionEvent( case ui::SELECTION_HANDLES_CLEARED: case ui::INSERTION_HANDLE_CLEARED: env_pre_target_handler_.reset(); + quick_menu_requested_ = false; UpdateQuickMenu(); break; case ui::SELECTION_HANDLE_DRAG_STARTED: @@ -283,11 +290,10 @@ void TouchSelectionControllerClientAura::OnSelectionEvent( break; case ui::SELECTION_HANDLES_MOVED: case ui::INSERTION_HANDLE_MOVED: - insertion_quick_menu_allowed_ = false; UpdateQuickMenu(); break; case ui::INSERTION_HANDLE_TAPPED: - insertion_quick_menu_allowed_ = !insertion_quick_menu_allowed_; + quick_menu_requested_ = !quick_menu_requested_; UpdateQuickMenu(); break; case ui::SELECTION_ESTABLISHED: diff --git a/content/browser/renderer_host/input/touch_selection_controller_client_aura.h b/content/browser/renderer_host/input/touch_selection_controller_client_aura.h index 0c2f6fe..ec9ffd5 100644 --- a/content/browser/renderer_host/input/touch_selection_controller_client_aura.h +++ b/content/browser/renderer_host/input/touch_selection_controller_client_aura.h @@ -11,6 +11,7 @@ #include "ui/touch_selection/touch_selection_menu_runner.h" namespace content { +struct ContextMenuParams; class RenderWidgetHostViewAura; // An implementation of |TouchSelectionControllerClient| to be used in Aura's @@ -35,11 +36,20 @@ class CONTENT_EXPORT TouchSelectionControllerClientAura void OnScrollStarted(); void OnScrollCompleted(); + // Gives an opportunity to the client to handle context menu request and show + // the quick menu instead, if appropriate. Returns |true| to indicate that no + // further handling is needed. + // TODO(mohsen): This is to match Chrome on Android behavior. However, it is + // better not to send context menu request from the renderer in this case and + // instead decide in the client about showing the quick menu in response to + // selection events. (http://crbug.com/548245) + bool HandleContextMenu(const ContextMenuParams& params); + private: friend class TestTouchSelectionControllerClientAura; class EnvPreTargetHandler; - bool IsQuickMenuAllowed() const; + bool IsQuickMenuAvailable() const; void ShowQuickMenu(); void UpdateQuickMenu(); @@ -62,10 +72,10 @@ class CONTENT_EXPORT TouchSelectionControllerClientAura RenderWidgetHostViewAura* rwhva_; base::Timer quick_menu_timer_; + bool quick_menu_requested_; bool touch_down_; bool scroll_in_progress_; bool handle_drag_in_progress_; - bool insertion_quick_menu_allowed_; bool show_quick_menu_immediately_for_test_; diff --git a/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc b/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc index 102a560..ac432e9 100644 --- a/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc +++ b/content/browser/renderer_host/input/touch_selection_controller_client_aura_browsertest.cc @@ -46,6 +46,11 @@ class TestTouchSelectionMenuRunner : public ui::TouchSelectionMenuRunner { ~TestTouchSelectionMenuRunner() override {} private: + bool IsMenuAvailable( + const ui::TouchSelectionMenuClient* client) const override { + return true; + } + void OpenMenu(ui::TouchSelectionMenuClient* client, const gfx::Rect& anchor_rect, const gfx::Size& handle_image_size, @@ -142,6 +147,11 @@ class TouchSelectionControllerClientAuraTest : public ContentBrowserTest { return false; } + bool EmptyTextfield() { + return ExecuteScript(shell()->web_contents()->GetMainFrame(), + "empty_textfield()"); + } + private: void SetUpOnMainThread() override { ContentBrowserTest::SetUpOnMainThread(); @@ -252,6 +262,48 @@ IN_PROC_BROWSER_TEST_F(TouchSelectionControllerClientAuraTest, EXPECT_FALSE(ui::TouchSelectionMenuRunner::GetInstance()->IsRunning()); } +// Tests that tapping in an empty textfield does not bring up the insertion +// handle. +IN_PROC_BROWSER_TEST_F(TouchSelectionControllerClientAuraTest, + EmptyTextfieldInsertionOnTap) { + // Set the test page up. + ASSERT_NO_FATAL_FAILURE(StartTestWithPage("/touch_selection.html")); + WebContents* web_contents = + static_cast(shell()->web_contents()); + RenderWidgetHostViewAura* rwhva = static_cast( + web_contents->GetRenderWidgetHostView()); + TestTouchSelectionControllerClientAura* selection_controller_client = + new TestTouchSelectionControllerClientAura(rwhva); + rwhva->SetSelectionControllerClientForTest( + make_scoped_ptr(selection_controller_client)); + + // Clear textfield contents. + ASSERT_TRUE(EmptyTextfield()); + + EXPECT_EQ(ui::TouchSelectionController::INACTIVE, + rwhva->selection_controller()->active_status()); + EXPECT_FALSE(ui::TouchSelectionMenuRunner::GetInstance()->IsRunning()); + + // Tap inside the textfield and wait for the insertion cursor. + selection_controller_client->InitWaitForSelectionEvent( + ui::SELECTION_ESTABLISHED); + + gfx::PointF point; + ASSERT_TRUE(GetPointInsideTextfield(&point)); + ui::GestureEventDetails tap_details(ui::ET_GESTURE_TAP); + tap_details.set_tap_count(1); + ui::GestureEvent tap(point.x(), point.y(), 0, ui::EventTimeForNow(), + tap_details); + rwhva->OnGestureEvent(&tap); + + selection_controller_client->Wait(); + + // Check that insertion is not active and the quick menu is not showing. + EXPECT_EQ(ui::TouchSelectionController::INACTIVE, + rwhva->selection_controller()->active_status()); + EXPECT_FALSE(ui::TouchSelectionMenuRunner::GetInstance()->IsRunning()); +} + // Tests that the quick menu is hidden whenever a touch point is active. IN_PROC_BROWSER_TEST_F(TouchSelectionControllerClientAuraTest, QuickMenuHiddenOnTouch) { diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc index bbb79c9..22603d2 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura.cc @@ -2780,7 +2780,7 @@ void RenderWidgetHostViewAura::CreateSelectionController() { ui::GestureConfiguration::GetInstance()->long_press_time_in_ms()); tsc_config.tap_slop = ui::GestureConfiguration::GetInstance() ->max_touch_move_in_pixels_for_click(); - tsc_config.show_on_tap_for_empty_editable = true; + tsc_config.show_on_tap_for_empty_editable = false; tsc_config.enable_longpress_drag_selection = false; selection_controller_.reset(new ui::TouchSelectionController( selection_controller_client_.get(), tsc_config)); diff --git a/content/browser/web_contents/web_contents_view_aura.cc b/content/browser/web_contents/web_contents_view_aura.cc index 824dc46..28ee915 100644 --- a/content/browser/web_contents/web_contents_view_aura.cc +++ b/content/browser/web_contents/web_contents_view_aura.cc @@ -944,9 +944,12 @@ void WebContentsViewAura::SetOverscrollControllerEnabled(bool enabled) { void WebContentsViewAura::ShowContextMenu(RenderFrameHost* render_frame_host, const ContextMenuParams& params) { - ui::TouchSelectionController* selection_controller = GetSelectionController(); - if (selection_controller) - selection_controller->HideAndDisallowShowingAutomatically(); + TouchSelectionControllerClientAura* selection_controller_client = + GetSelectionControllerClient(); + if (selection_controller_client && + selection_controller_client->HandleContextMenu(params)) { + return; + } if (delegate_) { RenderWidgetHostViewAura* view = ToRenderWidgetHostViewAura( web_contents_->GetRenderWidgetHostView()); diff --git a/content/test/data/touch_selection.html b/content/test/data/touch_selection.html index 7bf8644..dc8ab06 100644 --- a/content/test/data/touch_selection.html +++ b/content/test/data/touch_selection.html @@ -4,7 +4,7 @@ diff --git a/ui/touch_selection/touch_selection_controller.h b/ui/touch_selection/touch_selection_controller.h index 797b755..ae0161b 100644 --- a/ui/touch_selection/touch_selection_controller.h +++ b/ui/touch_selection/touch_selection_controller.h @@ -66,6 +66,8 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController // Controls whether an insertion handle is shown on a tap for an empty // editable text. Defauls to false. + // TODO(mohsen): This flag used to be set to |true| on Aura. That's not the + // case anymore and it is always |false|. Consider removing it. bool show_on_tap_for_empty_editable; }; diff --git a/ui/touch_selection/touch_selection_menu_runner.h b/ui/touch_selection/touch_selection_menu_runner.h index 8db3213..f4e05da 100644 --- a/ui/touch_selection/touch_selection_menu_runner.h +++ b/ui/touch_selection/touch_selection_menu_runner.h @@ -42,6 +42,10 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionMenuRunner { static TouchSelectionMenuRunner* GetInstance(); + // Checks whether there is any command available to show in the menu. + virtual bool IsMenuAvailable( + const TouchSelectionMenuClient* client) const = 0; + // Creates and displays the quick menu, if there is any command available. // |anchor_rect| is in screen coordinates. virtual void OpenMenu(TouchSelectionMenuClient* client, diff --git a/ui/views/touchui/touch_selection_menu_runner_views.cc b/ui/views/touchui/touch_selection_menu_runner_views.cc index 55b532c..29fd56e 100644 --- a/ui/views/touchui/touch_selection_menu_runner_views.cc +++ b/ui/views/touchui/touch_selection_menu_runner_views.cc @@ -38,24 +38,19 @@ const int kEllipsesButtonTag = -1; class TouchSelectionMenuRunnerViews::Menu : public BubbleDelegateView, public ButtonListener { public: - // Closes the menu. This will eventually self-destroy the object. - void Close(); - - // Returns a new instance of Menu if there is any command available; - // otherwise, returns |nullptr|. - static Menu* Create(TouchSelectionMenuRunnerViews* owner, - ui::TouchSelectionMenuClient* client, - const gfx::Rect& anchor_rect, - const gfx::Size& handle_image_size, - aura::Window* context); - - private: Menu(TouchSelectionMenuRunnerViews* owner, ui::TouchSelectionMenuClient* client, const gfx::Rect& anchor_rect, const gfx::Size& handle_image_size, aura::Window* context); + // Checks whether there is any command available to show in the menu. + static bool IsMenuAvailable(const ui::TouchSelectionMenuClient* client); + + // Closes the menu. This will eventually self-destroy the object. + void Close(); + + private: ~Menu() override; // Queries the |client_| for what commands to show in the menu and sizes the @@ -78,24 +73,6 @@ class TouchSelectionMenuRunnerViews::Menu : public BubbleDelegateView, DISALLOW_COPY_AND_ASSIGN(Menu); }; -TouchSelectionMenuRunnerViews::Menu* -TouchSelectionMenuRunnerViews::Menu::Create( - TouchSelectionMenuRunnerViews* owner, - ui::TouchSelectionMenuClient* client, - const gfx::Rect& anchor_rect, - const gfx::Size& handle_image_size, - aura::Window* context) { - DCHECK(client); - - for (size_t i = 0; i < arraysize(kMenuCommands); i++) { - if (client->IsCommandIdEnabled(kMenuCommands[i])) - return new Menu(owner, client, anchor_rect, handle_image_size, context); - } - - // No command is available, so return |nullptr|. - return nullptr; -} - TouchSelectionMenuRunnerViews::Menu::Menu(TouchSelectionMenuRunnerViews* owner, ui::TouchSelectionMenuClient* client, const gfx::Rect& anchor_rect, @@ -136,6 +113,17 @@ TouchSelectionMenuRunnerViews::Menu::Menu(TouchSelectionMenuRunnerViews* owner, GetWidget()->Show(); } +bool TouchSelectionMenuRunnerViews::Menu::IsMenuAvailable( + const ui::TouchSelectionMenuClient* client) { + DCHECK(client); + + for (size_t i = 0; i < arraysize(kMenuCommands); i++) { + if (client->IsCommandIdEnabled(kMenuCommands[i])) + return true; + } + return false; +} + TouchSelectionMenuRunnerViews::Menu::~Menu() { } @@ -223,6 +211,11 @@ gfx::Rect TouchSelectionMenuRunnerViews::GetAnchorRectForTest() const { return menu_ ? menu_->GetAnchorRect() : gfx::Rect(); } +bool TouchSelectionMenuRunnerViews::IsMenuAvailable( + const ui::TouchSelectionMenuClient* client) const { + return TouchSelectionMenuRunnerViews::Menu::IsMenuAvailable(client); +} + void TouchSelectionMenuRunnerViews::OpenMenu( ui::TouchSelectionMenuClient* client, const gfx::Rect& anchor_rect, @@ -230,7 +223,8 @@ void TouchSelectionMenuRunnerViews::OpenMenu( aura::Window* context) { CloseMenu(); - menu_ = Menu::Create(this, client, anchor_rect, handle_image_size, context); + if (TouchSelectionMenuRunnerViews::Menu::IsMenuAvailable(client)) + menu_ = new Menu(this, client, anchor_rect, handle_image_size, context); } void TouchSelectionMenuRunnerViews::CloseMenu() { diff --git a/ui/views/touchui/touch_selection_menu_runner_views.h b/ui/views/touchui/touch_selection_menu_runner_views.h index 46032ee..b3ba517 100644 --- a/ui/views/touchui/touch_selection_menu_runner_views.h +++ b/ui/views/touchui/touch_selection_menu_runner_views.h @@ -26,6 +26,8 @@ class VIEWS_EXPORT TouchSelectionMenuRunnerViews gfx::Rect GetAnchorRectForTest() const; // ui::TouchSelectionMenuRunner: + bool IsMenuAvailable( + const ui::TouchSelectionMenuClient* client) const override; void OpenMenu(ui::TouchSelectionMenuClient* client, const gfx::Rect& anchor_rect, const gfx::Size& handle_image_size, -- cgit v1.1