Commit f4df4189 authored by Tommy Li's avatar Tommy Li Committed by Commit Bot

[omnibox] Stop copying hyperlinks from the omnibox to the clipboard

We had this feature to better support Query in Omnibox.

Now that Query in Omnibox has been deleted this is no longer needed,
since plaintext URLs are generally handled somewhat better than links.

This CL also removes some stale references to Query in Omnibox in
some comments.

Bug: 874592
Change-Id: Ia1cf70d0a0598897202e3241a90e459d470d3620
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250831Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780005}
parent 967de6fb
...@@ -103,11 +103,6 @@ using metrics::OmniboxEventProto; ...@@ -103,11 +103,6 @@ using metrics::OmniboxEventProto;
namespace { namespace {
// TODO(tommycli): Remove this killswitch once we are confident that the new
// behavior doesn't cause big user breakage.
constexpr base::Feature kOmniboxCanCopyHyperlinksToClipboard{
"OmniboxCanCopyHyperlinksToClipboard", base::FEATURE_ENABLED_BY_DEFAULT};
// When certain field trials are enabled, the path is hidden this long after // When certain field trials are enabled, the path is hidden this long after
// page load. // page load.
const uint32_t kPathFadeOutDelayMs = 4000; const uint32_t kPathFadeOutDelayMs = 4000;
...@@ -1964,10 +1959,8 @@ void OmniboxViewViews::OnAfterCutOrCopy(ui::ClipboardBuffer clipboard_buffer) { ...@@ -1964,10 +1959,8 @@ void OmniboxViewViews::OnAfterCutOrCopy(ui::ClipboardBuffer clipboard_buffer) {
ui::ScopedClipboardWriter scoped_clipboard_writer(clipboard_buffer); ui::ScopedClipboardWriter scoped_clipboard_writer(clipboard_buffer);
scoped_clipboard_writer.WriteText(selected_text); scoped_clipboard_writer.WriteText(selected_text);
if (write_url && // Regardless of |write_url|, don't write a hyperlink to the clipboard.
base::FeatureList::IsEnabled(kOmniboxCanCopyHyperlinksToClipboard)) { // Plaintext URLs are simply handled more consistently than hyperlinks.
scoped_clipboard_writer.WriteHyperlink(selected_text, url.spec());
}
} }
void OmniboxViewViews::OnWriteDragData(ui::OSExchangeData* data) { void OmniboxViewViews::OnWriteDragData(ui::OSExchangeData* data) {
......
...@@ -788,10 +788,10 @@ TEST_P(OmniboxViewViewsClipboardTest, ClipboardCopyOrCutURL) { ...@@ -788,10 +788,10 @@ TEST_P(OmniboxViewViewsClipboardTest, ClipboardCopyOrCutURL) {
expected_text = base::ASCIIToUTF16("https://test.com/"); expected_text = base::ASCIIToUTF16("https://test.com/");
EXPECT_EQ(expected_text, omnibox_view()->GetText()); EXPECT_EQ(expected_text, omnibox_view()->GetText());
// Make sure both HTML and Plain Text formats are available. // Make sure the plain text format is available, but the HTML one isn't.
EXPECT_TRUE(clipboard->IsFormatAvailable( EXPECT_TRUE(clipboard->IsFormatAvailable(
ui::ClipboardFormatType::GetPlainTextType(), clipboard_buffer)); ui::ClipboardFormatType::GetPlainTextType(), clipboard_buffer));
EXPECT_TRUE(clipboard->IsFormatAvailable( EXPECT_FALSE(clipboard->IsFormatAvailable(
ui::ClipboardFormatType::GetHtmlType(), clipboard_buffer)); ui::ClipboardFormatType::GetHtmlType(), clipboard_buffer));
// Windows clipboard only supports text URLs. // Windows clipboard only supports text URLs.
......
...@@ -460,8 +460,7 @@ bool OmniboxEditModel::ShouldShowCurrentPageIcon() const { ...@@ -460,8 +460,7 @@ bool OmniboxEditModel::ShouldShowCurrentPageIcon() const {
return true; return true;
// If user input is in progress, keep showing the current page's icon so long // If user input is in progress, keep showing the current page's icon so long
// as the text matches the current page's URL, elided or unelided. This logic // as the text matches the current page's URL, elided or unelided.
// also works for Query in Omnibox, since the query is in |display_text_|.
return view_->GetText() == display_text_ || return view_->GetText() == display_text_ ||
view_->GetText() == url_for_editing_; view_->GetText() == url_for_editing_;
} }
......
...@@ -116,8 +116,7 @@ class OmniboxEditModel { ...@@ -116,8 +116,7 @@ class OmniboxEditModel {
// Adjusts the copied text before writing it to the clipboard. If the copied // Adjusts the copied text before writing it to the clipboard. If the copied
// text is a URL with the scheme elided, this method reattaches the scheme. // text is a URL with the scheme elided, this method reattaches the scheme.
// Copied text that looks like a search query, including the Query in Omnibox // Copied text that looks like a search query will not be modified.
// case, will not be modified.
// //
// |sel_min| gives the minimum of the selection, e.g. min(sel_start, sel_end). // |sel_min| gives the minimum of the selection, e.g. min(sel_start, sel_end).
// |text| is the currently selected text, and may be modified by this method. // |text| is the currently selected text, and may be modified by this method.
...@@ -505,12 +504,10 @@ class OmniboxEditModel { ...@@ -505,12 +504,10 @@ class OmniboxEditModel {
// no input is in progress or the Omnibox is not focused. // no input is in progress or the Omnibox is not focused.
OmniboxFocusSource focus_source_ = OmniboxFocusSource::INVALID; OmniboxFocusSource focus_source_ = OmniboxFocusSource::INVALID;
// Display-only text representing the current page. This could be any of: // Display-only text representing the current page. This could either:
// - The same as |url_for_editing_| if Steady State Elisions is OFF. // - The same as |url_for_editing_| if Steady State Elisions is OFF.
// - A simplified version of |url_for_editing_| with some destructive // - A simplified version of |url_for_editing_| with some destructive
// elisions applied. This is the case if Steady State Elisions is ON. // elisions applied. This is the case if Steady State Elisions is ON.
// - The user entered search query, if the user is on the search results
// page of the default search provider and Query in Omnibox is ON.
// //
// This should not be considered suitable for editing. // This should not be considered suitable for editing.
base::string16 display_text_; base::string16 display_text_;
......
...@@ -207,8 +207,7 @@ class ZeroSuggestProvider : public BaseSearchProvider { ...@@ -207,8 +207,7 @@ class ZeroSuggestProvider : public BaseSearchProvider {
// Loader used to retrieve results. // Loader used to retrieve results.
std::unique_ptr<network::SimpleURLLoader> loader_; std::unique_ptr<network::SimpleURLLoader> loader_;
// The verbatim match for the current text, whether it's a URL or search query // The verbatim match for the current text, which is always a URL.
// (which can occur for Query in Omnibox / Query Refinements).
AutocompleteMatch current_text_match_; AutocompleteMatch current_text_match_;
// Contains suggest and navigation results as well as relevance parsed from // Contains suggest and navigation results as well as relevance parsed from
......
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