From 9b5da04c2a4e833cfca29917594f73a800ce4283 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Wed, 5 Feb 2020 11:52:32 +0900 Subject: [PATCH] Fixed brave scheme url is changed to chrome scheme url when copying from omnibox fix https://github.com/brave/brave-browser/issues/1973 --- browser/ui/BUILD.gn | 5 ++ .../views/omnibox/brave_omnibox_view_views.cc | 65 +++++++++++++++++++ .../views/omnibox/brave_omnibox_view_views.h | 24 +++++++ .../brave_omnibox_view_views_browsertest.cc | 43 ++++++++++++ .../views/location_bar/location_bar_view.cc | 5 +- test/BUILD.gn | 1 + 6 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 browser/ui/views/omnibox/brave_omnibox_view_views.cc create mode 100644 browser/ui/views/omnibox/brave_omnibox_view_views.h create mode 100644 browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc diff --git a/browser/ui/BUILD.gn b/browser/ui/BUILD.gn index 4a05dfb9361c..8d3deb6a4c9c 100644 --- a/browser/ui/BUILD.gn +++ b/browser/ui/BUILD.gn @@ -115,6 +115,8 @@ source_set("ui") { "views/frame/brave_browser_view.h", "views/importer/brave_import_lock_dialog_view.cc", "views/importer/brave_import_lock_dialog_view.h", + "views/omnibox/brave_omnibox_view_views.cc", + "views/omnibox/brave_omnibox_view_views.h", "views/reader_mode/brave_reader_mode_icon_view.cc", "views/reader_mode/brave_reader_mode_icon_view.h", "views/rounded_separator.cc", @@ -205,6 +207,7 @@ source_set("ui") { "//chrome/app/vector_icons:vector_icons", "//chrome/common", "//components/gcm_driver:gcm_buildflags", + "//components/omnibox/browser", "//components/prefs", "//components/sessions", "//content/public/browser", @@ -213,8 +216,10 @@ source_set("ui") { "//skia", "//ui/accessibility", "//ui/base", + "//ui/base/clipboard", "//ui/gfx", "//ui/resources", + "//url", ] # This is no longer compiled into Chromium on Android, but we still diff --git a/browser/ui/views/omnibox/brave_omnibox_view_views.cc b/browser/ui/views/omnibox/brave_omnibox_view_views.cc new file mode 100644 index 000000000000..ab4851eb2afe --- /dev/null +++ b/browser/ui/views/omnibox/brave_omnibox_view_views.cc @@ -0,0 +1,65 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/browser/ui/views/omnibox/brave_omnibox_view_views.h" + +#include "base/feature_list.h" +#include "brave/browser/ui/toolbar/brave_location_bar_model_delegate.h" +#include "brave/common/url_constants.h" +#include "components/omnibox/browser/omnibox_edit_model.h" +#include "content/public/common/url_constants.h" +#include "ui/base/clipboard/scoped_clipboard_writer.h" +#include "url/gurl.h" + +namespace { +// Copied from omnibox_view_views.cc +constexpr base::Feature kOmniboxCanCopyHyperlinksToClipboard{ + "OmniboxCanCopyHyperlinksToClipboard", base::FEATURE_ENABLED_BY_DEFAULT}; + +bool ShouldAdjust(const base::string16& original, + const base::string16& adjusted) { + // Only adjust scheme is changed from brave to chrome. + if (GURL(original).scheme() == kBraveUIScheme && + GURL(adjusted).scheme() == content::kChromeUIScheme) { + return true; + } + + return false; +} + +void ReplaceChromeSchemeToBrave(GURL* url) { + DCHECK_EQ(content::kChromeUIScheme, url->scheme()); + GURL::Replacements replacements; + replacements.SetSchemeStr(kBraveUIScheme); + *url = url->ReplaceComponents(replacements); +} + +} // namespace + +void BraveOmniboxViewViews::OnAfterCutOrCopy( + ui::ClipboardBuffer clipboard_buffer) { + ui::Clipboard* cb = ui::Clipboard::GetForCurrentThread(); + base::string16 selected_text; + cb->ReadText(clipboard_buffer, &selected_text); + const base::string16 original_selected_text = selected_text; + + GURL url; + bool write_url = false; + model()->AdjustTextForCopy(GetSelectedRange().GetMin(), &selected_text, &url, + &write_url); + + if (ShouldAdjust(original_selected_text, selected_text)) { + BraveLocationBarModelDelegate::FormattedStringFromURL(url, &selected_text); + ReplaceChromeSchemeToBrave(&url); + } + + ui::ScopedClipboardWriter scoped_clipboard_writer(clipboard_buffer); + scoped_clipboard_writer.WriteText(selected_text); + + if (write_url && + base::FeatureList::IsEnabled(kOmniboxCanCopyHyperlinksToClipboard)) { + scoped_clipboard_writer.WriteHyperlink(selected_text, url.spec()); + } +} diff --git a/browser/ui/views/omnibox/brave_omnibox_view_views.h b/browser/ui/views/omnibox/brave_omnibox_view_views.h new file mode 100644 index 000000000000..cd2d13fdce5c --- /dev/null +++ b/browser/ui/views/omnibox/brave_omnibox_view_views.h @@ -0,0 +1,24 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_BROWSER_UI_VIEWS_OMNIBOX_BRAVE_OMNIBOX_VIEW_VIEWS_H_ +#define BRAVE_BROWSER_UI_VIEWS_OMNIBOX_BRAVE_OMNIBOX_VIEW_VIEWS_H_ + +#include "chrome/browser/ui/views/omnibox/omnibox_view_views.h" + +class BraveOmniboxViewViews : public OmniboxViewViews { + public: + using OmniboxViewViews::OmniboxViewViews; + ~BraveOmniboxViewViews() override = default; + + BraveOmniboxViewViews(const BraveOmniboxViewViews&) = delete; + BraveOmniboxViewViews& operator=(const BraveOmniboxViewViews&) = delete; + + private: + // OmniboxViewViews overrides: + void OnAfterCutOrCopy(ui::ClipboardBuffer clipboard_buffer) override; +}; + +#endif // BRAVE_BROWSER_UI_VIEWS_OMNIBOX_BRAVE_OMNIBOX_VIEW_VIEWS_H_ diff --git a/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc b/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc new file mode 100644 index 000000000000..07cc0c9fabb6 --- /dev/null +++ b/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc @@ -0,0 +1,43 @@ +/* Copyright (c) 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "chrome/browser/ui/views/frame/browser_view.h" +#include "chrome/browser/ui/views/location_bar/location_bar_view.h" +#include "chrome/browser/ui/views/omnibox/omnibox_view_views.h" +#include "chrome/browser/ui/views/toolbar/toolbar_view.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/ui_test_utils.h" +#include "url/gurl.h" +#include "ui/base/clipboard/clipboard.h" +#include "ui/strings/grit/ui_strings.h" + +class BraveOmniboxViewViewsTest : public InProcessBrowserTest { + public: + LocationBarView* location_bar() { + auto* browser_view = BrowserView::GetBrowserViewForBrowser(browser()); + return browser_view->toolbar()->location_bar(); + } + OmniboxViewViews* omnibox_view() { return location_bar()->omnibox_view(); } +}; + +// Load brave url and check copied url also has brave scheme. +IN_PROC_BROWSER_TEST_F(BraveOmniboxViewViewsTest, CopyURLToClipboardTest) { + const std::string test_url("brave://version/"); + ui_test_utils::NavigateToURL(browser(), GURL(test_url)); + + omnibox_view()->SelectAll(true); + omnibox_view()->ExecuteCommand(IDS_APP_COPY, 0); + ui::Clipboard* clipboard = ui::Clipboard::GetForCurrentThread(); + std::string text_from_clipboard; + clipboard->ReadAsciiText(ui::ClipboardBuffer::kCopyPaste, + &text_from_clipboard); + EXPECT_EQ(test_url, text_from_clipboard); + +#if defined(OS_LINUX) + clipboard->ReadAsciiText(ui::ClipboardBuffer::kSelection, + &text_from_clipboard); + EXPECT_EQ(test_url, text_from_clipboard); +#endif +} diff --git a/chromium_src/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chromium_src/chrome/browser/ui/views/location_bar/location_bar_view.cc index c2c2d6650fea..d5f668b1388e 100644 --- a/chromium_src/chrome/browser/ui/views/location_bar/location_bar_view.cc +++ b/chromium_src/chrome/browser/ui/views/location_bar/location_bar_view.cc @@ -4,12 +4,15 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "brave/browser/ui/omnibox/brave_omnibox_client_impl.h" +#include "brave/browser/ui/views/omnibox/brave_omnibox_view_views.h" #define BRAVE_LAYOUT_TRAILING_DECORATIONS \ if (right_most && right_most->GetVisible()) \ trailing_decorations.AddDecoration(0, height(), false, 0, 0, right_most); #define ChromeOmniboxClient BraveOmniboxClientImpl -#include "../../../../../../../chrome/browser/ui/views/location_bar/location_bar_view.cc" +#define OmniboxViewViews BraveOmniboxViewViews +#include "../../../../../../../chrome/browser/ui/views/location_bar/location_bar_view.cc" // NOLINT #undef ChromeOmniboxClient +#undef OmniboxViewViews #undef BRAVE_LAYOUT_TRAILING_DECORATIONS diff --git a/test/BUILD.gn b/test/BUILD.gn index 9dea520ae899..67d6c6f4b204 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -534,6 +534,7 @@ test("brave_browser_tests") { "//brave/browser/ui/brave_browser_command_controller_browsertest.cc", "//brave/browser/ui/content_settings/brave_autoplay_blocked_image_model_browsertest.cc", "//brave/browser/ui/views/brave_actions/brave_actions_container_browsertest.cc", + "//brave/browser/ui/views/omnibox/brave_omnibox_view_views_browsertest.cc", "//brave/browser/ui/views/omnibox/omnibox_autocomplete_browsertest.cc", "//brave/browser/ui/views/profiles/brave_profile_menu_view_browsertest.cc", "//brave/browser/ui/views/tabs/brave_tab_context_menu_contents_browsertest.cc",