From c82efbcebbdbf336e36ece866f7579b144323d7c Mon Sep 17 00:00:00 2001 From: Max Karolinskiy Date: Thu, 18 Oct 2018 00:23:25 -0400 Subject: [PATCH] Hide RenderViewContextMenu::AddSpellCheckServiceItem method with one in BraveRenderViewContextMenu that does not add Ask Brave menu item. Fixes brave/brave-browser#1623 Also, overrides SpellingMenuObserver to remove a possible dangling menu separator. Adds browser tests for spelling menu and spelling options submenu that check that the "Ask Brave for suggestions" menu item is not present. --- browser/BUILD.gn | 1 + browser/renderer_context_menu/BUILD.gn | 16 ++ .../brave_mock_render_view_context_menu.cc | 251 ++++++++++++++++++ .../brave_mock_render_view_context_menu.h | 109 ++++++++ ...rave_spelling_menu_observer_browsertest.cc | 125 +++++++++ ...brave_spelling_options_submenu_observer.cc | 69 +++++ .../brave_spelling_options_submenu_observer.h | 32 +++ ...ng_options_submenu_observer_browsertest.cc | 153 +++++++++++ .../render_view_context_menu.cc | 29 ++ .../render_view_context_menu.h | 5 + ...-spelling_options_submenu_observer.h.patch | 17 ++ test/BUILD.gn | 9 + 12 files changed, 816 insertions(+) create mode 100644 browser/renderer_context_menu/BUILD.gn create mode 100644 browser/renderer_context_menu/brave_mock_render_view_context_menu.cc create mode 100644 browser/renderer_context_menu/brave_mock_render_view_context_menu.h create mode 100644 browser/renderer_context_menu/brave_spelling_menu_observer_browsertest.cc create mode 100644 browser/renderer_context_menu/brave_spelling_options_submenu_observer.cc create mode 100644 browser/renderer_context_menu/brave_spelling_options_submenu_observer.h create mode 100644 browser/renderer_context_menu/brave_spelling_options_submenu_observer_browsertest.cc create mode 100644 patches/chrome-browser-renderer_context_menu-spelling_options_submenu_observer.h.patch diff --git a/browser/BUILD.gn b/browser/BUILD.gn index f240b5504a7a..19114d196e6f 100644 --- a/browser/BUILD.gn +++ b/browser/BUILD.gn @@ -127,6 +127,7 @@ source_set("browser") { "permissions", "profiles", "referrals", + "renderer_context_menu", "renderer_host", "tor", "ui", diff --git a/browser/renderer_context_menu/BUILD.gn b/browser/renderer_context_menu/BUILD.gn new file mode 100644 index 000000000000..bff60fc33b8b --- /dev/null +++ b/browser/renderer_context_menu/BUILD.gn @@ -0,0 +1,16 @@ +source_set("renderer_context_menu") { + sources = [] + if (is_win || is_linux) { + sources += [ + "brave_spelling_options_submenu_observer.cc", + "brave_spelling_options_submenu_observer.h", + ] + } + + deps = [] + if (is_win || is_linux) { + deps += [ + "//chrome/browser", + ] + } +} diff --git a/browser/renderer_context_menu/brave_mock_render_view_context_menu.cc b/browser/renderer_context_menu/brave_mock_render_view_context_menu.cc new file mode 100644 index 000000000000..c286bca5d8f3 --- /dev/null +++ b/browser/renderer_context_menu/brave_mock_render_view_context_menu.cc @@ -0,0 +1,251 @@ +/* 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/renderer_context_menu/brave_mock_render_view_context_menu.h" + +#include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/renderer_context_menu/render_view_context_menu.h" +#include "chrome/grit/generated_resources.h" +#include "chrome/test/base/testing_profile.h" +#include "components/prefs/pref_service.h" +#include "components/renderer_context_menu/render_view_context_menu_observer.h" +#include "components/renderer_context_menu/render_view_context_menu_proxy.h" +#include "content/public/browser/browser_context.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/base/l10n/l10n_util.h" + +BraveMockRenderViewContextMenu::MockMenuItem::MockMenuItem() + : command_id(0), + enabled(false), + checked(false), + hidden(false), + is_submenu(false), + has_submenu(false) {} + +BraveMockRenderViewContextMenu::MockMenuItem::MockMenuItem( + const MockMenuItem& other) = default; + +BraveMockRenderViewContextMenu::MockMenuItem::~MockMenuItem() {} + +BraveMockRenderViewContextMenu::MockMenuItem& +BraveMockRenderViewContextMenu::MockMenuItem::operator=( + const MockMenuItem& other) = default; + +void BraveMockRenderViewContextMenu::MockMenuItem::PrintMockMenuItem( + unsigned int offset) const { + std::cout << std::setfill(' '); + if (offset) + std::cout << std::setfill(' ') << std::setw(offset) << ' '; + std::cout << (has_submenu ? "> " : " "); + if (command_id == -1) { + std::cout << std::setfill(' ') << std::setw(9) << ' '; + std::cout << std::setfill('-') << std::setw(15) << '-'; + } else { + std::cout << std::setw(8) << command_id; + std::cout << " " << title; + if (!enabled || checked || hidden) { + std::cout << " ("; + if (!enabled) + std::cout << " disabled "; + if (checked) + std::cout << " checked "; + if (hidden) + std::cout << " hidden "; + std::cout << ")"; + } + } + std::cout << std::endl; +} + +BraveMockRenderViewContextMenu::BraveMockRenderViewContextMenu(Profile* profile) + : observer_(nullptr), profile_(profile), enable_print_menu_(false) {} + +BraveMockRenderViewContextMenu::~BraveMockRenderViewContextMenu() {} + +// SimpleMenuModel::Delegate implementation. + +bool BraveMockRenderViewContextMenu::IsCommandIdChecked(int command_id) const { + return observer_->IsCommandIdChecked(command_id); +} + +bool BraveMockRenderViewContextMenu::IsCommandIdEnabled(int command_id) const { + return observer_->IsCommandIdEnabled(command_id); +} + +void BraveMockRenderViewContextMenu::ExecuteCommand(int command_id, + int event_flags) { + observer_->ExecuteCommand(command_id); +} + +// RenderViewContextMenuProxy implementation. + +void BraveMockRenderViewContextMenu::AddMenuItem(int command_id, + const base::string16& title) { + MockMenuItem item; + item.command_id = command_id; + item.enabled = observer_->IsCommandIdEnabled(command_id); + item.checked = false; + item.hidden = false; + item.title = title; + items_.push_back(item); +} + +void BraveMockRenderViewContextMenu::AddCheckItem(int command_id, + const base::string16& title) { + MockMenuItem item; + item.command_id = command_id; + item.enabled = observer_->IsCommandIdEnabled(command_id); + item.checked = observer_->IsCommandIdChecked(command_id); + item.hidden = false; + item.title = title; + items_.push_back(item); +} + +void BraveMockRenderViewContextMenu::AddSeparator() { + MockMenuItem item; + item.command_id = -1; + item.enabled = false; + item.checked = false; + item.hidden = false; + items_.push_back(item); +} + +void BraveMockRenderViewContextMenu::AddSubMenu(int command_id, + const base::string16& label, + ui::MenuModel* model) { + MockMenuItem item; + item.command_id = command_id; + item.enabled = observer_->IsCommandIdEnabled(command_id); + item.checked = observer_->IsCommandIdChecked(command_id); + item.hidden = false; + item.title = label; + item.has_submenu = true; + items_.push_back(item); + + for (int i = 0; i < model->GetItemCount(); ++i) { + MockMenuItem sub_item; + sub_item.is_submenu = true; + if (model->GetTypeAt(i) != ui::MenuModel::TYPE_SEPARATOR) { + sub_item.command_id = model->GetCommandIdAt(i); + sub_item.enabled = observer_->IsCommandIdSupported(sub_item.command_id) + ? model->IsEnabledAt(i) + : false; + sub_item.checked = observer_->IsCommandIdSupported(sub_item.command_id) + ? model->IsItemCheckedAt(i) + : false; + sub_item.hidden = !model->IsVisibleAt(i); + sub_item.title = model->GetLabelAt(i); + } else { + sub_item.command_id = -1; + } + items_.push_back(sub_item); + } +} + +void BraveMockRenderViewContextMenu::UpdateMenuItem( + int command_id, + bool enabled, + bool hidden, + const base::string16& title) { + for (auto& item : items_) { + if (item.command_id == command_id) { + item.enabled = enabled; + item.hidden = hidden; + item.title = title; + return; + } + } + + FAIL() << "Menu observer is trying to change a menu item it doesn't own." + << " command_id: " << command_id; +} + +void BraveMockRenderViewContextMenu::UpdateMenuIcon(int command_id, + const gfx::Image& image) { + for (auto& item : items_) { + if (item.command_id == command_id) { + return; + } + } + + FAIL() << "Menu observer is trying to change a menu item it doesn't own." + << " command_id: " << command_id; +} + +void BraveMockRenderViewContextMenu::RemoveMenuItem(int command_id) { + auto it = items_.begin(); + while (it != items_.end()) { + if (it->command_id == command_id) { + bool submenu = it->has_submenu; + it = items_.erase(it); + if (submenu) { + while (it != items_.end() && it->is_submenu) + it = items_.erase(it); + } + break; + } else + ++it; + } +} + +void BraveMockRenderViewContextMenu::RemoveAdjacentSeparators() {} + +void BraveMockRenderViewContextMenu::AddSpellCheckServiceItem(bool is_checked) { + // Call the static method of RenderViewContextMenu which should our override + // that doesn't add the item. + RenderViewContextMenu::AddSpellCheckServiceItem(nullptr, is_checked); +} + +content::RenderViewHost* BraveMockRenderViewContextMenu::GetRenderViewHost() + const { + return nullptr; +} + +content::BrowserContext* BraveMockRenderViewContextMenu::GetBrowserContext() + const { + return profile_; +} + +content::WebContents* BraveMockRenderViewContextMenu::GetWebContents() const { + return nullptr; +} + +// Methods that don't implement inherited interfaces. + +void BraveMockRenderViewContextMenu::SetObserver( + RenderViewContextMenuObserver* observer) { + observer_ = observer; +} + +size_t BraveMockRenderViewContextMenu::GetMenuSize() const { + return items_.size(); +} + +bool BraveMockRenderViewContextMenu::GetMenuItem(size_t index, + MockMenuItem* item) const { + if (index >= items_.size()) + return false; + *item = items_[index]; + return true; +} + +PrefService* BraveMockRenderViewContextMenu::GetPrefs() { + return profile_->GetPrefs(); +} + +void BraveMockRenderViewContextMenu::PrintMenu(const std::string& title) const { + if (!enable_print_menu_) + return; + + std::cout << title << std::endl; + std::cout << std::setfill('-') << std::setw(40) << '-' << std::endl; + for (const auto& item : items_) + item.PrintMockMenuItem(item.is_submenu ? 4 : 0); + std::cout << std::setfill('-') << std::setw(40) << '-' << std::endl; +} + +void BraveMockRenderViewContextMenu::EnablePrintMenu(bool enable) { + enable_print_menu_ = enable; +} diff --git a/browser/renderer_context_menu/brave_mock_render_view_context_menu.h b/browser/renderer_context_menu/brave_mock_render_view_context_menu.h new file mode 100644 index 000000000000..868e71e3eb14 --- /dev/null +++ b/browser/renderer_context_menu/brave_mock_render_view_context_menu.h @@ -0,0 +1,109 @@ +/* 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_RENDERER_CONTEXT_MENU_BRAVE_MOCK_RENDER_VIEW_CONTEXT_MENU_H_ +#define BRAVE_BROWSER_RENDERER_CONTEXT_MENU_BRAVE_MOCK_RENDER_VIEW_CONTEXT_MENU_H_ + +#include +#include +#include + +#include "base/macros.h" +#include "base/strings/string16.h" +#include "components/renderer_context_menu/render_view_context_menu_proxy.h" +#include "ui/base/models/simple_menu_model.h" +#include "ui/gfx/image/image.h" + +class BraveMockRenderViewContextMenu; +class PrefService; +class Profile; +class RenderViewContextMenuObserver; + +// A mock context menu proxy used in tests. This class overrides virtual methods +// derived from the RenderViewContextMenuProxy class to monitor calls from a +// MenuObserver class. +class BraveMockRenderViewContextMenu : public ui::SimpleMenuModel::Delegate, + public RenderViewContextMenuProxy { + public: + // A menu item used in this test. + struct MockMenuItem { + MockMenuItem(); + MockMenuItem(const MockMenuItem& other); + ~MockMenuItem(); + + MockMenuItem& operator=(const MockMenuItem& other); + + void PrintMockMenuItem(unsigned int offset = 0) const; + + int command_id; + bool enabled; + bool checked; + bool hidden; + base::string16 title; + bool is_submenu; // This item lives in a submenu. + bool has_submenu; // This item is a submenu. + }; + + explicit BraveMockRenderViewContextMenu(Profile* profile); + ~BraveMockRenderViewContextMenu() override; + + // SimpleMenuModel::Delegate implementation. + bool IsCommandIdChecked(int command_id) const override; + bool IsCommandIdEnabled(int command_id) const override; + void ExecuteCommand(int command_id, int event_flags) override; + + // RenderViewContextMenuProxy implementation. + void AddMenuItem(int command_id, const base::string16& title) override; + void AddCheckItem(int command_id, const base::string16& title) override; + void AddSeparator() override; + void AddSubMenu(int command_id, + const base::string16& label, + ui::MenuModel* model) override; + void UpdateMenuItem(int command_id, + bool enabled, + bool hidden, + const base::string16& title) override; + void UpdateMenuIcon(int command_id, const gfx::Image& image) override; + void RemoveMenuItem(int command_id) override; + void RemoveAdjacentSeparators() override; + void AddSpellCheckServiceItem(bool is_checked) override; + content::RenderViewHost* GetRenderViewHost() const override; + content::BrowserContext* GetBrowserContext() const override; + content::WebContents* GetWebContents() const override; + + // Attaches a RenderViewContextMenuObserver to be tested. + void SetObserver(RenderViewContextMenuObserver* observer); + + // Returns the number of items added by the test. + size_t GetMenuSize() const; + + // Returns the item at |index|. + bool GetMenuItem(size_t index, MockMenuItem* item) const; + + // Returns the writable profile used in this test. + PrefService* GetPrefs(); + + // Prints the menu to the standard output. + void PrintMenu(const std::string& title) const; + void EnablePrintMenu(bool enable = true); + + private: + // An observer used for initializing the status of menu items added in this + // test. This is owned by our owner and the owner is responsible for its + // lifetime. + RenderViewContextMenuObserver* observer_; + + // Either a regular profile or an incognito profile. + Profile* profile_; + + // A list of menu items added. + std::vector items_; + + // Is menu printing enabled. + bool enable_print_menu_; + + DISALLOW_COPY_AND_ASSIGN(BraveMockRenderViewContextMenu); +}; + +#endif // BRAVE_BROWSER_RENDERER_CONTEXT_MENU_BRAVE_MOCK_RENDER_VIEW_CONTEXT_MENU_H_ diff --git a/browser/renderer_context_menu/brave_spelling_menu_observer_browsertest.cc b/browser/renderer_context_menu/brave_spelling_menu_observer_browsertest.cc new file mode 100644 index 000000000000..fd06ba60bdba --- /dev/null +++ b/browser/renderer_context_menu/brave_spelling_menu_observer_browsertest.cc @@ -0,0 +1,125 @@ +/* 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/renderer_context_menu/spelling_menu_observer.h" + +#include "base/macros.h" +#include "base/strings/utf_string_conversions.h" +#include "base/values.h" +#include "brave/browser/renderer_context_menu/brave_mock_render_view_context_menu.h" +#include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "components/prefs/pref_service.h" +#include "components/spellcheck/browser/pref_names.h" +#include "content/public/common/context_menu_params.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// A test class used in this file. This test should be a browser test because it +// accesses resources. +class BraveSpellingMenuObserverTest : public InProcessBrowserTest { + public: + BraveSpellingMenuObserverTest(); + + void SetUpOnMainThread() override {} + + void TearDownOnMainThread() override { + observer_.reset(); + menu_.reset(); + } + + void Reset(bool incognito = false) { + observer_.reset(); + menu_.reset(new BraveMockRenderViewContextMenu( + incognito ? browser()->profile()->GetOffTheRecordProfile() + : browser()->profile())); + observer_.reset(new SpellingMenuObserver(menu_.get())); + menu_->SetObserver(observer_.get()); + // Uncomment to print the menu to standard output for each test. + //menu_->EnablePrintMenu(); + } + + void InitMenu(const char* word, const char* suggestion) { + content::ContextMenuParams params; + params.is_editable = true; + params.misspelled_word = base::ASCIIToUTF16(word); + params.dictionary_suggestions.clear(); + if (suggestion) + params.dictionary_suggestions.push_back(base::ASCIIToUTF16(suggestion)); + observer_->InitMenu(params); + } + + void CheckExpected() { + for (size_t i = 0; i < menu()->GetMenuSize(); ++i) { + BraveMockRenderViewContextMenu::MockMenuItem item; + menu()->GetMenuItem(i, &item); + EXPECT_NE(IDC_CONTENT_CONTEXT_SPELLING_TOGGLE, item.command_id); + } + } + + ~BraveSpellingMenuObserverTest() override; + BraveMockRenderViewContextMenu* menu() { return menu_.get(); } + SpellingMenuObserver* observer() { return observer_.get(); } + + private: + std::unique_ptr observer_; + std::unique_ptr menu_; + DISALLOW_COPY_AND_ASSIGN(BraveSpellingMenuObserverTest); +}; + +BraveSpellingMenuObserverTest::BraveSpellingMenuObserverTest() {} + +BraveSpellingMenuObserverTest::~BraveSpellingMenuObserverTest() {} + +} // namespace + +// Tests that right-clicking not add "Ask Brave for suggestions". +IN_PROC_BROWSER_TEST_F(BraveSpellingMenuObserverTest, + CheckAskBraveNotShown) { + // Test menu with a misspelled word. + Reset(); + InitMenu("wiimode", nullptr); + menu()->PrintMenu("Test menu with a misspelled word."); + CheckExpected(); + + // Test menu with a correct word and spelling service enabled. + Reset(); + menu()->GetPrefs()->SetBoolean( + spellcheck::prefs::kSpellCheckUseSpellingService, true); + InitMenu("", nullptr); + menu()->PrintMenu("Test menu with spelling service enabled."); + CheckExpected(); + + // Test menu with a misspelled word and spelling service enabled. + Reset(); + menu()->GetPrefs()->SetBoolean( + spellcheck::prefs::kSpellCheckUseSpellingService, true); + InitMenu("wiimode", nullptr); + menu()->PrintMenu( + "Test menu with a misspelled word spelling service enabled."); + CheckExpected(); + + // Test menu with a misspelled word, a suggestion, and spelling service + // enabled. + Reset(); + menu()->GetPrefs()->SetBoolean( + spellcheck::prefs::kSpellCheckUseSpellingService, true); + InitMenu("wiimode", "wii mode"); + menu()->PrintMenu( + "Test menu with a misspelled word, a suggestion, spelling service " + "enabled."); + CheckExpected(); + + // Test menu with a misspelled word spelling service enabled in incognito + // profile (which doesn't allow spelling service). + Reset(true); + menu()->GetPrefs()->SetBoolean( + spellcheck::prefs::kSpellCheckUseSpellingService, true); + InitMenu("sjxdjiiiiii", nullptr); + menu()->PrintMenu("Test menu with spelling service enabled in incognito."); + CheckExpected(); +} diff --git a/browser/renderer_context_menu/brave_spelling_options_submenu_observer.cc b/browser/renderer_context_menu/brave_spelling_options_submenu_observer.cc new file mode 100644 index 000000000000..f142526f1fec --- /dev/null +++ b/browser/renderer_context_menu/brave_spelling_options_submenu_observer.cc @@ -0,0 +1,69 @@ +/* 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/renderer_context_menu/brave_spelling_options_submenu_observer.h" + +#include "chrome/app/chrome_command_ids.h" +#include "chrome/grit/generated_resources.h" +#include "components/renderer_context_menu/render_view_context_menu_proxy.h" +#include "ui/base/l10n/l10n_util.h" + +BraveSpellingOptionsSubMenuObserver::BraveSpellingOptionsSubMenuObserver( + RenderViewContextMenuProxy* proxy, + ui::SimpleMenuModel::Delegate* delegate, + int group_id) + : SpellingOptionsSubMenuObserver(proxy, delegate, group_id), + gtest_mode_(GTEST_MODE_DISABLED) {} + +void BraveSpellingOptionsSubMenuObserver::InitMenu( + const content::ContextMenuParams& params) { + // Let Chromium build the submenu. + SpellingOptionsSubMenuObserver::InitMenu(params); + + // Assumptions: + // 1. Use of spelling service is disabled in Brave profile preferences. + // 2. We overrode RenderViewContextMenu::AddSpellCheckServiceItem so that the + // spelling suggestions toggle isn't added to the menu by the base class. + DCHECK(!use_spelling_service_.GetValue()); + DCHECK(submenu_model_.GetIndexOfCommandId( + IDC_CONTENT_CONTEXT_SPELLING_TOGGLE) == -1); + + // Check if we ended up with a separator as the last item and, if so, get rid + // of it. + int index = submenu_model_.GetItemCount() - 1; + if (index >= 0 && + submenu_model_.GetTypeAt(index) == ui::MenuModel::TYPE_SEPARATOR) { + submenu_model_.RemoveItemAt(index); + } + DCHECK(submenu_model_.GetItemCount()); + + // Special accommodations for gtest. + if (gtest_mode_ != GTEST_MODE_DISABLED) { + if (gtest_mode_ == GTEST_MODE_EMPTY_SUBMENU) { + // Simulate empty submenu situation to test the UpdateMenuItem code below. + submenu_model_.Clear(); + } + // In browser tests, the mock menu item doesn't store the submenu_model_ + // pointer and instead flattens the menu into a vector in AddSubmenuItem, + // which means we need to update the proxy manually. + proxy_->RemoveMenuItem(IDC_SPELLCHECK_MENU); + proxy_->AddSubMenu( + IDC_SPELLCHECK_MENU, + l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_SPELLCHECK_MENU), + &submenu_model_); + } + + // If somehow we ended up with an empty submenu then disable it. + if (!submenu_model_.GetItemCount()) + proxy_->UpdateMenuItem( + IDC_SPELLCHECK_MENU, + false, // enabled + false, // hidden + l10n_util::GetStringUTF16(IDS_CONTENT_CONTEXT_SPELLCHECK_MENU)); +} + +void BraveSpellingOptionsSubMenuObserver::SetGtestMode( + BraveSpellingOptionsSubMenuObserver::GtestMode mode) { + gtest_mode_ = mode; +} diff --git a/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h b/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h new file mode 100644 index 000000000000..e72b2c3af2df --- /dev/null +++ b/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h @@ -0,0 +1,32 @@ +/* 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_RENDERER_CONTEXT_MENU_SPELLING_OPTIONS_SUBMENU_OBSERVER_H_ +#define BRAVE_BROWSER_RENDERER_CONTEXT_MENU_SPELLING_OPTIONS_SUBMENU_OBSERVER_H_ + +#include "chrome/browser/renderer_context_menu/spelling_options_submenu_observer.h" + +// Subclass SpellingOptionsSubMenuObserver to override InitMenu so that we can +// remove extaneous separator and disable the submenu if it ends up empty. +class BraveSpellingOptionsSubMenuObserver + : public SpellingOptionsSubMenuObserver { + public: + BraveSpellingOptionsSubMenuObserver(RenderViewContextMenuProxy* proxy, + ui::SimpleMenuModel::Delegate* delegate, + int group_id); + void InitMenu(const content::ContextMenuParams& params) override; + + // The following enum and method are only used for testing. + enum GtestMode { + GTEST_MODE_DISABLED, + GTEST_MODE_NORMAL, + GTEST_MODE_EMPTY_SUBMENU, + }; + void SetGtestMode(GtestMode mode = GTEST_MODE_NORMAL); + + private: + GtestMode gtest_mode_; +}; + +#endif // BRAVE_BROWSER_RENDERER_CONTEXT_MENU_SPELLING_OPTIONS_SUBMENU_OBSERVER_H_ diff --git a/browser/renderer_context_menu/brave_spelling_options_submenu_observer_browsertest.cc b/browser/renderer_context_menu/brave_spelling_options_submenu_observer_browsertest.cc new file mode 100644 index 000000000000..1da5aa241473 --- /dev/null +++ b/browser/renderer_context_menu/brave_spelling_options_submenu_observer_browsertest.cc @@ -0,0 +1,153 @@ +/* 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/renderer_context_menu/brave_spelling_options_submenu_observer.h" + +#include + +#include "base/values.h" +#include "brave/browser/renderer_context_menu/brave_mock_render_view_context_menu.h" +#include "brave/common/pref_names.h" +#include "chrome/app/chrome_command_ids.h" +#include "chrome/browser/profiles/profile.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/common/pref_names.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "components/prefs/pref_service.h" +#include "components/spellcheck/browser/pref_names.h" +#include "content/public/common/context_menu_params.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +// A test class used in this file. This test should be a browser test because it +// accesses resources. +class BraveSpellingOptionsSubMenuObserverTest : public InProcessBrowserTest { + public: + BraveSpellingOptionsSubMenuObserverTest() {} + ~BraveSpellingOptionsSubMenuObserverTest() override {} + + void Clear() { + if (menu_) + menu_->SetObserver(nullptr); + observer_.reset(); + menu_.reset(); + } + + void Reset(bool incognito = false, + BraveSpellingOptionsSubMenuObserver::GtestMode gtest_mode = + BraveSpellingOptionsSubMenuObserver::GTEST_MODE_NORMAL) { + Clear(); + menu_.reset(new BraveMockRenderViewContextMenu( + incognito ? browser()->profile()->GetOffTheRecordProfile() + : browser()->profile())); + std::unique_ptr observer = + std::make_unique(menu_.get(), + menu_.get(), 1); + observer->SetGtestMode(gtest_mode); + observer_ = std::move(observer); + menu_->SetObserver(observer_.get()); + // Uncomment to print the menu to standard output for each test. + //menu_->EnablePrintMenu(); + } + + void SetUpOnMainThread() override { + InProcessBrowserTest::SetUpOnMainThread(); + } + + void TearDownOnMainThread() override { + Clear(); + } + + void InitMenu(bool enable_spellcheck, + const std::string& accept_languages, + const std::vector& dictionaries) { + menu()->GetPrefs()->SetBoolean(spellcheck::prefs::kSpellCheckEnable, + enable_spellcheck); + menu()->GetPrefs()->SetString(prefs::kAcceptLanguages, accept_languages); + base::ListValue dictionaries_value; + dictionaries_value.AppendStrings(dictionaries); + menu()->GetPrefs()->Set(spellcheck::prefs::kSpellCheckDictionaries, + dictionaries_value); + observer()->InitMenu(content::ContextMenuParams()); + } + + void CheckExpected() { + for (size_t i = 0; i < menu()->GetMenuSize(); ++i) { + BraveMockRenderViewContextMenu::MockMenuItem item; + menu()->GetMenuItem(i, &item); + EXPECT_NE(IDC_CONTENT_CONTEXT_SPELLING_TOGGLE, item.command_id); + } + // Check that the menu doesn't end with a separator. + BraveMockRenderViewContextMenu::MockMenuItem item; + menu()->GetMenuItem(menu()->GetMenuSize() - 1, &item); + EXPECT_NE(-1, item.command_id); // -1 == separator + } + + BraveMockRenderViewContextMenu* menu() { return menu_.get(); } + SpellingOptionsSubMenuObserver* observer() { return observer_.get(); } + + private: + std::unique_ptr menu_; + std::unique_ptr observer_; + + DISALLOW_COPY_AND_ASSIGN(BraveSpellingOptionsSubMenuObserverTest); +}; + +// Tests that "Ask Brave for suggestions" isn't shown in the menu and the menu +// doesn't end with a separator. + +IN_PROC_BROWSER_TEST_F(BraveSpellingOptionsSubMenuObserverTest, + CheckAskBraveNotShown) { + // Test with spellcheck enabled. + Reset(); + InitMenu(true, "en-US", std::vector(1, "en-US")); + menu()->PrintMenu("Test with spellcheck enabled."); + CheckExpected(); + + // Test with spellcheck disabled. + Reset(); + InitMenu(false, "en-US", std::vector(1, "en-US")); + menu()->PrintMenu("Test with spellcheck disabled."); + CheckExpected(); + + // Test with no dictionaries. + Reset(); + InitMenu(false, "en-US", std::vector()); + menu()->PrintMenu("Test with no dictionaries."); + CheckExpected(); + + // Test empty submenu. + Reset(false, BraveSpellingOptionsSubMenuObserver::GTEST_MODE_EMPTY_SUBMENU); + InitMenu(false, "en-US", std::vector()); + menu()->PrintMenu("Test empty submenu."); + EXPECT_EQ(1U, menu()->GetMenuSize()); + BraveMockRenderViewContextMenu::MockMenuItem item; + menu()->GetMenuItem(0, &item); + EXPECT_EQ(IDC_SPELLCHECK_MENU, item.command_id); + EXPECT_FALSE(item.enabled); +} + +IN_PROC_BROWSER_TEST_F(BraveSpellingOptionsSubMenuObserverTest, + CheckAskBraveNotShownIncognito) { + // Test with spellcheck enabled. + Reset(true); + InitMenu(true, "en-US", std::vector(1, "en-US")); + menu()->PrintMenu("Test incognito profile with spellcheck enabled."); + CheckExpected(); + + // Test with spellcheck disabled. + Reset(true); + InitMenu(false, "en-US", std::vector(1, "en-US")); + menu()->PrintMenu("Test incognito profile with spellcheck disabled."); + CheckExpected(); + + // Test with no dictionaries. + Reset(true); + InitMenu(false, "en-US", std::vector()); + menu()->PrintMenu("Test incognito profile with no dictionaries."); + CheckExpected(); +} + +} // namespace 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 3f966274d847..4457b9fc51d5 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 @@ -3,14 +3,27 @@ // found in the LICENSE file. #include "chrome/browser/renderer_context_menu/render_view_context_menu.h" +#include "components/spellcheck/spellcheck_buildflags.h" +#if !BUILDFLAG(USE_BROWSER_SPELLCHECKER) +#include "brave/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h" +#endif // Our .h file creates a masquerade for RenderViewContextMenu. Switch // back to the Chromium one for the Chromium implementation. #undef RenderViewContextMenu #define RenderViewContextMenu RenderViewContextMenu_Chromium +#if !defined(OS_MACOSX) +// Use our subclass to initialize SpellingOptionsSubMenuObserver. +#define SpellingOptionsSubMenuObserver BraveSpellingOptionsSubMenuObserver +#endif + #include "../../../../chrome/browser/renderer_context_menu/render_view_context_menu.cc" +#if !defined(OS_MACOSX) +#undef SpellingOptionsSubMenuObserver +#endif + // Make it clear which class we mean here. #undef RenderViewContextMenu @@ -62,3 +75,19 @@ void BraveRenderViewContextMenu::ExecuteCommand(int id, int event_flags) { RenderViewContextMenu_Chromium::ExecuteCommand(id, event_flags); } } + +void BraveRenderViewContextMenu::AddSpellCheckServiceItem(bool is_checked) { + // Call our implementation, not the one in the base class. + // Assumption: + // Use of spelling service is disabled in Brave profile preferences. + DCHECK(!GetProfile()->GetPrefs()->GetBoolean( + spellcheck::prefs::kSpellCheckUseSpellingService)); + AddSpellCheckServiceItem(&menu_model_, is_checked); +} + +// static +void BraveRenderViewContextMenu::AddSpellCheckServiceItem( + ui::SimpleMenuModel* menu, + bool is_checked) { + // Suppress adding "Spellcheck->Ask Brave for suggestions" item. +} 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 ffe438e4af3e..6b90dfe1c1e8 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 @@ -16,8 +16,13 @@ class BraveRenderViewContextMenu : public RenderViewContextMenu_Chromium { BraveRenderViewContextMenu(content::RenderFrameHost* render_frame_host, const content::ContextMenuParams& params); void AppendBraveLinkItems() override; + // RenderViewContextMenuBase: bool IsCommandIdEnabled(int command_id) const override; void ExecuteCommand(int id, int event_flags) override; + void AddSpellCheckServiceItem(bool is_checked) override; + // Hide base class implementation. + static void AddSpellCheckServiceItem(ui::SimpleMenuModel* menu, + bool is_checked); }; // Use our own subclass as the real RenderViewContextMenu. diff --git a/patches/chrome-browser-renderer_context_menu-spelling_options_submenu_observer.h.patch b/patches/chrome-browser-renderer_context_menu-spelling_options_submenu_observer.h.patch new file mode 100644 index 000000000000..abd869230119 --- /dev/null +++ b/patches/chrome-browser-renderer_context_menu-spelling_options_submenu_observer.h.patch @@ -0,0 +1,17 @@ +diff --git a/chrome/browser/renderer_context_menu/spelling_options_submenu_observer.h b/chrome/browser/renderer_context_menu/spelling_options_submenu_observer.h +index 5abff450384c93c5f28495c711ac031b851b5b20..635998cbfd8b2b5a2abd90f42924aa62a2e9ae34 100644 +--- a/chrome/browser/renderer_context_menu/spelling_options_submenu_observer.h ++++ b/chrome/browser/renderer_context_menu/spelling_options_submenu_observer.h +@@ -16,10 +16,12 @@ + #include "ui/base/models/simple_menu_model.h" + + class RenderViewContextMenuProxy; ++class BraveSpellingOptionsSubMenuObserver; + + // A class that implements the 'spell-checker options' submenu. This class + // creates the submenu, adds it to the parent menu, and handles events. + class SpellingOptionsSubMenuObserver : public RenderViewContextMenuObserver { ++ friend class BraveSpellingOptionsSubMenuObserver; + public: + SpellingOptionsSubMenuObserver(RenderViewContextMenuProxy* proxy, + ui::SimpleMenuModel::Delegate* delegate, diff --git a/test/BUILD.gn b/test/BUILD.gn index 10d487cbf794..61780eeb4c44 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -208,6 +208,9 @@ test("brave_browser_tests") { "//brave/browser/extensions/brave_tor_client_updater_browsertest.cc", "//brave/browser/extensions/api/brave_shields_api_browsertest.cc", "//brave/browser/extensions/brave_webstore_inline_installer_browsertest.cc", + "//brave/browser/renderer_context_menu/brave_mock_render_view_context_menu.cc", + "//brave/browser/renderer_context_menu/brave_mock_render_view_context_menu.h", + "//brave/browser/renderer_context_menu/brave_spelling_menu_observer_browsertest.cc", "//brave/browser/search_engine_provider_controller_browsertest.cc", "//brave/browser/ui/content_settings/brave_autoplay_blocked_image_model_browsertest.cc", "//brave/browser/ui/content_settings/brave_widevine_blocked_image_model_browsertest.cc", @@ -239,6 +242,12 @@ test("brave_browser_tests") { ] } + if (is_win || is_linux) { + sources += [ + "//brave/browser/renderer_context_menu/brave_spelling_options_submenu_observer_browsertest.cc", + ] + } + # API tests sources += [ "//brave/browser/extensions/brave_shields_apitest.cc",