Commit 80dfe45f authored by Andrew Xu's avatar Andrew Xu Committed by Commit Bot

[Multipaste] Anchor menu at the cursor's location in some cases

Usually the clipboard history menu is anchored at the caret's location.
This CL anchors the history menu at the cursor's location when there is
no textfield. In addition, some apps may provide unreliable caret's
bounds, such as Google Doc. The menu is also anchored at the cursor's
location in that case. Subsequent CLs will solve the unreliable caret
bounds thoroughly.

A browser test is created to protect this feature.

Bug: 1110027
Change-Id: I30f31b0ab89542b5c7f63ebd25257dc765f57929
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328191
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: default avatarAlex Newcomer <newcomer@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#794244}
parent 0f741288
...@@ -140,15 +140,6 @@ bool ClipboardHistoryController::CanShowMenu() const { ...@@ -140,15 +140,6 @@ bool ClipboardHistoryController::CanShowMenu() const {
} }
void ClipboardHistoryController::ShowMenu() { void ClipboardHistoryController::ShowMenu() {
auto* host = ash::GetWindowTreeHostForDisplay(
display::Screen::GetScreen()->GetPrimaryDisplay().id());
// Some web apps render the caret in an IFrame, and we will not get the
// bounds in that case.
// TODO(https://crbug.com/1099930): Show the menu in the middle of the
// webview if the bounds are empty.
const gfx::Rect textfield_bounds =
host->GetInputMethod()->GetTextInputClient()->GetCaretBounds();
if (!CanShowMenu()) if (!CanShowMenu())
return; return;
...@@ -175,7 +166,7 @@ void ClipboardHistoryController::ShowMenu() { ...@@ -175,7 +166,7 @@ void ClipboardHistoryController::ShowMenu() {
context_menu_ = context_menu_ =
std::make_unique<ClipboardHistoryMenuModelAdapter>(std::move(menu_model)); std::make_unique<ClipboardHistoryMenuModelAdapter>(std::move(menu_model));
context_menu_->Run(textfield_bounds); context_menu_->Run(CalculateAnchorRect());
} }
void ClipboardHistoryController::MenuOptionSelected(int index) { void ClipboardHistoryController::MenuOptionSelected(int index) {
...@@ -219,4 +210,46 @@ void ClipboardHistoryController::MenuOptionSelected(int index) { ...@@ -219,4 +210,46 @@ void ClipboardHistoryController::MenuOptionSelected(int index) {
base::TimeDelta::FromMilliseconds(100)); base::TimeDelta::FromMilliseconds(100));
} }
bool ClipboardHistoryController::IsMenuShowing() const {
return context_menu_ && context_menu_->IsRunning();
}
gfx::Rect ClipboardHistoryController::GetClipboardHistoryMenuBoundsForTest()
const {
return context_menu_->GetClipboardHistoryMenuBoundsForTest();
}
gfx::Rect ClipboardHistoryController::CalculateAnchorRect() const {
display::Display display = display::Screen::GetScreen()->GetPrimaryDisplay();
auto* host = ash::GetWindowTreeHostForDisplay(display.id());
// Some web apps render the caret in an IFrame, and we will not get the
// bounds in that case.
// TODO(https://crbug.com/1099930): Show the menu in the middle of the
// webview if the bounds are empty.
ui::TextInputClient* text_input_client =
host->GetInputMethod()->GetTextInputClient();
// |text_input_client| may be null. For example, in clamshell mode and without
// any window open.
const gfx::Rect textfield_bounds =
text_input_client ? text_input_client->GetCaretBounds() : gfx::Rect();
// Note that the width of caret's bounds may be zero in some views (such as
// the search bar of Google search web page). So we cannot use
// gfx::Size::IsEmpty() here. In addition, the applications using IFrame may
// provide unreliable |textfield_bounds| which are not fully contained by the
// display bounds.
// TODO(https://crbug.com/1110027).
const bool textfield_bounds_are_valid =
textfield_bounds.size() != gfx::Size() &&
display.bounds().Contains(textfield_bounds);
if (textfield_bounds_are_valid)
return textfield_bounds;
return gfx::Rect(display::Screen::GetScreen()->GetCursorScreenPoint(),
gfx::Size());
}
} // namespace ash } // namespace ash
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
#include "ash/ash_export.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "ui/base/models/simple_menu_model.h" #include "ui/base/models/simple_menu_model.h"
...@@ -22,7 +23,7 @@ class ClipboardHistoryMenuModelAdapter; ...@@ -22,7 +23,7 @@ class ClipboardHistoryMenuModelAdapter;
// Shows a menu with the last few things saved in the clipboard when the // Shows a menu with the last few things saved in the clipboard when the
// keyboard shortcut is pressed. // keyboard shortcut is pressed.
class ClipboardHistoryController { class ASH_EXPORT ClipboardHistoryController {
public: public:
ClipboardHistoryController(); ClipboardHistoryController();
ClipboardHistoryController(const ClipboardHistoryController&) = delete; ClipboardHistoryController(const ClipboardHistoryController&) = delete;
...@@ -42,9 +43,15 @@ class ClipboardHistoryController { ...@@ -42,9 +43,15 @@ class ClipboardHistoryController {
// Called when a menu option is selected. // Called when a menu option is selected.
void MenuOptionSelected(int index); void MenuOptionSelected(int index);
bool IsMenuShowing() const;
gfx::Rect GetClipboardHistoryMenuBoundsForTest() const;
ClipboardHistory* clipboard_history() { return clipboard_history_.get(); } ClipboardHistory* clipboard_history() { return clipboard_history_.get(); }
private: private:
gfx::Rect CalculateAnchorRect() const;
// The menu being shown. // The menu being shown.
std::unique_ptr<ClipboardHistoryMenuModelAdapter> context_menu_; std::unique_ptr<ClipboardHistoryMenuModelAdapter> context_menu_;
// Used to keep track of what is being copied to the clipboard. // Used to keep track of what is being copied to the clipboard.
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "ui/views/controls/menu/menu_item_view.h" #include "ui/views/controls/menu/menu_item_view.h"
#include "ui/views/controls/menu/menu_runner.h" #include "ui/views/controls/menu/menu_runner.h"
#include "ui/views/controls/menu/menu_types.h" #include "ui/views/controls/menu/menu_types.h"
#include "ui/views/controls/menu/submenu_view.h"
namespace ash { namespace ash {
...@@ -33,4 +34,13 @@ void ClipboardHistoryMenuModelAdapter::Run(const gfx::Rect& anchor_rect) { ...@@ -33,4 +34,13 @@ void ClipboardHistoryMenuModelAdapter::Run(const gfx::Rect& anchor_rect) {
views::MenuAnchorPosition::kBubbleRight, ui::MENU_SOURCE_KEYBOARD); views::MenuAnchorPosition::kBubbleRight, ui::MENU_SOURCE_KEYBOARD);
} }
} // namespace ash bool ClipboardHistoryMenuModelAdapter::IsRunning() const {
\ No newline at end of file return menu_runner_ && menu_runner_->IsRunning();
}
gfx::Rect
ClipboardHistoryMenuModelAdapter::GetClipboardHistoryMenuBoundsForTest() const {
return root_view_->GetSubmenu()->GetBoundsInScreen();
}
} // namespace ash
...@@ -39,6 +39,10 @@ class ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter { ...@@ -39,6 +39,10 @@ class ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
// Shows the menu, anchored below |anchor_rect|. // Shows the menu, anchored below |anchor_rect|.
void Run(const gfx::Rect& anchor_rect); void Run(const gfx::Rect& anchor_rect);
bool IsRunning() const;
gfx::Rect GetClipboardHistoryMenuBoundsForTest() const;
private: private:
// The model which holds the contents of the menu. // The model which holds the contents of the menu.
std::unique_ptr<ui::SimpleMenuModel> const model_; std::unique_ptr<ui::SimpleMenuModel> const model_;
...@@ -51,4 +55,4 @@ class ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter { ...@@ -51,4 +55,4 @@ class ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
} // namespace ash } // namespace ash
#endif // ASH_CLIPBOARD_CLIPBOARD_HISTORY_MENU_MODEL_ADAPTER_H_ #endif // ASH_CLIPBOARD_CLIPBOARD_HISTORY_MENU_MODEL_ADAPTER_H_
\ No newline at end of file
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
#include "components/user_manager/user_manager.h" #include "components/user_manager/user_manager.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/clipboard/scoped_clipboard_writer.h"
#include "ui/events/test/event_generator.h"
#include "ui/views/controls/menu/menu_config.h"
namespace { namespace {
...@@ -28,13 +30,42 @@ void SetClipboardText(const std::string& text) { ...@@ -28,13 +30,42 @@ void SetClipboardText(const std::string& text) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
ash::ClipboardHistoryController* GetClipboardHistoryController() {
return ash::Shell::Get()->clipboard_history_controller();
}
const std::list<ui::ClipboardData>& GetClipboardData() { const std::list<ui::ClipboardData>& GetClipboardData() {
return ash::Shell::Get() return GetClipboardHistoryController()->clipboard_history()->GetItems();
->clipboard_history_controller() }
->clipboard_history()
->GetItems(); gfx::Rect GetClipboardHistoryMenuScreenBounds() {
return GetClipboardHistoryController()
->GetClipboardHistoryMenuBoundsForTest();
} }
class ClipboardTestHelper {
public:
ClipboardTestHelper() = default;
~ClipboardTestHelper() = default;
void Init() {
event_generator_ = std::make_unique<ui::test::EventGenerator>(
ash::Shell::GetPrimaryRootWindow());
}
ui::test::EventGenerator* event_generator() { return event_generator_.get(); }
void ShowContextMenuViaAccelerator() {
event_generator_->PressKey(ui::KeyboardCode::VKEY_COMMAND, ui::EF_NONE);
event_generator_->PressKey(ui::KeyboardCode::VKEY_V, ui::EF_COMMAND_DOWN);
event_generator_->ReleaseKey(ui::KeyboardCode::VKEY_V, ui::EF_COMMAND_DOWN);
event_generator_->ReleaseKey(ui::KeyboardCode::VKEY_COMMAND, ui::EF_NONE);
}
private:
std::unique_ptr<ui::test::EventGenerator> event_generator_;
};
} // namespace } // namespace
// Verify clipboard history's features in the multiprofile environment. // Verify clipboard history's features in the multiprofile environment.
...@@ -51,11 +82,28 @@ class ClipboardHistoryWithMultiProfileBrowserTest ...@@ -51,11 +82,28 @@ class ClipboardHistoryWithMultiProfileBrowserTest
~ClipboardHistoryWithMultiProfileBrowserTest() override = default; ~ClipboardHistoryWithMultiProfileBrowserTest() override = default;
ui::test::EventGenerator* GetEventGenerator() {
return test_helper_->event_generator();
}
protected: protected:
void ShowContextMenuViaAccelerator() {
test_helper_->ShowContextMenuViaAccelerator();
}
void SetUpOnMainThread() override {
chromeos::LoginManagerTest::SetUpOnMainThread();
test_helper_ = std::make_unique<ClipboardTestHelper>();
test_helper_->Init();
}
AccountId account_id1_; AccountId account_id1_;
AccountId account_id2_; AccountId account_id2_;
chromeos::LoginManagerMixin login_mixin_{&mixin_host_}; chromeos::LoginManagerMixin login_mixin_{&mixin_host_};
std::unique_ptr<ClipboardTestHelper> test_helper_;
base::test::ScopedFeatureList feature_list_; base::test::ScopedFeatureList feature_list_;
}; };
...@@ -111,3 +159,33 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest, ...@@ -111,3 +159,33 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
EXPECT_EQ(copypaste_data1, it->text()); EXPECT_EQ(copypaste_data1, it->text());
} }
} }
// Verifies that the history menu is anchored at the cursor's location when
// not having any textfield.
IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
ShowHistoryMenuWhenNoTextfieldExists) {
LoginUser(account_id1_);
// Close the browser window to ensure that textfield does not exist.
CloseAllBrowsers();
// No clipboard data. So the clipboard history menu should not show.
ASSERT_TRUE(GetClipboardData().empty());
ShowContextMenuViaAccelerator();
EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());
SetClipboardText("test");
const gfx::Point mouse_location =
ash::Shell::Get()->GetPrimaryRootWindow()->bounds().CenterPoint();
GetEventGenerator()->MoveMouseTo(mouse_location);
ShowContextMenuViaAccelerator();
// Verifies that the menu is anchored at the cursor's location.
ASSERT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
const gfx::Point menu_origin = GetClipboardHistoryMenuScreenBounds().origin();
EXPECT_EQ(mouse_location.x() +
views::MenuConfig::instance().touchable_anchor_offset,
menu_origin.x());
EXPECT_EQ(mouse_location.y(), menu_origin.y());
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment