Commit e1a47782 authored by Anthony Cui's avatar Anthony Cui Committed by Chromium LUCI CQ

Allow translate from menu when page is translated to display UI

On desktop, if the page has already been translated then the manual translate option in the context menu is disabled. Instead, keep the option enabled, and when selected re-surface the translate bubble UI without triggering a new translation.
On Android, the menu option is enabled but does not perform any function. Instead, follow the same strategy and re-surface the UI without a new translation.

Bug: 1024514
Change-Id: Ide42c3deead6ca5839ff03daff1c790084759a4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2622383Reviewed-by: default avatarMegan Jablonski <megjablon@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Anthony Cui <cuianthony@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843674}
parent bb5f17cf
......@@ -2634,7 +2634,6 @@ bool RenderViewContextMenu::IsTranslateEnabled() const {
return ((params_.edit_flags & ContextMenuDataEditFlags::kCanTranslate) !=
0) &&
!original_lang.empty() && // Did we receive the page language yet?
!chrome_translate_client->GetLanguageState().IsPageTranslated() &&
// There are some application locales which can't be used as a
// target language for translation. In that case GetTargetLanguage()
// may return empty.
......
......@@ -258,8 +258,8 @@ bool ChromeTranslateClient::ShowTranslateUI(
return false;
}
ShowTranslateBubbleResult result =
ShowBubble(step, source_language, target_language, error_type);
ShowTranslateBubbleResult result = ShowBubble(
step, source_language, target_language, error_type, triggered_from_menu);
if (result != ShowTranslateBubbleResult::SUCCESS &&
step == translate::TRANSLATE_STEP_BEFORE_TRANSLATE) {
translate_manager_->RecordTranslateEvent(
......@@ -390,7 +390,8 @@ ShowTranslateBubbleResult ChromeTranslateClient::ShowBubble(
translate::TranslateStep step,
const std::string& source_language,
const std::string& target_language,
translate::TranslateErrors::Type error_type) {
translate::TranslateErrors::Type error_type,
bool is_user_gesture) {
DCHECK(translate_manager_);
Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
......@@ -399,7 +400,7 @@ ShowTranslateBubbleResult ChromeTranslateClient::ShowBubble(
if (!browser) {
return TranslateBubbleFactory::Show(NULL, web_contents(), step,
source_language, target_language,
error_type);
error_type, is_user_gesture);
}
if (web_contents() != browser->tab_strip_model()->GetActiveWebContents())
......@@ -414,15 +415,15 @@ ShowTranslateBubbleResult ChromeTranslateClient::ShowBubble(
return ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_ACTIVE;
// During auto-translating, the bubble should not be shown.
if (step == translate::TRANSLATE_STEP_TRANSLATING ||
step == translate::TRANSLATE_STEP_AFTER_TRANSLATE) {
if (!is_user_gesture && (step == translate::TRANSLATE_STEP_TRANSLATING ||
step == translate::TRANSLATE_STEP_AFTER_TRANSLATE)) {
if (GetLanguageState().InTranslateNavigation())
return ShowTranslateBubbleResult::SUCCESS;
}
return TranslateBubbleFactory::Show(browser->window(), web_contents(), step,
source_language, target_language,
error_type);
error_type, is_user_gesture);
}
#endif
......
......@@ -136,7 +136,8 @@ class ChromeTranslateClient
translate::TranslateStep step,
const std::string& source_language,
const std::string& target_language,
translate::TranslateErrors::Type error_type);
translate::TranslateErrors::Type error_type,
bool is_user_gesture);
#endif
std::unique_ptr<translate::ContentTranslateDriver> translate_driver_;
......
......@@ -8,6 +8,7 @@
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "components/translate/core/browser/translate_step.h"
namespace {
......@@ -17,12 +18,14 @@ ShowTranslateBubbleResult ShowDefault(
translate::TranslateStep step,
const std::string& source_language,
const std::string& target_language,
translate::TranslateErrors::Type error_type) {
translate::TranslateErrors::Type error_type,
bool is_user_gesture) {
// |window| might be null when testing.
if (!window)
return ShowTranslateBubbleResult::BROWSER_WINDOW_NOT_VALID;
return window->ShowTranslateBubble(web_contents, step, source_language,
target_language, error_type, false);
target_language, error_type,
is_user_gesture);
}
} // namespace
......@@ -37,7 +40,8 @@ ShowTranslateBubbleResult TranslateBubbleFactory::Show(
translate::TranslateStep step,
const std::string& source_language,
const std::string& target_language,
translate::TranslateErrors::Type error_type) {
translate::TranslateErrors::Type error_type,
bool is_user_gesture) {
if (current_factory_) {
return current_factory_->ShowImplementation(window, web_contents, step,
source_language,
......@@ -45,7 +49,7 @@ ShowTranslateBubbleResult TranslateBubbleFactory::Show(
}
return ShowDefault(window, web_contents, step, source_language,
target_language, error_type);
target_language, error_type, is_user_gesture);
}
// static
......
......@@ -31,7 +31,8 @@ class TranslateBubbleFactory {
translate::TranslateStep step,
const std::string& source_language,
const std::string& target_language,
translate::TranslateErrors::Type error_type);
translate::TranslateErrors::Type error_type,
bool is_user_gesture);
// Sets the factory to change the behavior how to show the bubble.
// TranslateBubbleFactory doesn't take the ownership of |factory|.
......
......@@ -312,10 +312,10 @@ bool TranslateManager::CanManuallyTranslate(bool menuLogging) {
void TranslateManager::InitiateManualTranslation(bool auto_translate,
bool triggered_from_menu) {
// If a translation has already been triggered, do nothing.
if (language_state_.IsPageTranslated() ||
language_state_.translation_pending())
// If a translation is in progress, do nothing.
if (language_state_.translation_pending()) {
return;
}
std::unique_ptr<TranslatePrefs> translate_prefs(
translate_client_->GetTranslatePrefs());
......@@ -326,20 +326,19 @@ void TranslateManager::InitiateManualTranslation(bool auto_translate,
language_state_.SetTranslateEnabled(true);
const TranslateStep step = language_state_.IsPageTranslated()
? TRANSLATE_STEP_AFTER_TRANSLATE
: TRANSLATE_STEP_BEFORE_TRANSLATE;
// Translate the page if it has not been translated and manual translate
// should trigger translation automatically. Otherwise, only show the infobar.
if (auto_translate) {
if (auto_translate && !language_state_.IsPageTranslated()) {
TranslatePage(source_code, target_lang, triggered_from_menu);
return;
}
const translate::TranslateStep step =
language_state_.IsPageTranslated()
? translate::TRANSLATE_STEP_AFTER_TRANSLATE
: translate::TRANSLATE_STEP_BEFORE_TRANSLATE;
translate_client_->ShowTranslateUI(step, source_code, target_lang,
TranslateErrors::NONE,
true /* triggered_by_menu */);
triggered_from_menu);
}
void TranslateManager::TranslatePage(const std::string& original_source_lang,
......
......@@ -1195,9 +1195,7 @@ TEST_F(TranslateManagerTest, InitiateManualTranslation) {
TranslateAcceptLanguages accept_langugages(&prefs_, accept_languages_prefs);
ON_CALL(mock_translate_client_, GetTranslateAcceptLanguages())
.WillByDefault(Return(&accept_langugages));
EXPECT_CALL(mock_translate_client_,
ShowTranslateUI(_, _, _, _, true /* triggered_from_menu */))
.WillOnce(Return(true));
EXPECT_CALL(mock_translate_client_, ShowTranslateUI).WillOnce(Return(true));
translate_manager_ = std::make_unique<translate::TranslateManager>(
&mock_translate_client_, &mock_translate_ranker_, &mock_language_model_);
......
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