From e6bcce8cc6eebf490be411d8b37b032bc69b753d Mon Sep 17 00:00:00 2001 From: Cepera Date: Fri, 10 Mar 2023 13:22:33 +0300 Subject: [PATCH 1/2] Do not show copy clean link for plain text (#17541) * Do not show copy clean link for plain text * Added tests for clean link item --- browser/renderer_context_menu/BUILD.gn | 5 + browser/renderer_context_menu/test/BUILD.gn | 23 +++ .../test/render_view_context_menu_unittest.cc | 135 ++++++++++++++++++ .../render_view_context_menu.cc | 46 +++++- .../render_view_context_menu.h | 1 + test/BUILD.gn | 1 + 6 files changed, 205 insertions(+), 6 deletions(-) create mode 100644 browser/renderer_context_menu/test/BUILD.gn create mode 100644 browser/renderer_context_menu/test/render_view_context_menu_unittest.cc diff --git a/browser/renderer_context_menu/BUILD.gn b/browser/renderer_context_menu/BUILD.gn index 171e85c186c9..2a6d58267894 100644 --- a/browser/renderer_context_menu/BUILD.gn +++ b/browser/renderer_context_menu/BUILD.gn @@ -1,3 +1,8 @@ +# Copyright (c) 2018 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 https://mozilla.org/MPL/2.0/. + source_set("renderer_context_menu") { # Remove when https://github.com/brave/brave-browser/issues/10657 is resolved check_includes = false diff --git a/browser/renderer_context_menu/test/BUILD.gn b/browser/renderer_context_menu/test/BUILD.gn new file mode 100644 index 000000000000..dcffcf1fb2a9 --- /dev/null +++ b/browser/renderer_context_menu/test/BUILD.gn @@ -0,0 +1,23 @@ +# Copyright (c) 2023 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 https://mozilla.org/MPL/2.0/. + +source_set("unit_tests") { + testonly = true + assert(is_win || is_linux || is_mac) + sources = [ "render_view_context_menu_unittest.cc" ] + deps = [ + "//base", + "//brave/app:command_ids", + "//chrome/browser", + "//chrome/test:test_support", + "//components/custom_handlers", + "//components/custom_handlers:test_support", + "//components/search_engines", + "//content/test:test_support", + "//testing/gmock", + "//testing/gtest", + "//third_party/abseil-cpp:absl", + ] +} diff --git a/browser/renderer_context_menu/test/render_view_context_menu_unittest.cc b/browser/renderer_context_menu/test/render_view_context_menu_unittest.cc new file mode 100644 index 000000000000..fd876f50e096 --- /dev/null +++ b/browser/renderer_context_menu/test/render_view_context_menu_unittest.cc @@ -0,0 +1,135 @@ +/* Copyright (c) 2023 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 https://mozilla.org/MPL/2.0/. */ + +#include "chrome/browser/renderer_context_menu/render_view_context_menu.h" + +#include "brave/app/brave_command_ids.h" +#include "chrome/browser/autocomplete/autocomplete_classifier_factory.h" +#include "chrome/browser/autocomplete/chrome_autocomplete_provider_client.h" +#include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h" +#include "chrome/browser/search_engines/template_url_service_factory.h" +#include "chrome/test/base/scoped_testing_local_state.h" +#include "chrome/test/base/testing_browser_process.h" +#include "chrome/test/base/testing_profile.h" +#include "components/custom_handlers/protocol_handler_registry.h" +#include "components/custom_handlers/test_protocol_handler_registry_delegate.h" +#include "components/search_engines/template_url_service.h" +#include "content/public/browser/context_menu_params.h" +#include "content/public/browser/web_contents.h" +#include "content/public/test/browser_task_environment.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +content::ContextMenuParams CreateSelectedTextParams( + const std::u16string& selected_text) { + content::ContextMenuParams rv; + rv.is_editable = false; + rv.page_url = GURL("http://test.page/"); + rv.selection_text = selected_text; + return rv; +} + +content::ContextMenuParams CreateLinkParams(const GURL& selected_link) { + content::ContextMenuParams rv; + rv.is_editable = false; + rv.unfiltered_link_url = selected_link; + rv.page_url = GURL("http://test.page/"); + rv.link_url = selected_link; + return rv; +} + +std::unique_ptr BuildProtocolHandlerRegistry( + content::BrowserContext* context) { + Profile* profile = Profile::FromBrowserContext(context); + return std::make_unique( + profile->GetPrefs(), + std::make_unique()); +} + +} // namespace + +class BraveRenderViewContextMenuMock : public BraveRenderViewContextMenu { + using BraveRenderViewContextMenu::BraveRenderViewContextMenu; + void Show() override {} +}; + +class BraveRenderViewContextMenuTest : public testing::Test { + protected: + BraveRenderViewContextMenuTest() + : testing_local_state_(TestingBrowserProcess::GetGlobal()) {} + content::WebContents* GetWebContents() { return web_contents_.get(); } + + // Returns a test context menu. + std::unique_ptr CreateContextMenu( + content::WebContents* web_contents, + content::ContextMenuParams params) { + auto menu = std::make_unique( + *web_contents->GetPrimaryMainFrame(), params); + menu->Init(); + return menu; + } + + void SetUp() override { + TestingProfile::Builder builder; + builder.AddTestingFactory( + TemplateURLServiceFactory::GetInstance(), + base::BindRepeating(&TemplateURLServiceFactory::BuildInstanceFor)); + profile_ = builder.Build(); + web_contents_ = content::WebContents::Create( + content::WebContents::CreateParams(profile_.get())); + auto* service = TemplateURLServiceFactory::GetForProfile(profile_.get()); + EXPECT_TRUE(service); + client_ = + std::make_unique(profile_.get()); + registry_ = std::make_unique( + profile_.get()->GetPrefs(), nullptr); + AutocompleteClassifierFactory::GetInstance()->SetTestingFactoryAndUse( + profile_.get(), + base::BindRepeating(&AutocompleteClassifierFactory::BuildInstanceFor)); + ProtocolHandlerRegistryFactory::GetInstance()->SetTestingFactory( + profile_.get(), base::BindRepeating(&BuildProtocolHandlerRegistry)); + } + void TearDown() override { registry_.reset(); } + + private: + content::BrowserTaskEnvironment browser_task_environment; + std::unique_ptr profile_; + std::unique_ptr registry_; + ScopedTestingLocalState testing_local_state_; + std::unique_ptr client_; + std::unique_ptr web_contents_; +}; + +TEST_F(BraveRenderViewContextMenuTest, MenuForPlainText) { + content::ContextMenuParams params = CreateSelectedTextParams(u"plain text"); + auto context_menu = CreateContextMenu(GetWebContents(), params); + EXPECT_TRUE(context_menu); + absl::optional clean_link_index = + context_menu->menu_model().GetIndexOfCommandId(IDC_COPY_CLEAN_LINK); + EXPECT_FALSE(clean_link_index.has_value()); +} + +TEST_F(BraveRenderViewContextMenuTest, MenuForSelectedUrl) { + content::ContextMenuParams params = CreateSelectedTextParams(u"brave.com"); + auto context_menu = CreateContextMenu(GetWebContents(), params); + EXPECT_TRUE(context_menu); + absl::optional clean_link_index = + context_menu->menu_model().GetIndexOfCommandId(IDC_COPY_CLEAN_LINK); + EXPECT_TRUE(clean_link_index.has_value()); + EXPECT_TRUE(context_menu->IsCommandIdEnabled(IDC_COPY_CLEAN_LINK)); +} + +TEST_F(BraveRenderViewContextMenuTest, MenuForLink) { + content::ContextMenuParams params = + CreateLinkParams(GURL("https://brave.com")); + auto context_menu = CreateContextMenu(GetWebContents(), params); + EXPECT_TRUE(context_menu); + absl::optional clean_link_index = + context_menu->menu_model().GetIndexOfCommandId(IDC_COPY_CLEAN_LINK); + EXPECT_TRUE(clean_link_index.has_value()); + EXPECT_TRUE(context_menu->IsCommandIdEnabled(IDC_COPY_CLEAN_LINK)); +} diff --git a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc index 838f2aee16c8..38bec49148cf 100644 --- a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc +++ b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.cc @@ -19,6 +19,7 @@ #include "chrome/common/channel_info.h" #include "components/omnibox/browser/autocomplete_classifier.h" #include "components/omnibox/browser/autocomplete_controller.h" +#include "components/omnibox/browser/autocomplete_match_type.h" #include "net/base/filename_util.h" #include "ui/base/models/menu_separator_types.h" #include "ui/base/resource/resource_bundle.h" @@ -46,7 +47,8 @@ namespace { -GURL GetSelectionNavigationURL(Profile* profile, const std::u16string& text) { +AutocompleteMatch GetAutocompleteMatchForText(Profile* profile, + const std::u16string& text) { AutocompleteMatch match; AutocompleteClassifier classifier( std::make_unique( @@ -56,6 +58,19 @@ GURL GetSelectionNavigationURL(Profile* profile, const std::u16string& text) { classifier.Classify(text, false, false, metrics::OmniboxEventProto::INVALID_SPEC, &match, NULL); classifier.Shutdown(); + return match; +} + +GURL GetSelectionNavigationURL(Profile* profile, const std::u16string& text) { + return GetAutocompleteMatchForText(profile, text).destination_url; +} + +absl::optional GetSelectedURL(Profile* profile, + const std::u16string& text) { + auto match = GetAutocompleteMatchForText(profile, text); + if (match.type != AutocompleteMatchType::URL_WHAT_YOU_TYPED) { + return absl::nullopt; + } return match.destination_url; } @@ -169,7 +184,8 @@ bool BraveRenderViewContextMenu::IsCommandIdEnabled(int id) const { return params_.has_image_contents; #endif case IDC_COPY_CLEAN_LINK: - return params_.link_url.is_valid(); + return params_.link_url.is_valid() || + GetSelectedURL(GetProfile(), params_.selection_text).has_value(); case IDC_CONTENT_CONTEXT_FORCE_PASTE: // only enable if there is plain text data to paste - this is what // IsPasteAndMatchStyleEnabled checks internally, but IsPasteEnabled @@ -237,9 +253,19 @@ void BraveRenderViewContextMenu::ExecuteIPFSCommand(int id, int event_flags) { void BraveRenderViewContextMenu::ExecuteCommand(int id, int event_flags) { switch (id) { - case IDC_COPY_CLEAN_LINK: - brave::CopyLinkWithStrictCleaning(GetBrowser(), params_.link_url); - break; + case IDC_COPY_CLEAN_LINK: { + GURL link_url = params_.link_url; + if (!link_url.is_valid()) { + auto selected_url = + GetSelectedURL(GetProfile(), params_.selection_text); + if (selected_url.has_value()) { + link_url = selected_url.value(); + } else { + return; + } + } + brave::CopyLinkWithStrictCleaning(GetBrowser(), link_url); + }; break; case IDC_CONTENT_CONTEXT_FORCE_PASTE: { std::u16string result; ui::Clipboard::GetForCurrentThread()->ReadText( @@ -453,7 +479,15 @@ void BraveRenderViewContextMenu::InitMenu() { link_index.value() + 1, IDC_COPY_CLEAN_LINK, IDS_COPY_CLEAN_LINK); } } - + if (GetSelectedURL(GetProfile(), params_.selection_text).has_value()) { + absl::optional copy_index = + menu_model_.GetIndexOfCommandId(IDC_CONTENT_CONTEXT_COPY); + if (copy_index.has_value() && + !menu_model_.GetIndexOfCommandId(IDC_COPY_CLEAN_LINK).has_value()) { + menu_model_.InsertItemWithStringIdAt( + copy_index.value() + 1, IDC_COPY_CLEAN_LINK, IDS_COPY_CLEAN_LINK); + } + } #if BUILDFLAG(ENABLE_IPFS) BuildIPFSMenu(); #endif diff --git a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h index 7b5136edccb7..298f98acd285 100644 --- a/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h +++ b/chromium_src/chrome/browser/renderer_context_menu/render_view_context_menu.h @@ -45,6 +45,7 @@ class BraveRenderViewContextMenu : public RenderViewContextMenu_Chromium { void AddAccessibilityLabelsServiceItem(bool is_checked) override; private: + friend class BraveRenderViewContextMenuTest; // RenderViewContextMenuBase: void InitMenu() override; #if BUILDFLAG(ENABLE_IPFS) diff --git a/test/BUILD.gn b/test/BUILD.gn index 3c5b9aedb914..e8e85170520b 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -398,6 +398,7 @@ test("brave_unit_tests") { ] deps += [ "//brave/app:brave_generated_resources_grit", + "//brave/browser/renderer_context_menu/test:unit_tests", "//brave/browser/ui/bookmark", "//brave/browser/ui/bookmark:unittest", "//brave/browser/ui/color:unit_tests", From 9e591f366fc56b3991ec991a4e1beff1b3b047fc Mon Sep 17 00:00:00 2001 From: sergei Date: Mon, 20 Mar 2023 11:14:31 +0300 Subject: [PATCH 2/2] Block failed test --- test/filters/unit_tests.filter | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/filters/unit_tests.filter b/test/filters/unit_tests.filter index 836f00011309..519a92bcbbb8 100644 --- a/test/filters/unit_tests.filter +++ b/test/filters/unit_tests.filter @@ -578,3 +578,6 @@ -WebUsbServiceImplTest.* -WellKnownChangePasswordNavigationThrottleTest.* -ZipFileInstallerTest.* + +# Crash because of starting search provider +-RenderViewContextMenuPrefsTest.ShowAllPasswordsIncognito