Skip to content

Remove google translate install prompt #15093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 15, 2022
Merged

Remove google translate install prompt #15093

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1bc0a59
Remove Google Extension installation bubble and icon
atuchin-m Sep 14, 2022
da31808
update browser_tests
atuchin-m Sep 14, 2022
2589714
remove go-translate from flags
atuchin-m Sep 14, 2022
144d802
Remove enable_brave_translate_extension flag
atuchin-m Sep 14, 2022
7538e89
Fix brave_unit_tests
atuchin-m Sep 14, 2022
0ab2b74
update patches
atuchin-m Sep 14, 2022
41cd799
git cl format
atuchin-m Sep 14, 2022
a88dd55
Fix format issues
atuchin-m Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions app/brave_generated_resources.grd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,23 +466,6 @@ Or change later at <ph name="SETTINGS_EXTENIONS_LINK">$2<ex>brave://settings/ext
To disable this extension in private mode and Tor mode, unselect this option.
</message>

<!-- In-page Translation -->
<message name="IDS_BRAVE_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_TITLE" desc="Title text for the translate bubble when asking to translate a page.">
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This key is replaced to chromium IDS_BRAVE_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_TITLE: Translate this page?
https://github.com/brave/brave-core/blob/master/app/generated_resources.grd#L9916

It's used only as the bubble title and invisible for UI (I believe it's used in accessibility mode)

Translate this page? This will send all text on this page to Microsoft for translation.
</message>
<message name="IDS_BRAVE_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_INSTALL_TITLE" desc="Title text for the translate bubble when asking to install Google Translate.">
Install Google Translate extension to translate this page?
</message>
<message name="IDS_BRAVE_TRANSLATE_BUBBLE_INSTALL" desc="Text to show for the translate bubble's accept button to install Google Translate.">
Install
</message>
<message name="IDS_BRAVE_TRANSLATE_BUBBLE_CANCEL" desc="Text to show for the translate bubble's cancel button to dismiss the bubble.">
Cancel
</message>
<message name="IDS_BRAVE_TRANSLATE_BUBBLE_DONT_ASK_AGAIN" desc="Text to show for the don't ask again button.">
Don't ask me again
</message>

<!-- App shortcuts -->
<if expr="not is_win">
<message name="IDS_APP_SHORTCUTS_SUBDIR_NAME_BRAVE_NIGHTLY" desc="Name for the Brave Apps Start Menu folder name.">
Expand Down
7 changes: 1 addition & 6 deletions browser/about_flags.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,15 +493,10 @@ constexpr char kAllowCertainClientHintsDescription[] =

#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#define BRAVE_TRANSLATE_GO_FEATURE_ENTRIES \
{"brave-translate-go", \
flag_descriptions::kBraveTranslateGoName, \
flag_descriptions::kBraveTranslateGoDescription, \
kOsDesktop | kOsAndroid, \
FEATURE_VALUE_TYPE(translate::features::kUseBraveTranslateGo)}, \
{"translate", \
flag_descriptions::kTranslateName, \
flag_descriptions::kTranslateDescription, \
kOsAndroid, \
kOsDesktop | kOsAndroid, \
FEATURE_VALUE_TYPE(translate::kTranslate)},
#else
#define BRAVE_TRANSLATE_GO_FEATURE_ENTRIES
Expand Down
2 changes: 1 addition & 1 deletion browser/sources.gni
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ if (enable_ftx) {
brave_chrome_browser_deps += [ "//brave/browser/ftx" ]
}

if (enable_brave_translate_go || enable_brave_translate_extension) {
if (enable_brave_translate_go) {
brave_chrome_browser_sources += [
"//brave/browser/translate/brave_translate_prefs_migration.cc",
"//brave/browser/translate/brave_translate_prefs_migration.h",
Expand Down
6 changes: 2 additions & 4 deletions browser/tor/tor_profile_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
#include "components/safe_browsing/core/common/safe_browsing_prefs.h"
#include "third_party/blink/public/common/peerconnection/webrtc_ip_handling_policy.h"

#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_EXTENSION) || \
BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#include "components/translate/core/browser/translate_pref_names.h"
#endif

Expand Down Expand Up @@ -128,8 +127,7 @@ void TorProfileManager::InitTorProfileUserPrefs(Profile* profile) {
// Disable the automatic translate bubble in Tor because we currently don't
// support extensions in Tor mode and users cannot disable this through
// settings page for Tor windows.
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_EXTENSION) || \
BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
pref_service->SetBoolean(translate::prefs::kOfferTranslateEnabled, false);
#endif
}
3 changes: 1 addition & 2 deletions browser/tor/tor_profile_manager_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/peerconnection/webrtc_ip_handling_policy.h"

#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_EXTENSION) || \
BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#include "components/translate/core/browser/translate_pref_names.h"
#endif

Expand Down
73 changes: 2 additions & 71 deletions browser/translate/brave_translate_browsertest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "brave/components/l10n/common/locale_util.h"
#include "brave/components/translate/core/common/brave_translate_features.h"
#include "brave/components/translate/core/common/buildflags.h"
#include "brave/grit/brave_generated_resources.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/translate/chrome_translate_client.h"
#include "chrome/browser/translate/translate_test_utils.h"
Expand All @@ -25,6 +24,7 @@
#include "chrome/browser/ui/views/translate/translate_bubble_controller.h"
#include "chrome/browser/ui/views/translate/translate_bubble_view.h"
#include "chrome/common/chrome_isolated_world_ids.h"
#include "chrome/grit/generated_resources.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/infobars/content/content_infobar_manager.h"
Expand Down Expand Up @@ -268,7 +268,7 @@ IN_PROC_BROWSER_TEST_F(BraveTranslateBrowserTest, InternalTranslation) {
// installation).
ASSERT_EQ(bubble->GetWindowTitle(),
brave_l10n::GetLocalizedResourceUTF16String(
IDS_BRAVE_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_TITLE));
IDS_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_TITLE));

// Translate the page. Note: the event onTranslateElementLoad() is
// called from main.js (see SetupTestScriptExpectations()).
Expand Down Expand Up @@ -434,74 +434,5 @@ IN_PROC_BROWSER_TEST_F(BraveTranslateBrowserMigrationTest,

#endif // BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)

class BraveTranslateBrowserDisabledFeatureTest
: public BraveTranslateBrowserGoogleRedirectTest {
public:
BraveTranslateBrowserDisabledFeatureTest() {
scoped_feature_list_.InitAndDisableFeature(features::kUseBraveTranslateGo);
}

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

IN_PROC_BROWSER_TEST_F(BraveTranslateBrowserDisabledFeatureTest,
FeatureDisabled) {
// Set target language to FR, that is unsupported target language
// for the brave backend.
GetChromeTranslateClient()->GetTranslatePrefs()->SetRecentTargetLanguage(
"fr");

net::EmbeddedTestServer chrome_test_embedded_test_server;
chrome_test_embedded_test_server.ServeFilesFromSourceDirectory(
"chrome/test/data");
ASSERT_TRUE(chrome_test_embedded_test_server.Start());

EXPECT_CALL(backend_request_, Call(_)).Times(0);
const GURL kTestUrls[] = {
// ES is supported by the brave backend.
embedded_test_server()->GetURL("/espanol_page.html"),

// EN is unsupported but the bubble must be shown anyway.
chrome_test_embedded_test_server.GetURL("/german_page.html")};

for (const auto& url : kTestUrls) {
SCOPED_TRACE(url);

ResetObserver();
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
WaitUntilLanguageDetermined();

auto* bubble = TranslateBubbleController::FromWebContents(
browser()->tab_strip_model()->GetActiveWebContents())
->GetTranslateBubble();
ASSERT_TRUE(bubble);

// The that we see a bubble that suggests Google translate extension
// installation.
ASSERT_EQ(bubble->GetWindowTitle(),
brave_l10n::GetLocalizedResourceUTF16String(
IDS_BRAVE_TRANSLATE_BUBBLE_BEFORE_TRANSLATE_INSTALL_TITLE));

// Check the we don't download the translate scripts
base::MockCallback<TranslateScript::RequestCallback> mock_callback;
EXPECT_CALL(mock_callback, Run(false));
TranslateDownloadManager::GetInstance()->script()->Request(
mock_callback.Get(), false);

// The resulting callback must be postted immediately, so simply use
// RunUtilIdle() to wait for it.
base::RunLoop().RunUntilIdle();

// Close the bubble to avoid reusing an existing bubble.
TranslateBubbleController::FromWebContents(
browser()->tab_strip_model()->GetActiveWebContents())
->CloseBubble();

// Check no bad flags infobar is shown (about the different translate
// script/origin).
EXPECT_TRUE(HasNoBadFlagsInfobar());
}
}

} // namespace translate
6 changes: 0 additions & 6 deletions browser/translate/brave_translate_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ bool IsTranslateExtensionEnabled(content::BrowserContext* context) {
#endif // BUILDFLAG(ENABLE_EXTENSIONS)
}

bool ShouldOfferExtensionInstallation(content::BrowserContext* context) {
if (!IsTranslateExtensionAvailable())
return false;
return !IsTranslateExtensionEnabled(context);
}

bool IsInternalTranslationEnabled(content::BrowserContext* context) {
return !IsTranslateExtensionEnabled(context) && IsBraveTranslateGoAvailable();
}
Expand Down
2 changes: 0 additions & 2 deletions browser/translate/brave_translate_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ namespace translate {

bool IsTranslateExtensionEnabled(content::BrowserContext* context);

bool ShouldOfferExtensionInstallation(content::BrowserContext* context);

bool IsInternalTranslationEnabled(content::BrowserContext* context);

} // namespace translate
Expand Down
13 changes: 0 additions & 13 deletions browser/ui/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,6 @@ source_set("ui") {
]
}

if (enable_brave_translate_extension) {
sources += [
"views/translate/brave_translate_bubble_view.cc",
"views/translate/brave_translate_bubble_view.h",
"views/translate/brave_translate_icon_view.cc",
"views/translate/brave_translate_icon_view.h",
]
}

if (is_win) {
sources += [
"views/frame/brave_glass_browser_frame_view.cc",
Expand Down Expand Up @@ -713,10 +704,6 @@ source_set("ui") {
"//ui/events",
"//ui/views",
]

if (enable_brave_translate_extension) {
deps += [ "//components/translate/core/browser" ]
}
}
}

Expand Down
22 changes: 7 additions & 15 deletions browser/ui/views/frame/brave_browser_view.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@
#include "chrome/browser/ui/views/location_bar/location_bar_bubble_delegate_view.h"
#endif

#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_EXTENSION) || \
BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#include "brave/browser/translate/brave_translate_utils.h"
#endif

Expand Down Expand Up @@ -288,30 +287,23 @@ void BraveBrowserView::ShowUpdateChromeDialog() {
#endif
}

// The translate bubble will be shown if ENABLE_BRAVE_TRANSLATE_GO or
// ENABLE_BRAVE_TRANSLATE_EXTENSIONS build flag is enabled and Google Translate
// is not installed. In ENABLE_BRAVE_TRANSLATE case, we utilize chromium's
// translate UI directly along with go-translate. In
// ENABLE_BRAVE_TRANSLATE_EXTENSION case, we repurpose the translate bubble to
// offer Google Translate extension installation, and the bubble will only be
// shown when Google Translate is not installed.
// The translate bubble will be shown if ENABLE_BRAVE_TRANSLATE_GO build flag
// is enabled. We utilize chromium's translate UI directly along with
// go-translate.
ShowTranslateBubbleResult BraveBrowserView::ShowTranslateBubble(
content::WebContents* web_contents,
translate::TranslateStep step,
const std::string& source_language,
const std::string& target_language,
translate::TranslateErrors::Type error_type,
bool is_user_gesture) {
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_EXTENSION) || \
BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
if (translate::ShouldOfferExtensionInstallation(GetProfile()) ||
translate::IsInternalTranslationEnabled(GetProfile())) {
#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
if (translate::IsInternalTranslationEnabled(GetProfile())) {
return BrowserView::ShowTranslateBubble(web_contents, step, source_language,
target_language, error_type,
is_user_gesture);
}
#endif // BUILDFLAG(ENABLE_BRAVE_TRANSLATE_EXTENSION) ||
// BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
#endif // BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
return ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_VALID;
}

Expand Down
Loading