Commit 3a410a84 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

WebLayer: Adjust infobar behavior for never translate lang/site

Chrome's translate infobar disappears when the user selects the
overflow menu options of never translate language/site. However, this
behavior is contingent on a snackbar then appearing informing the user
of their choice and giving them the ability to undo that choice until
the snackbar disappears (a few seconds). WebLayer does not currently
have snackbars in its infobar implementation. Hence, with the translate
UX eng owner (megjablon@) we made the decision to have the selection
of never translate lang/site *not* cause the infobar to disappear in
WebLayer. The user can then either undo their choice or manually close
the infobar as desired. This CL implements that change, including
ensure that the relevant overflow menu item is checked and that
unchecks of these menu items are properly processed.

UX changes discussed in detail here (internal-only link, apologies):

https://docs.google.com/document/d/1o6Q6vs5_MCcDNXx6mouhylhzQbfgItxbwe-KqNAbTlo/edit#heading=h.2hzrc3fbkgdf

file, which will enable the TranslateCompactInfoBar ForTesting methods
to be removed in production.

Binary-Size: Will follow up by moving JNI boundary to be on test support
Bug: 1093846
Change-Id: I79d4955e4a19d3dc3db1edfc34b63b173d4108d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247024
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarMugdha Lakhani <nator@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781207}
parent 057905c5
...@@ -490,17 +490,21 @@ public class TranslateCompactInfoBar extends InfoBar ...@@ -490,17 +490,21 @@ public class TranslateCompactInfoBar extends InfoBar
case ACTION_OVERFLOW_NEVER_LANGUAGE: case ACTION_OVERFLOW_NEVER_LANGUAGE:
case ACTION_AUTO_NEVER_LANGUAGE: case ACTION_AUTO_NEVER_LANGUAGE:
mUserInteracted = true; mUserInteracted = true;
// After applying this option, the infobar will dismiss. mOptions.toggleNeverTranslateLanguageState(
!mOptions.getTranslateState(TranslateOptions.Type.NEVER_LANGUAGE));
TranslateCompactInfoBarJni.get().applyBoolTranslateOption( TranslateCompactInfoBarJni.get().applyBoolTranslateOption(
mNativeTranslateInfoBarPtr, TranslateCompactInfoBar.this, mNativeTranslateInfoBarPtr, TranslateCompactInfoBar.this,
TranslateOption.NEVER_TRANSLATE, true); TranslateOption.NEVER_TRANSLATE,
mOptions.getTranslateState(TranslateOptions.Type.NEVER_LANGUAGE));
return; return;
case ACTION_OVERFLOW_NEVER_SITE: case ACTION_OVERFLOW_NEVER_SITE:
mUserInteracted = true; mUserInteracted = true;
// After applying this option, the infobar will dismiss. mOptions.toggleNeverTranslateDomainState(
!mOptions.getTranslateState(TranslateOptions.Type.NEVER_DOMAIN));
TranslateCompactInfoBarJni.get().applyBoolTranslateOption( TranslateCompactInfoBarJni.get().applyBoolTranslateOption(
mNativeTranslateInfoBarPtr, TranslateCompactInfoBar.this, mNativeTranslateInfoBarPtr, TranslateCompactInfoBar.this,
TranslateOption.NEVER_TRANSLATE_SITE, true); TranslateOption.NEVER_TRANSLATE_SITE,
mOptions.getTranslateState(TranslateOptions.Type.NEVER_DOMAIN));
return; return;
default: default:
assert false : "Unsupported Menu Item Id, in handle post snackbar"; assert false : "Unsupported Menu Item Id, in handle post snackbar";
...@@ -549,6 +553,18 @@ public class TranslateCompactInfoBar extends InfoBar ...@@ -549,6 +553,18 @@ public class TranslateCompactInfoBar extends InfoBar
} }
} }
@CalledByNative
// Simulates a click of the overflow menu item for "never translate this language."
private void clickNeverTranslateLanguageMenuItemForTesting() {
onOverflowMenuItemClicked(TranslateMenu.ID_OVERFLOW_NEVER_LANGUAGE);
}
@CalledByNative
// Simulates a click of the overflow menu item for "never translate this site."
private void clickNeverTranslateSiteMenuItemForTesting() {
onOverflowMenuItemClicked(TranslateMenu.ID_OVERFLOW_NEVER_SITE);
}
@NativeMethods @NativeMethods
interface Natives { interface Natives {
void applyStringTranslateOption(long nativeTranslateCompactInfoBar, void applyStringTranslateOption(long nativeTranslateCompactInfoBar,
......
...@@ -292,6 +292,12 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener { ...@@ -292,6 +292,12 @@ public class TranslateMenuHelper implements AdapterView.OnItemClickListener {
if (getItem(position).mId == TranslateMenu.ID_OVERFLOW_ALWAYS_TRANSLATE if (getItem(position).mId == TranslateMenu.ID_OVERFLOW_ALWAYS_TRANSLATE
&& mOptions.getTranslateState(TranslateOptions.Type.ALWAYS_LANGUAGE)) { && mOptions.getTranslateState(TranslateOptions.Type.ALWAYS_LANGUAGE)) {
checkboxIcon.setVisibility(View.VISIBLE); checkboxIcon.setVisibility(View.VISIBLE);
} else if (getItem(position).mId == TranslateMenu.ID_OVERFLOW_NEVER_LANGUAGE
&& mOptions.getTranslateState(TranslateOptions.Type.NEVER_LANGUAGE)) {
checkboxIcon.setVisibility(View.VISIBLE);
} else if (getItem(position).mId == TranslateMenu.ID_OVERFLOW_NEVER_SITE
&& mOptions.getTranslateState(TranslateOptions.Type.NEVER_DOMAIN)) {
checkboxIcon.setVisibility(View.VISIBLE);
} else { } else {
checkboxIcon.setVisibility(View.INVISIBLE); checkboxIcon.setVisibility(View.INVISIBLE);
} }
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "base/android/build_info.h" #include "base/android/build_info.h"
#include "components/infobars/core/infobar_manager.h" // nogncheck #include "components/infobars/core/infobar_manager.h" // nogncheck
#include "components/translate/core/browser/translate_download_manager.h"
#include "weblayer/browser/infobar_android.h" #include "weblayer/browser/infobar_android.h"
#include "weblayer/browser/infobar_service.h" #include "weblayer/browser/infobar_service.h"
#include "weblayer/browser/translate_compact_infobar.h" #include "weblayer/browser/translate_compact_infobar.h"
...@@ -167,6 +168,8 @@ class TranslateBrowserTest : public WebLayerBrowserTest { ...@@ -167,6 +168,8 @@ class TranslateBrowserTest : public WebLayerBrowserTest {
GetTranslateClient(shell()) GetTranslateClient(shell())
->GetTranslateManager() ->GetTranslateManager()
->SetIgnoreMissingKeyForTesting(true); ->SetIgnoreMissingKeyForTesting(true);
GetTranslateClient(shell())->GetTranslatePrefs()->ResetToDefaults();
} }
void TearDownOnMainThread() override { void TearDownOnMainThread() override {
...@@ -547,4 +550,226 @@ IN_PROC_BROWSER_TEST_F(TranslateBrowserTest, TranslationViaInfoBar) { ...@@ -547,4 +550,226 @@ IN_PROC_BROWSER_TEST_F(TranslateBrowserTest, TranslationViaInfoBar) {
} }
#endif #endif
#if defined(OS_ANDROID)
// Test that the translation infobar stays present when the "never translate
// language" item is clicked. Note that this behavior is intentionally different
// from that of Chrome, where the infobar is removed in this case and a snackbar
// is shown. As WebLayer has no snackbars, the UX decision was to simply leave
// the infobar open to allow the user to revert the decision if desired.
IN_PROC_BROWSER_TEST_F(TranslateBrowserTest,
TranslateInfoBarNeverTranslateLanguage) {
auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
auto* infobar_service = InfoBarService::FromWebContents(web_contents);
SetTranslateScript(kTestValidScript);
TranslateClientImpl* translate_client = GetTranslateClient(shell());
NavigateAndWaitForCompletion(GURL("about:blank"), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("und", translate_client->GetLanguageState().original_language());
TestInfoBarManagerObserver infobar_observer;
infobar_service->AddObserver(&infobar_observer);
base::RunLoop run_loop;
infobar_observer.set_on_infobar_added_callback(run_loop.QuitClosure());
// Navigate to a page in French and wait for the infobar to be added.
EXPECT_EQ(0u, infobar_service->infobar_count());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/french_page.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("fr", translate_client->GetLanguageState().original_language());
run_loop.Run();
auto* infobar =
static_cast<TranslateCompactInfoBar*>(infobar_service->infobar_at(0));
infobar->ClickOverflowMenuItemForTesting(
TranslateCompactInfoBar::OverflowMenuItemId::NEVER_TRANSLATE_LANGUAGE);
// The translate infobar should still be present.
EXPECT_EQ(1u, infobar_service->infobar_count());
// However, the infobar should not be shown on a new navigation to a page in
// French.
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/french_page2.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("fr", translate_client->GetLanguageState().original_language());
// NOTE: There is no notification to wait for for the event of the infobar not
// showing. However, in practice the infobar is added synchronously, so if it
// were to be shown, this check would fail.
EXPECT_EQ(0u, infobar_service->infobar_count());
// The infobar *should* be shown on a navigation to this site if the page's
// language is detected as something other than French.
base::RunLoop run_loop2;
infobar_observer.set_on_infobar_added_callback(run_loop2.QuitClosure());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/german_page.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("de", translate_client->GetLanguageState().original_language());
run_loop2.Run();
EXPECT_EQ(1u, infobar_service->infobar_count());
infobar_service->RemoveObserver(&infobar_observer);
}
// Test that the translation infobar stays present when the "never translate
// site" item is clicked. Note that this behavior is intentionally different
// from that of Chrome, where the infobar is removed in this case and a snackbar
// is shown. As WebLayer has no snackbars, the UX decision was to simply leave
// the infobar open to allow the user to revert the decision if desired.
IN_PROC_BROWSER_TEST_F(TranslateBrowserTest,
TranslateInfoBarNeverTranslateSite) {
auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
auto* infobar_service = InfoBarService::FromWebContents(web_contents);
SetTranslateScript(kTestValidScript);
TranslateClientImpl* translate_client = GetTranslateClient(shell());
NavigateAndWaitForCompletion(GURL("about:blank"), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("und", translate_client->GetLanguageState().original_language());
TestInfoBarManagerObserver infobar_observer;
infobar_service->AddObserver(&infobar_observer);
base::RunLoop run_loop;
infobar_observer.set_on_infobar_added_callback(run_loop.QuitClosure());
// Navigate to a page in French and wait for the infobar to be added.
EXPECT_EQ(0u, infobar_service->infobar_count());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/french_page.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("fr", translate_client->GetLanguageState().original_language());
run_loop.Run();
auto* infobar =
static_cast<TranslateCompactInfoBar*>(infobar_service->infobar_at(0));
infobar->ClickOverflowMenuItemForTesting(
TranslateCompactInfoBar::OverflowMenuItemId::NEVER_TRANSLATE_SITE);
// The translate infobar should still be present.
EXPECT_EQ(1u, infobar_service->infobar_count());
// However, the infobar should not be shown on a new navigation to this site,
// independent of the detected language.
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/french_page2.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("fr", translate_client->GetLanguageState().original_language());
// NOTE: There is no notification to wait for for the event of the infobar not
// showing. However, in practice the infobar is added synchronously, so if it
// were to be shown, this check would fail.
EXPECT_EQ(0u, infobar_service->infobar_count());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/german_page.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("de", translate_client->GetLanguageState().original_language());
EXPECT_EQ(0u, infobar_service->infobar_count());
infobar_service->RemoveObserver(&infobar_observer);
}
// Parameterized to run tests on the "never translate language" and "never
// translate site" menu items.
class NeverTranslateMenuItemTranslateBrowserTest
: public TranslateBrowserTest,
public testing::WithParamInterface<
TranslateCompactInfoBar::OverflowMenuItemId> {};
// Test that clicking and unclicking a never translate item ends up being a
// no-op.
IN_PROC_BROWSER_TEST_P(NeverTranslateMenuItemTranslateBrowserTest,
TranslateInfoBarToggleAndToggleBackNeverTranslateItem) {
auto* web_contents = static_cast<TabImpl*>(shell()->tab())->web_contents();
auto* infobar_service = InfoBarService::FromWebContents(web_contents);
SetTranslateScript(kTestValidScript);
TranslateClientImpl* translate_client = GetTranslateClient(shell());
NavigateAndWaitForCompletion(GURL("about:blank"), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("und", translate_client->GetLanguageState().original_language());
TestInfoBarManagerObserver infobar_observer;
infobar_service->AddObserver(&infobar_observer);
// Navigate to a page in French, wait for the infobar to be added, and click
// twice on the given overflow menu item.
{
base::RunLoop run_loop;
infobar_observer.set_on_infobar_added_callback(run_loop.QuitClosure());
EXPECT_EQ(0u, infobar_service->infobar_count());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/french_page.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("fr", translate_client->GetLanguageState().original_language());
run_loop.Run();
auto* infobar =
static_cast<TranslateCompactInfoBar*>(infobar_service->infobar_at(0));
infobar->ClickOverflowMenuItemForTesting(GetParam());
// The translate infobar should still be present.
EXPECT_EQ(1u, infobar_service->infobar_count());
infobar->ClickOverflowMenuItemForTesting(GetParam());
}
// The infobar should be shown on a new navigation to a page in the same
// language.
{
base::RunLoop run_loop;
infobar_observer.set_on_infobar_added_callback(run_loop.QuitClosure());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/french_page2.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("fr", translate_client->GetLanguageState().original_language());
run_loop.Run();
}
// The infobar should be shown on a new navigation to a page in a different
// language in the same site.
{
base::RunLoop run_loop;
infobar_observer.set_on_infobar_added_callback(run_loop.QuitClosure());
NavigateAndWaitForCompletion(
GURL(embedded_test_server()->GetURL("/german_page.html")), shell());
WaitUntilLanguageDetermined(shell());
EXPECT_EQ("de", translate_client->GetLanguageState().original_language());
run_loop.Run();
}
infobar_service->RemoveObserver(&infobar_observer);
}
INSTANTIATE_TEST_SUITE_P(
All,
NeverTranslateMenuItemTranslateBrowserTest,
::testing::Values(
TranslateCompactInfoBar::OverflowMenuItemId::NEVER_TRANSLATE_LANGUAGE,
TranslateCompactInfoBar::OverflowMenuItemId::NEVER_TRANSLATE_SITE));
#endif // #if defined(OS_ANDROID)
} // namespace weblayer } // namespace weblayer
...@@ -138,16 +138,15 @@ void TranslateCompactInfoBar::ApplyBoolTranslateOption( ...@@ -138,16 +138,15 @@ void TranslateCompactInfoBar::ApplyBoolTranslateOption(
delegate->ToggleAlwaysTranslate(); delegate->ToggleAlwaysTranslate();
} }
} else if (option == TranslateUtils::OPTION_NEVER_TRANSLATE) { } else if (option == TranslateUtils::OPTION_NEVER_TRANSLATE) {
if (value && delegate->IsTranslatableLanguageByPrefs()) { bool language_blocklisted = !delegate->IsTranslatableLanguageByPrefs();
if (language_blocklisted != value) {
action_flags_ |= FLAG_NEVER_LANGUAGE; action_flags_ |= FLAG_NEVER_LANGUAGE;
delegate->ToggleTranslatableLanguageByPrefs(); delegate->ToggleTranslatableLanguageByPrefs();
RemoveSelf();
} }
} else if (option == TranslateUtils::OPTION_NEVER_TRANSLATE_SITE) { } else if (option == TranslateUtils::OPTION_NEVER_TRANSLATE_SITE) {
if (value && !delegate->IsSiteBlacklisted()) { if (delegate->IsSiteBlacklisted() != value) {
action_flags_ |= FLAG_NEVER_SITE; action_flags_ |= FLAG_NEVER_SITE;
delegate->ToggleSiteBlacklist(); delegate->ToggleSiteBlacklist();
RemoveSelf();
} }
} else { } else {
DCHECK(false); DCHECK(false);
...@@ -229,4 +228,19 @@ void TranslateCompactInfoBar::SelectButtonForTesting(ActionType action_type) { ...@@ -229,4 +228,19 @@ void TranslateCompactInfoBar::SelectButtonForTesting(ActionType action_type) {
action_type); action_type);
} }
void TranslateCompactInfoBar::ClickOverflowMenuItemForTesting(
OverflowMenuItemId item_id) {
JNIEnv* env = base::android::AttachCurrentThread();
switch (item_id) {
case OverflowMenuItemId::NEVER_TRANSLATE_LANGUAGE:
Java_TranslateCompactInfoBar_clickNeverTranslateLanguageMenuItemForTesting(
env, GetJavaInfoBar());
return;
case OverflowMenuItemId::NEVER_TRANSLATE_SITE:
Java_TranslateCompactInfoBar_clickNeverTranslateSiteMenuItemForTesting(
env, GetJavaInfoBar());
return;
}
}
} // namespace weblayer } // namespace weblayer
...@@ -26,6 +26,11 @@ class TranslateCompactInfoBar ...@@ -26,6 +26,11 @@ class TranslateCompactInfoBar
std::unique_ptr<translate::TranslateInfoBarDelegate> delegate); std::unique_ptr<translate::TranslateInfoBarDelegate> delegate);
~TranslateCompactInfoBar() override; ~TranslateCompactInfoBar() override;
enum class OverflowMenuItemId {
NEVER_TRANSLATE_LANGUAGE = 0,
NEVER_TRANSLATE_SITE = 1,
};
// JNI method specific to string settings in translate. // JNI method specific to string settings in translate.
void ApplyStringTranslateOption( void ApplyStringTranslateOption(
JNIEnv* env, JNIEnv* env,
...@@ -64,6 +69,9 @@ class TranslateCompactInfoBar ...@@ -64,6 +69,9 @@ class TranslateCompactInfoBar
// |action_type|. // |action_type|.
void SelectButtonForTesting(ActionType action_type); void SelectButtonForTesting(ActionType action_type);
// Instructs the Java infobar to click the specified overflow menu item.
void ClickOverflowMenuItemForTesting(OverflowMenuItemId item_id);
private: private:
// InfoBarAndroid: // InfoBarAndroid:
base::android::ScopedJavaLocalRef<jobject> CreateRenderInfoBar( base::android::ScopedJavaLocalRef<jobject> CreateRenderInfoBar(
......
<html>
<head><title>Cette page est en Français</title></head>
<body>
Cette page a été rédigée en français. Saviez-vous que le Français est la langue officielle des jeux olympiques? Ça vous en bouche un coin, pas vrai?
</body>
</html>
<html>
<head><title>Diese Seite ist in deutsch</title></head>
<body>
Kein Witz! Dies ist eine Seite in Deutsch. Großartig, findest du nicht?
Deutsch wird in vielen Ländern gesprochen und ist eine schöne Sprache. Ich hoffe, das ist genug Text, um CLD auszulösen.
</body>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment