Skip to content

Commit

Permalink
Disable translate feature when ENABLE_BRAVE_TRANSLATE_GO flag is false
Browse files Browse the repository at this point in the history
This also removes duplicated menu separator after remove translate menu item

fixes brave/brave-browser#15714
  • Loading branch information
AlexeyBarabash committed Jun 9, 2021
1 parent 72aa287 commit d570210
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 49 deletions.
1 change: 1 addition & 0 deletions app/DEPS
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
include_rules = [
"+brave/browser",
"+brave/common",
"+brave/browser/translate/buildflags/buildflags.h",
"+brave/renderer/brave_content_renderer_client.h",
"+chrome/app",
"+chrome/browser",
Expand Down
4 changes: 4 additions & 0 deletions app/brave_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/time/time.h"
#include "brave/app/brave_command_line_helper.h"
#include "brave/browser/brave_content_browser_client.h"
#include "brave/browser/translate/buildflags/buildflags.h"
#include "brave/common/brave_switches.h"
#include "brave/common/resource_bundle_helper.h"
#include "brave/components/brave_ads/browser/buildflags/buildflags.h"
Expand Down Expand Up @@ -240,6 +241,9 @@ bool BraveMainDelegate::BasicStartupComplete(int* exit_code) {
translate::kTranslate.name,
#else
kEnableProfilePickerOnStartupFeature.name,
#if !BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
translate::kTranslate.name,
#endif
#endif
};

Expand Down
5 changes: 5 additions & 0 deletions app/brave_main_delegate_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* 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/translate/buildflags/buildflags.h"
#include "chrome/browser/domain_reliability/service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/profile_picker.h"
Expand All @@ -20,6 +21,7 @@
#include "components/omnibox/common/omnibox_features.h"
#include "components/password_manager/core/common/password_manager_features.h"
#include "components/security_state/core/features.h"
#include "components/translate/core/browser/translate_prefs.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test.h"
Expand Down Expand Up @@ -89,6 +91,9 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisabledFeatures) {
&net::features::kFirstPartySets,
&network::features::kTrustTokens,
&network_time::kNetworkTimeServiceQuerying,
#if !BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
&translate::kTranslate,
#endif
};

for (const auto* feature : disabled_features)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "brave/browser/ipfs/import/ipfs_import_controller.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/browser/renderer_context_menu/brave_spelling_options_submenu_observer.h"
#include "brave/browser/translate/buildflags/buildflags.h"
#include "brave/components/ipfs/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/grit/brave_theme_resources.h"
Expand Down Expand Up @@ -294,10 +293,8 @@ void BraveRenderViewContextMenu::BuildIPFSMenu() {
void BraveRenderViewContextMenu::InitMenu() {
RenderViewContextMenu_Chromium::InitMenu();

#if BUILDFLAG(ENABLE_TOR) || !BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
int index = -1;
#endif
#if BUILDFLAG(ENABLE_TOR)
int index = -1;
// Add Open Link with Tor
if (!TorProfileServiceFactory::IsTorDisabled() &&
!params_.link_url.is_empty()) {
Expand All @@ -319,27 +316,4 @@ void BraveRenderViewContextMenu::InitMenu() {
#if BUILDFLAG(IPFS_ENABLED)
BuildIPFSMenu();
#endif

// Only show the translate item when go-translate is enabled.
#if !BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
// Separator always precedes translate item,
// see RenderViewContextMenu::AppendPageItems.
// Remove separator because it will be duplicated after removing menu item.
RemoveSeparatorBeforeMenuItemOnPreInit(IDC_CONTENT_CONTEXT_TRANSLATE);

index = menu_model_.GetIndexOfCommandId(IDC_CONTENT_CONTEXT_TRANSLATE);
if (index != -1) {
menu_model_.RemoveItemAt(index);
}
#endif
}

void BraveRenderViewContextMenu::RemoveSeparatorBeforeMenuItemOnPreInit(
int command_id) {
// toolkit_delegate_ will be initialized after
// RenderViewContextMenuBase::Init(), but RemoveSeparatorBeforeMenuItem uses
// it, so hide and revert toolkit_delegate_ to avoid nullptr crash.
auto toolkit_delegate = std::move(toolkit_delegate_);
RemoveSeparatorBeforeMenuItem(command_id);
toolkit_delegate_ = std::move(toolkit_delegate);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ class BraveRenderViewContextMenu : public RenderViewContextMenu_Chromium {
void ExecuteIPFSCommand(int id, int event_flags);
bool IsIPFSCommandIdEnabled(int command) const;

void RemoveSeparatorBeforeMenuItemOnPreInit(int command_id);

ui::SimpleMenuModel ipfs_submenu_model_;
#endif
};
Expand Down
3 changes: 0 additions & 3 deletions chromium_src/components/renderer_context_menu/DEPS

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ if (!is_android) {
"//brave/browser/farbling:browser_tests",
"//brave/browser/permissions:browser_tests",
"//brave/browser/tor:browser_tests",
"//brave/browser/translate/buildflags:buildflags",
"//brave/browser/ui/sidebar:browser_tests",
"//brave/browser/ui/tabs/test:browser_tests",
"//brave/browser/widevine:browser_tests",
Expand Down

0 comments on commit d570210

Please sign in to comment.