Paste as plain text from ClipboardHistory when pressing shift.

Previously, it was not possible to paste as plain text when pasting via
the clipboard history feature. After this change, it is possible to
paste as plain text by holding down the shift key when selecting a
history item to paste.

Bug: 1114774
Change-Id: I6f74cdb3f3915074be8be5562b36850e44477f67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348294
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Andrew Xu <andrewxu@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797926}

ash/clipboard/clipboard_history_controller.cc

@@ -63,11 +63,10 @@ ui::ImageModel GetImageModelForClipboardData(const ui::ClipboardData& item) {
   return ui::ImageModel();
 }
 
-void WriteClipboardDataToClipboard(const ui::ClipboardData& data) {
+ui::ClipboardNonBacked* GetClipboard() {
   auto* clipboard = ui::ClipboardNonBacked::GetForCurrentThread();
-  CHECK(clipboard);
-
-  clipboard->WriteClipboardData(std::make_unique<ui::ClipboardData>(data));
+  DCHECK(clipboard);
+  return clipboard;
 }
 
 }  // namespace
@@ -96,7 +95,7 @@ class ClipboardHistoryController::AcceleratorTarget
   // ui::AcceleratorTarget:
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override {
     if (controller_->IsMenuShowing())
-      controller_->ExecuteSelectedMenuItem();
+      controller_->ExecuteSelectedMenuItem(accelerator.modifiers());
     else
       controller_->ShowMenu();
     return true;
@@ -122,7 +121,7 @@ class ClipboardHistoryController::MenuDelegate
 
   // ui::SimpleMenuModel::Delegate:
   void ExecuteCommand(int command_id, int event_flags) override {
-    controller_->MenuOptionSelected(/*index=*/command_id);
+    controller_->MenuOptionSelected(/*index=*/command_id, event_flags);
   }
 
  private:
@@ -155,19 +154,16 @@ bool ClipboardHistoryController::CanShowMenu() const {
   return !clipboard_history_->IsEmpty();
 }
 
-void ClipboardHistoryController::ExecuteSelectedMenuItem() {
+void ClipboardHistoryController::ExecuteSelectedMenuItem(int event_flags) {
   DCHECK(IsMenuShowing());
   auto command = context_menu_->GetSelectedMenuItemCommand();
 
-  // TODO(crbug.com/1106849): Update once sequential paste is supported.
   // Force close the context menu. Failure to do so before dispatching our
   // synthetic key event will result in the context menu consuming the event.
-  // Currently we don't support sequential copy-paste. Once we do, we'll have to
-  // update this logic.
   context_menu_->Cancel();
 
   // If no menu item is currently selected, we'll fallback to the first item.
-  menu_delegate_->ExecuteCommand(command.value_or(0), ui::EF_NONE);
+  menu_delegate_->ExecuteCommand(command.value_or(0), event_flags);
 }
 
 void ClipboardHistoryController::ShowMenu() {
@@ -200,7 +196,8 @@ void ClipboardHistoryController::ShowMenu() {
   context_menu_->Run(CalculateAnchorRect());
 }
 
-void ClipboardHistoryController::MenuOptionSelected(int index) {
+void ClipboardHistoryController::MenuOptionSelected(int index,
+                                                    int event_flags) {
   auto it = clipboard_items_.begin();
   std::advance(it, index);
 
@@ -210,14 +207,25 @@ void ClipboardHistoryController::MenuOptionSelected(int index) {
     return;
   }
 
-  // Pause clipboard history when manipulating the clipboard for the purpose of
-  // a paste.
-  ClipboardHistory::ScopedPause scoped_pause(clipboard_history_.get());
-
-  // Place the selected item on top of the clipboard.
-  const bool selected_item_not_on_top = it != clipboard_items_.begin();
-  if (selected_item_not_on_top)
-    WriteClipboardDataToClipboard(*it);
+  auto* clipboard = GetClipboard();
+  std::unique_ptr<ui::ClipboardData> original_data;
+
+  // If necessary, replace the clipboard's |original_data| temporarily so that
+  // we can paste the selected history item.
+  const bool shift_key_pressed = event_flags & ui::EF_SHIFT_DOWN;
+  if (shift_key_pressed || *it != *clipboard->GetClipboardData()) {
+    std::unique_ptr<ui::ClipboardData> temp_data;
+    if (shift_key_pressed) {
+      // When the shift key is pressed, we only paste plain text.
+      temp_data = std::make_unique<ui::ClipboardData>();
+      temp_data->set_text(it->text());
+    } else {
+      temp_data = std::make_unique<ui::ClipboardData>(*it);
+    }
+    // Pause clipboard history when manipulating the clipboard for a paste.
+    ClipboardHistory::ScopedPause scoped_pause(clipboard_history_.get());
+    original_data = clipboard->WriteClipboardData(std::move(temp_data));
+  }
 
   ui::KeyEvent synthetic_key_event(ui::ET_KEY_PRESSED, ui::VKEY_V,
                                    static_cast<ui::DomCode>(0),
@@ -227,7 +235,7 @@ void ClipboardHistoryController::MenuOptionSelected(int index) {
   DCHECK(host);
   host->DeliverEventToSink(&synthetic_key_event);
 
-  if (!selected_item_not_on_top)
+  if (!original_data)
     return;
 
   // Replace the original item back on top of the clipboard. Some apps take a
@@ -238,7 +246,7 @@ void ClipboardHistoryController::MenuOptionSelected(int index) {
       FROM_HERE,
       base::BindOnce(
           [](const base::WeakPtr<ClipboardHistoryController>& weak_ptr,
-             ui::ClipboardData clipboard_data) {
+             std::unique_ptr<ui::ClipboardData> original_data) {
             // When restoring the original item back on top of the clipboard we
             // need to pause clipboard history. Failure to do so will result in
             // the original item being re-recorded when this restoration step
@@ -248,9 +256,9 @@ void ClipboardHistoryController::MenuOptionSelected(int index) {
               scoped_pause = std::make_unique<ClipboardHistory::ScopedPause>(
                   weak_ptr->clipboard_history_.get());
             }
-            WriteClipboardDataToClipboard(clipboard_data);
+            GetClipboard()->WriteClipboardData(std::move(original_data));
           },
-          weak_ptr_factory_.GetWeakPtr(), *clipboard_items_.begin()),
+          weak_ptr_factory_.GetWeakPtr(), std::move(original_data)),
       base::TimeDelta::FromMilliseconds(100));
 }
 

ash/clipboard/clipboard_history_controller.h

@@ -51,8 +51,8 @@ class ASH_EXPORT ClipboardHistoryController {
 
   bool CanShowMenu() const;
   void ShowMenu();
-  void ExecuteSelectedMenuItem();
-  void MenuOptionSelected(int index);
+  void ExecuteSelectedMenuItem(int event_flags);
+  void MenuOptionSelected(int index, int event_flags);
 
   gfx::Rect CalculateAnchorRect() const;
 

chrome/browser/ui/ash/clipboard_history_browsertest.cc

@@ -8,13 +8,20 @@
 #include "ash/clipboard/clipboard_history.h"
 #include "ash/clipboard/clipboard_history_controller.h"
 #include "ash/shell.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/chromeos/login/login_manager_test.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/ui/user_adding_screen.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/user_manager/user_manager.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/views/controls/menu/menu_config.h"
@@ -34,14 +41,34 @@ std::unique_ptr<views::Widget> CreateTestWidget() {
   return widget;
 }
 
+void FlushMessageLoop() {
+  base::RunLoop run_loop;
+  base::SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                   run_loop.QuitClosure());
+  run_loop.Run();
+}
+
 void SetClipboardText(const std::string& text) {
   ui::ScopedClipboardWriter(ui::ClipboardBuffer::kCopyPaste)
-      .WriteText(base::ASCIIToUTF16(text));
+      .WriteText(base::UTF8ToUTF16(text));
+
+  // ClipboardHistory will post a task to process clipboard data in order to
+  // debounce multiple clipboard writes occurring in sequence. Here we give
+  // ClipboardHistory the chance to run its posted tasks before proceeding.
+  FlushMessageLoop();
+}
+
+void SetClipboardTextAndHtml(const std::string& text, const std::string& html) {
+  {
+    ui::ScopedClipboardWriter scw(ui::ClipboardBuffer::kCopyPaste);
+    scw.WriteText(base::UTF8ToUTF16(text));
+    scw.WriteHTML(base::UTF8ToUTF16(html), /*source_url=*/"");
+  }
 
   // ClipboardHistory will post a task to process clipboard data in order to
   // debounce multiple clipboard writes occurring in sequence. Here we give
   // ClipboardHistory the chance to run its posted tasks before proceeding.
-  base::RunLoop().RunUntilIdle();
+  FlushMessageLoop();
 }
 
 ash::ClipboardHistoryController* GetClipboardHistoryController() {
@@ -324,3 +351,107 @@ IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
   EXPECT_EQ("A", base::UTF16ToUTF8(textfield->GetText()));
   Release(ui::KeyboardCode::VKEY_COMMAND);
 }
+
+IN_PROC_BROWSER_TEST_F(ClipboardHistoryWithMultiProfileBrowserTest,
+                       ShouldPasteHistoryAsPlainText) {
+  LoginUser(account_id1_);
+
+  // Create a browser and cache its active web contents.
+  auto* browser = CreateBrowser(
+      chromeos::ProfileHelper::Get()->GetProfileByAccountId(account_id1_));
+  auto* web_contents = browser->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents);
+
+  // Load the web contents synchronously.
+  // The contained script:
+  //  - Listens for paste events and caches the last pasted data.
+  //  - Notifies observers of paste events by changing document title.
+  //  - Provides an API to expose the last pasted data.
+  ASSERT_TRUE(content::NavigateToURL(web_contents, GURL(R"(data:text/html,
+    <!DOCTYPE html>
+    <html>
+      <body>
+        <script>
+
+          let lastPaste = undefined;
+          let lastPasteId = 1;
+
+          window.addEventListener('paste', e => {
+            e.stopPropagation();
+            e.preventDefault();
+
+            const clipboardData = e.clipboardData || window.clipboardData;
+            lastPaste = clipboardData.types.reduce((data, type) => {
+              data.push(`${type}: ${clipboardData.getData(type)}`);
+              return data;
+            }, []);
+
+            document.title = `Paste ${lastPasteId++}`;
+          });
+
+          window.getLastPaste = () => {
+            return lastPaste || [];
+          };
+
+        </script>
+      </body>
+    </html>
+  )")));
+
+  // Cache a function to return the last paste.
+  auto GetLastPaste = [&]() {
+    auto result = content::EvalJs(
+        web_contents, "(function() { return window.getLastPaste(); })();");
+    EXPECT_EQ(result.error, "");
+    return result.ExtractList();
+  };
+
+  // Confirm initial state.
+  ASSERT_TRUE(GetLastPaste().GetList().empty());
+
+  // Write some things to the clipboard.
+  SetClipboardTextAndHtml("A", "<span>A</span>");
+  SetClipboardTextAndHtml("B", "<span>B</span>");
+  SetClipboardTextAndHtml("C", "<span>C</span>");
+
+  // Open clipboard history and paste the last history item.
+  PressAndRelease(ui::KeyboardCode::VKEY_V, ui::EF_COMMAND_DOWN);
+  EXPECT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
+  PressAndRelease(ui::KeyboardCode::VKEY_RETURN);
+  EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());
+
+  // Wait for the paste event to propagate to the web contents.
+  // The web contents will notify us a paste occurred by updating page title.
+  ignore_result(
+      content::TitleWatcher(web_contents, base ::UTF8ToUTF16("Paste 1"))
+          .WaitAndGetTitle());
+
+  // Confirm the expected paste data.
+  base::ListValue last_paste = GetLastPaste();
+  ASSERT_EQ(last_paste.GetList().size(), 2u);
+  EXPECT_EQ(last_paste.GetList()[0].GetString(), "text/plain: A");
+  EXPECT_EQ(last_paste.GetList()[1].GetString(), "text/html: <span>A</span>");
+
+  // Open clipboard history and paste the middle history item as plain text.
+  PressAndRelease(ui::KeyboardCode::VKEY_V, ui::EF_COMMAND_DOWN);
+  EXPECT_TRUE(GetClipboardHistoryController()->IsMenuShowing());
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
+  PressAndRelease(ui::KeyboardCode::VKEY_DOWN);
+  PressAndRelease(ui::KeyboardCode::VKEY_RETURN, ui::EF_SHIFT_DOWN);
+  EXPECT_FALSE(GetClipboardHistoryController()->IsMenuShowing());
+
+  // Wait for the paste event to propagate to the web contents.
+  // The web contents will notify us a paste occurred by updating page title.
+  ignore_result(
+      content::TitleWatcher(web_contents, base ::UTF8ToUTF16("Paste 2"))
+          .WaitAndGetTitle());
+
+  // Confirm the expected paste data.
+  last_paste = GetLastPaste();
+  ASSERT_EQ(last_paste.GetList().size(), 1u);
+  EXPECT_EQ(last_paste.GetList()[0].GetString(), "text/plain: A");
+}

ui/base/clipboard/clipboard_non_backed.cc

@@ -217,12 +217,15 @@ class ClipboardInternal {
     *result = data->custom_data_data();
   }
 
-  // Writes |data| to the ClipboardData.
-  void WriteData(std::unique_ptr<ClipboardData> data) {
+  // Writes |data| to the ClipboardData and returns the previous data.
+  std::unique_ptr<ClipboardData> WriteData(
+      std::unique_ptr<ClipboardData> data) {
     DCHECK(data);
-    ++sequence_number_;
+    std::unique_ptr<ClipboardData> previous_data = std::move(data_);
     data_ = std::move(data);
+    ++sequence_number_;
     ClipboardMonitor::GetInstance()->NotifyClipboardDataChanged();
+    return previous_data;
   }
 
   void SetDlpController(
@@ -365,10 +368,10 @@ const ClipboardData* ClipboardNonBacked::GetClipboardData() const {
   return clipboard_internal_->GetData();
 }
 
-void ClipboardNonBacked::WriteClipboardData(
+std::unique_ptr<ClipboardData> ClipboardNonBacked::WriteClipboardData(
     std::unique_ptr<ClipboardData> data) {
   DCHECK(CalledOnValidThread());
-  clipboard_internal_->WriteData(std::move(data));
+  return clipboard_internal_->WriteData(std::move(data));
 }
 
 void ClipboardNonBacked::OnPreShutdown() {}

ui/base/clipboard/clipboard_non_backed.h

@@ -34,8 +34,9 @@ class COMPONENT_EXPORT(UI_BASE_CLIPBOARD) ClipboardNonBacked
   // Returns the current ClipboardData.
   const ClipboardData* GetClipboardData() const;
 
-  // Writes the current ClipboardData.
-  void WriteClipboardData(std::unique_ptr<ClipboardData> data);
+  // Writes the current ClipboardData and returns the previous data.
+  std::unique_ptr<ClipboardData> WriteClipboardData(
+      std::unique_ptr<ClipboardData> data);
 
  private:
   friend class Clipboard;

ui/base/clipboard/clipboard_non_backed_unittest.cc

@@ -36,4 +36,22 @@ TEST_F(ClipboardNonBackedTest, WriteAndGetClipboardData) {
   EXPECT_EQ(expected_clipboard_data_ptr, actual_clipboard_data_ptr);
 }
 
+// Verifies that WriteClipboardData() writes a ClipboardData instance to the
+// clipboard and returns the previous instance.
+TEST_F(ClipboardNonBackedTest, WriteClipboardData) {
+  auto first_data = std::make_unique<ClipboardData>();
+  auto second_data = std::make_unique<ClipboardData>();
+
+  auto* first_data_ptr = first_data.get();
+  auto* second_data_ptr = second_data.get();
+
+  auto previous_data = clipboard()->WriteClipboardData(std::move(first_data));
+  EXPECT_EQ(previous_data.get(), nullptr);
+
+  previous_data = clipboard()->WriteClipboardData(std::move(second_data));
+
+  EXPECT_EQ(first_data_ptr, previous_data.get());
+  EXPECT_EQ(second_data_ptr, clipboard()->GetClipboardData());
+}
+
 }  // namespace ui

ui/views/controls/menu/menu_controller.cc

@@ -1187,18 +1187,18 @@ ui::PostDispatchAction MenuController::OnWillDispatchKeyEvent(
     // Special handling for Option-Up and Option-Down, which should behave like
     // Home and End respectively in menus.
     if ((event->flags() & ui::EF_ALT_DOWN)) {
+      ui::KeyEvent rewritten_event(*event);
       if (event->key_code() == ui::VKEY_UP) {
-        key_handled = OnKeyPressed(ui::VKEY_HOME);
+        rewritten_event.set_key_code(ui::VKEY_HOME);
       } else if (event->key_code() == ui::VKEY_DOWN) {
-        key_handled = OnKeyPressed(ui::VKEY_END);
-      } else {
-        key_handled = OnKeyPressed(event->key_code());
+        rewritten_event.set_key_code(ui::VKEY_END);
       }
+      key_handled = OnKeyPressed(rewritten_event);
     } else {
-      key_handled = OnKeyPressed(event->key_code());
+      key_handled = OnKeyPressed(*event);
     }
 #else
-    key_handled = OnKeyPressed(event->key_code());
+    key_handled = OnKeyPressed(*event);
 #endif
 
     if (key_handled)
@@ -1486,13 +1486,16 @@ void MenuController::StartDrag(SubmenuView* source,
     did_initiate_drag_ = false;
 }
 
-bool MenuController::OnKeyPressed(ui::KeyboardCode key_code) {
-  // Do not process while performing drag-and-drop
+bool MenuController::OnKeyPressed(const ui::KeyEvent& event) {
+  DCHECK_EQ(event.type(), ui::ET_KEY_PRESSED);
+
+  // Do not process while performing drag-and-drop.
   if (for_drop_)
     return false;
 
   bool handled_key_code = false;
 
+  const ui::KeyboardCode key_code = event.key_code();
   switch (key_code) {
     case ui::VKEY_HOME:
       if (IsEditableCombobox())
@@ -1573,7 +1576,7 @@ bool MenuController::OnKeyPressed(ui::KeyboardCode key_code) {
           handled_key_code = true;
           if (!SendAcceleratorToHotTrackedView() &&
               pending_state_.item->GetEnabled()) {
-            Accept(pending_state_.item, 0);
+            Accept(pending_state_.item, event.flags());
           }
         }
       }

ui/views/controls/menu/menu_controller.h

@@ -353,9 +353,8 @@ class VIEWS_EXPORT MenuController
                                  const ui::LocatedEvent* event);
   void StartDrag(SubmenuView* source, const gfx::Point& location);
 
-  // Handles |key_code| as a keypress. Returns true if OnKeyPressed handled the
-  // key code.
-  bool OnKeyPressed(ui::KeyboardCode key_code);
+  // Returns true if OnKeyPressed handled the key |event|.
+  bool OnKeyPressed(const ui::KeyEvent& event);
 
   // Creates a MenuController. See |for_drop_| member for details on |for_drop|.
   MenuController(bool for_drop, internal::MenuControllerDelegate* delegate);