Commit 8e976c6f authored by donnd's avatar donnd Committed by Commit bot

[Contextual Search] Trigger translation on long-press.

Adds translation one-box triggering when the user selects
using long-press.  This currently requires server-side
auto-detection of the source language, but it should not
over-trigger due to a feature of the one-box (suppressed
when the source and target text ends up matching).

Adds a separate flag to control disabling this feature,
currently default-enabled.

BUG=413717

Review URL: https://codereview.chromium.org/1463583004

Cr-Commit-Position: refs/heads/master@{#361231}
parent c2259413
......@@ -33,12 +33,16 @@ public class ContextualSearchFieldTrial {
// Translation.
@VisibleForTesting
static final String DISABLE_FORCE_TRANSLATION_ONEBOX = "disable_force_translation_onebox";
@VisibleForTesting
static final String DISABLE_AUTO_DETECT_TRANSLATION_ONEBOX =
"disable_auto_detect_translation_onebox";
// Cached values to avoid repeated and redundant JNI operations.
private static Boolean sEnabled;
private static Boolean sIsPeekPromoEnabled;
private static Integer sPeekPromoMaxCount;
private static Boolean sDisableForceTranslationOnebox;
private static Boolean sDisableAutoDetectTranslationOnebox;
/**
* Don't instantiate.
......@@ -139,6 +143,18 @@ public class ContextualSearchFieldTrial {
return sDisableForceTranslationOnebox.booleanValue();
}
/**
* @return Whether forcing a translation Onebox based on auto-detection of the source language
* is disabled.
*/
static boolean disableAutoDetectTranslationOnebox() {
if (sDisableAutoDetectTranslationOnebox == null) {
sDisableAutoDetectTranslationOnebox = getBooleanParam(
DISABLE_AUTO_DETECT_TRANSLATION_ONEBOX);
}
return sDisableAutoDetectTranslationOnebox.booleanValue();
}
// --------------------------------------------------------------------------------------------
// Helpers.
// --------------------------------------------------------------------------------------------
......
......@@ -492,7 +492,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
boolean shouldPrefetch = mPolicy.shouldPrefetchSearchResult(isTap);
mSearchRequest = new ContextualSearchRequest(mSelectionController.getSelectedText(),
null, shouldPrefetch);
// TODO(donnd): figure out a way to do translation on long-press selections.
forceAutoDetectTranslateUnlessDisabled(mSearchRequest);
mDidStartLoadingResolvedSearchRequest = false;
mSearchPanel.displaySearchTerm(mSelectionController.getSelectedText());
if (shouldPrefetch) loadSearchUrl();
......@@ -738,16 +738,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
boolean shouldPreload = !doPreventPreload && mPolicy.shouldPrefetchSearchResult(true);
mSearchRequest = new ContextualSearchRequest(searchTerm, alternateTerm, shouldPreload);
// Trigger translation, if enabled.
if (!contextLanguage.isEmpty()) {
if (mPolicy.needsTranslation(contextLanguage, getReadableLanguages())) {
boolean doForceTranslate = !mPolicy.disableForceTranslationOnebox();
if (doForceTranslate) {
mSearchRequest.forceTranslation(contextLanguage,
mPolicy.bestTargetLanguage(getWritableLanguages()));
}
ContextualSearchUma.logTranslateOnebox(doForceTranslate);
}
}
forceTranslateIfNeeded(mSearchRequest, contextLanguage);
mDidStartLoadingResolvedSearchRequest = false;
if (mSearchPanel.isContentShowing()) {
mSearchRequest.setNormalPriority();
......@@ -797,6 +788,7 @@ public class ContextualSearchManager extends ContextualSearchObservable
// ============================================================================================
// Translation support
// TODO(donnd): move to a separate file.
// ============================================================================================
/**
......@@ -871,6 +863,48 @@ public class ContextualSearchManager extends ContextualSearchObservable
return new Locale(trimmedLocale).getLanguage();
}
/**
* Force translation from the given language for the current search request,
* unless disabled by experiment. Also log whenever conditions are right to translate.
* @param searchRequest The search request to force translation upon.
* @param sourceLanguage The language to translate from, or an empty string if not known.
*/
private void forceTranslateIfNeeded(ContextualSearchRequest searchRequest,
String sourceLanguage) {
if (!TextUtils.isEmpty(sourceLanguage)) {
if (mPolicy.needsTranslation(sourceLanguage, getReadableLanguages())) {
boolean doForceTranslate = !mPolicy.disableForceTranslationOnebox();
if (doForceTranslate && searchRequest != null) {
searchRequest.forceTranslation(sourceLanguage,
mPolicy.bestTargetLanguage(getWritableLanguages()));
}
// Log that conditions were right for translation, even though it may be disabled
// for an experiment so we can compare with the counter factual data.
ContextualSearchUma.logTranslateOnebox(doForceTranslate);
}
}
}
/**
* Force auto-detect translation for the current search request unless disabled by experiment.
* Also log that conditions are right to translate.
* @param searchRequest The search request to force translation upon.
*/
private void forceAutoDetectTranslateUnlessDisabled(ContextualSearchRequest searchRequest) {
// Always trigger translation using auto-detect when we're not resolving,
// unless disabled by policy.
boolean shouldAutoDetectTranslate = !mPolicy.disableAutoDetectTranslationOnebox();
if (shouldAutoDetectTranslate && searchRequest != null) {
// The translation one-box won't actually show when the source text ends up being
// the same as the target text, so we err on over-triggering.
searchRequest.forceAutoDetectTranslation(
mPolicy.bestTargetLanguage(getWritableLanguages()));
}
// Log that conditions were right for translation, even though it may be disabled
// for an experiment so we can compare with the counter factual data.
ContextualSearchUma.logTranslateOnebox(shouldAutoDetectTranslate);
}
// ============================================================================================
// OverlayContentDelegate
// ============================================================================================
......
......@@ -471,6 +471,16 @@ class ContextualSearchPolicy {
return ContextualSearchFieldTrial.disableForceTranslationOnebox();
}
/**
* @return Whether forcing a translation Onebox based on auto-detection of the source language
* is disabled.
*/
boolean disableAutoDetectTranslationOnebox() {
if (disableForceTranslationOnebox()) return true;
return ContextualSearchFieldTrial.disableAutoDetectTranslationOnebox();
}
/**
* Sets the limit for the tap triggered promo.
*/
......
......@@ -160,6 +160,15 @@ class ContextualSearchRequest {
mNormalPriorityUri = makeTranslateUri(mNormalPriorityUri, sourceLanguage, targetLanguage);
}
/**
* Adds translation parameters that will trigger auto-detection of the source language.
* @param targetLanguage The language the that the user prefers.
*/
void forceAutoDetectTranslation(String targetLanguage) {
// Use the empty string for the source language in order to trigger auto-detect.
forceTranslation("", targetLanguage);
}
/**
* @return Whether translation was forced for this request.
*/
......
......@@ -2585,4 +2585,51 @@ public class ContextualSearchManagerTest extends ChromeActivityTestCaseBase<Chro
// Make sure we did not try to trigger translate.
assertFalse(mManager.getRequest().isTranslationForced());
}
/**
* Tests that a long-press does trigger translation.
*/
@SmallTest
@Feature({"ContextualSearch"})
@Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
public void testLongpressTranslates() throws InterruptedException, TimeoutException {
// LongPress on any word should trigger translation.
simulateLongPressSearch("search");
// Make sure we did try to trigger translate.
assertTrue(mManager.getRequest().isTranslationForced());
}
/**
* Tests that a long-press does trigger translation.
*/
@SmallTest
@Feature({"ContextualSearch"})
@Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
@CommandLineFlags.Add(ContextualSearchFieldTrial.DISABLE_AUTO_DETECT_TRANSLATION_ONEBOX
+ "=true")
public void testLongpressAutoDetectDisabledDoesNotTranslate()
throws InterruptedException, TimeoutException {
// Unless disabled, LongPress on any word should trigger translation.
simulateLongPressSearch("search");
// Make sure we did not try to trigger translate.
assertFalse(mManager.getRequest().isTranslationForced());
}
/**
* Tests that a long-press does trigger translation.
*/
@SmallTest
@Feature({"ContextualSearch"})
@Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
@CommandLineFlags.Add(ContextualSearchFieldTrial.DISABLE_FORCE_TRANSLATION_ONEBOX + "=true")
public void testLongpressTranslateDisabledDoesNotTranslate()
throws InterruptedException, TimeoutException {
// Unless disabled, LongPress on any word should trigger translation.
simulateLongPressSearch("search");
// Make sure we did not try to trigger translate.
assertFalse(mManager.getRequest().isTranslationForced());
}
}
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