Skip to content

Commit

Permalink
Displays clipboard contents next to the "Paste and Go/Search" option in
Browse files Browse the repository at this point in the history
the Omnibox context menu.

Bug: 720253
Change-Id: I00e39f20a4a198592cb508c3815f6dce7ef17af3
Reviewed-on: https://chromium-review.googlesource.com/c/1242113
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631414}
  • Loading branch information
the-realtom authored and Commit Bot committed Feb 12, 2019
1 parent c9b9d65 commit 7a9e31b
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 17 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,7 @@ Thiago Farina <thiago.farina@gmail.com>
Thiago Marcos P. Santos <thiago.santos@intel.com>
Thomas Butter <tbutter@gmail.com>
Thomas Conti <tomc@amazon.com>
Thomas White <im.toms.inbox@gmail.com>
Tiago Vignatti <tiago.vignatti@intel.com>
Tibor Dusnoki <tibor.dusnoki.91@gmail.com>
Tim Ansell <mithro@mithis.com>
Expand Down
1 change: 1 addition & 0 deletions chrome/app/chrome_command_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@
#define IDC_TOGGLE_JAVASCRIPT_APPLE_EVENTS 40253
#define IDC_INSTALL_PWA 40254
#define IDC_MANAGED_UI_HELP 40255
#define IDC_PASTE_AND_GO 40256

// Spell-check
// Insert any additional suggestions before _LAST; these have to be consecutive.
Expand Down
8 changes: 4 additions & 4 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -4753,18 +4753,18 @@ Keep your key file in a safe place. You will need it to create new versions of y
</message>
<if expr="not use_titlecase">
<message name="IDS_PASTE_AND_GO" desc="The text label of the Paste And Go menu item when the clipboard contains a URL">
Pa&amp;ste and go
Pa&amp;ste and go to <ph name="URL">$1<ex>http://www.google.com/</ex></ph>
</message>
<message name="IDS_PASTE_AND_SEARCH" desc="The text label of the Paste And Go menu item when the clipboard contains a string to search for">
Pa&amp;ste and search
Pa&amp;ste and search for “<ph name="SEARCH_TERMS">$1<ex>flowers</ex></ph>”
</message>
</if>
<if expr="use_titlecase">
<message name="IDS_PASTE_AND_GO" desc="In Title Case: The text label of the Paste And Go menu item when the clipboard contains a URL">
Pa&amp;ste and Go
Pa&amp;ste and Go to <ph name="URL">$1<ex>http://www.google.com/</ex></ph>
</message>
<message name="IDS_PASTE_AND_SEARCH" desc="In Title Case: The text label of the Paste And Go menu item when the clipboard contains a string to search for">
Pa&amp;ste and Search
Pa&amp;ste and Search for “<ph name="SEARCH_TERMS">$1<ex>flowers</ex></ph>”
</message>
</if>
<message name="IDS_SHOW_URL" desc="The text label of the Show URL menu item when the omnibox contains an elided URL or search query terms">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d1e4fa1a62062a7ca03272e277c2dc1a7da2efbc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
64542dd293f80b1cfc358347bf86190f5cfb03f7
41 changes: 32 additions & 9 deletions chrome/browser/ui/views/omnibox/omnibox_view_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "components/omnibox/browser/omnibox_popup_model.h"
#include "components/search_engines/template_url_service.h"
#include "components/strings/grit/components_strings.h"
#include "components/url_formatter/elide_url.h"
#include "content/public/browser/web_contents.h"
#include "net/base/escape.h"
#include "third_party/metrics_proto/omnibox_event.pb.h"
Expand All @@ -60,6 +61,8 @@
#include "ui/gfx/font_list.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/gfx/selection_model.h"
#include "ui/gfx/text_elider.h"
#include "ui/gfx/text_utils.h"
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/border.h"
Expand Down Expand Up @@ -465,7 +468,7 @@ void OmniboxViewViews::ExecuteCommand(int command_id, int event_flags) {
DestroyTouchSelection();
switch (command_id) {
// These commands don't invoke the popup via OnBefore/AfterPossibleChange().
case IDS_PASTE_AND_GO:
case IDC_PASTE_AND_GO:
model()->PasteAndGo(GetClipboardText());
return;
case IDS_SHOW_URL:
Expand Down Expand Up @@ -960,20 +963,40 @@ void OmniboxViewViews::OnMouseExited(const ui::MouseEvent& event) {
}

bool OmniboxViewViews::IsItemForCommandIdDynamic(int command_id) const {
return command_id == IDS_PASTE_AND_GO;
return command_id == IDC_PASTE_AND_GO;
}

base::string16 OmniboxViewViews::GetLabelForCommandId(int command_id) const {
DCHECK_EQ(IDS_PASTE_AND_GO, command_id);
base::string16 clipboard_text = GetClipboardText();
DCHECK_EQ(IDC_PASTE_AND_GO, command_id);

constexpr size_t kMaxSelectionTextLength = 50;
const base::string16 clipboard_text = GetClipboardText();

base::string16 selection_text = gfx::TruncateString(
clipboard_text, kMaxSelectionTextLength, gfx::WORD_BREAK);

// If the clipboard text is too long, this command will be disabled, so
// we skip the potentially expensive classification of the text and default to
// IDS_PASTE_AND_SEARCH.
bool paste_and_search =
clipboard_text.size() > OmniboxEditModel::kMaxPasteAndGoTextLength ||
model()->ClassifiesAsSearch(clipboard_text);
return l10n_util::GetStringUTF16(paste_and_search ? IDS_PASTE_AND_SEARCH
: IDS_PASTE_AND_GO);

if (paste_and_search) {
return l10n_util::GetStringFUTF16(IDS_PASTE_AND_SEARCH, selection_text);
}

// To ensure the search and url strings began to truncate at the exact same
// number of characters, the pixel width at which the url begins to elide is
// derived from the truncated selection text. However, ideally there would be
// a better way to do this.
const float kMaxSelectionPixelWidth = GetStringWidthF(
selection_text, Textfield::GetFontList(), gfx::Typesetter::BROWSER);
base::string16 url = url_formatter::ElideUrl(
GURL(clipboard_text), Textfield::GetFontList(), kMaxSelectionPixelWidth,
gfx::Typesetter::BROWSER);

return l10n_util::GetStringFUTF16(IDS_PASTE_AND_GO, url);
}

const char* OmniboxViewViews::GetClassName() const {
Expand Down Expand Up @@ -1294,7 +1317,7 @@ void OmniboxViewViews::OnBlur() {
bool OmniboxViewViews::IsCommandIdEnabled(int command_id) const {
if (command_id == IDS_APP_PASTE)
return !read_only() && !GetClipboardText().empty();
if (command_id == IDS_PASTE_AND_GO)
if (command_id == IDC_PASTE_AND_GO)
return !read_only() && model()->CanPasteAndGo(GetClipboardText());

// Menu item is only shown when it is valid.
Expand Down Expand Up @@ -1666,8 +1689,8 @@ int OmniboxViewViews::OnDrop(const ui::OSExchangeData& data) {
void OmniboxViewViews::UpdateContextMenu(ui::SimpleMenuModel* menu_contents) {
int paste_position = menu_contents->GetIndexOfCommandId(IDS_APP_PASTE);
DCHECK_GE(paste_position, 0);
menu_contents->InsertItemWithStringIdAt(
paste_position + 1, IDS_PASTE_AND_GO, IDS_PASTE_AND_GO);
menu_contents->InsertItemWithStringIdAt(paste_position + 1, IDC_PASTE_AND_GO,
IDS_PASTE_AND_GO);

// Only add this menu entry if Query in Omnibox feature is enabled.
if (base::FeatureList::IsEnabled(omnibox::kQueryInOmnibox)) {
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/omnibox/omnibox_view_views.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ class OmniboxViewViews : public OmniboxView,
void AddedToWidget() override;
void RemovedFromWidget() override;
bool ShouldDoLearning() override;
base::string16 GetLabelForCommandId(int command_id) const override;
bool IsCommandIdEnabled(int command_id) const override;

// For testing only.
OmniboxPopupContentsView* GetPopupContentsViewForTesting() const {
Expand Down Expand Up @@ -230,7 +232,6 @@ class OmniboxViewViews : public OmniboxView,

// views::Textfield:
bool IsItemForCommandIdDynamic(int command_id) const override;
base::string16 GetLabelForCommandId(int command_id) const override;
const char* GetClassName() const override;
bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnMouseDragged(const ui::MouseEvent& event) override;
Expand All @@ -242,7 +243,6 @@ class OmniboxViewViews : public OmniboxView,
bool HandleAccessibleAction(const ui::AXActionData& action_data) override;
void OnFocus() override;
void OnBlur() override;
bool IsCommandIdEnabled(int command_id) const override;
base::string16 GetSelectionClipboardText() const override;
void DoInsertChar(base::char16 ch) override;
bool IsTextEditCommandEnabled(ui::TextEditCommand command) const override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/command_line.h"
#include "base/macros.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
Expand Down Expand Up @@ -146,7 +147,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, PasteAndGoDoesNotLeavePopupOpen) {
SetClipboardText(ui::CLIPBOARD_TYPE_COPY_PASTE, "http://www.example.com/");

// Paste and go.
omnibox_view_views->ExecuteCommand(IDS_PASTE_AND_GO, ui::EF_NONE);
omnibox_view_views->ExecuteCommand(IDC_PASTE_AND_GO, ui::EF_NONE);

// The popup should not be open.
EXPECT_FALSE(view->model()->popup_model()->IsOpen());
Expand Down Expand Up @@ -349,7 +350,7 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest,
// Execute a command and check if it deactivate touch editing. Paste & Go is
// chosen since it is specific to Omnibox and its execution wouldn't be
// delegated to the base Textfield class.
omnibox_view_views->ExecuteCommand(IDS_PASTE_AND_GO, ui::EF_NONE);
omnibox_view_views->ExecuteCommand(IDC_PASTE_AND_GO, ui::EF_NONE);
EXPECT_FALSE(textfield_test_api.touch_selection_controller());
}

Expand Down
44 changes: 44 additions & 0 deletions chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_tick_clock.h"
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h"
#include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
#include "chrome/browser/command_updater.h"
Expand All @@ -30,6 +31,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/metrics_proto/omnibox_event.pb.h"
#include "ui/base/clipboard/clipboard.h"
#include "ui/base/clipboard/scoped_clipboard_writer.h"
#include "ui/base/ime/input_method.h"
#include "ui/base/ime/text_edit_commands.h"
#include "ui/base/ui_base_features.h"
Expand Down Expand Up @@ -217,6 +219,7 @@ class OmniboxViewViewsTest : public OmniboxViewViewsTestBase {
OmniboxViewViewsTest() : OmniboxViewViewsTest(std::vector<base::Feature>()) {}

TestLocationBarModel* location_bar_model() { return &location_bar_model_; }
CommandUpdaterImpl* command_updater() { return &command_updater_; }
TestingOmniboxView* omnibox_view() const { return omnibox_view_; }

// TODO(tommycli): These base class accessors exist because Textfield and
Expand Down Expand Up @@ -569,6 +572,47 @@ TEST_F(OmniboxViewViewsTest, BackspaceExitsKeywordMode) {
EXPECT_TRUE(omnibox_view()->model()->keyword().empty());
}

TEST_F(OmniboxViewViewsTest, PasteAndGoToUrlOrSearchCommand) {
ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread();
ui::ClipboardType clipboard_type = ui::CLIPBOARD_TYPE_COPY_PASTE;
command_updater()->UpdateCommandEnabled(IDC_OPEN_CURRENT_URL, true);

// Test command is disabled for an empty clipboard.
clipboard->Clear(clipboard_type);
EXPECT_FALSE(omnibox_view()->IsCommandIdEnabled(IDC_PASTE_AND_GO));

// Test command is enabled and the correct label text is returned with a URL
// on the clipboard.
base::string16 expected_text =
#if defined(OS_MACOSX)
base::ASCIIToUTF16("Pa&ste and Go to https://test.com");
#else
base::ASCIIToUTF16("Pa&ste and go to https://test.com");
#endif
ui::ScopedClipboardWriter(clipboard_type)
.WriteText(base::ASCIIToUTF16("https://test.com/"));
base::string16 returned_text =
omnibox_view()->GetLabelForCommandId(IDC_PASTE_AND_GO);
EXPECT_TRUE(omnibox_view()->IsCommandIdEnabled(IDC_PASTE_AND_GO));
EXPECT_EQ(expected_text, returned_text);

// Test command is enabled and the correct label text is returned with a
// search on the clipboard.
expected_text =
#if defined(OS_MACOSX)
base::WideToUTF16(
L"Pa&ste and Search for \x201Cthis is a test sentence\x201D");
#else
base::WideToUTF16(
L"Pa&ste and search for \x201Cthis is a test sentence\x201D");
#endif
ui::ScopedClipboardWriter(clipboard_type)
.WriteText(base::ASCIIToUTF16("this is a test sentence"));
returned_text = omnibox_view()->GetLabelForCommandId(IDC_PASTE_AND_GO);
EXPECT_TRUE(omnibox_view()->IsCommandIdEnabled(IDC_PASTE_AND_GO));
EXPECT_EQ(expected_text, returned_text);
}

class OmniboxViewViewsClipboardTest
: public OmniboxViewViewsTest,
public ::testing::WithParamInterface<ui::TextEditCommand> {
Expand Down

0 comments on commit 7a9e31b

Please sign in to comment.