Commit 1dc33577 authored by Donn Denman's avatar Donn Denman Committed by Commit Bot

[TTS] Fix some bugs in longpress activation.

Adds another option to the Longpress Activation Chrome Flag to
preserve Tap activation.  Without it, the Tap gesture will no
longer trigger Contextual Search.

Also fix some minor bugs:
1) preserve the longpress selection even after the panel is closed.
2) fix a crash with Tap after a longpress selection triggered.
3) tap with existing longpress selection should not select.

BUG=956277, 959040

Change-Id: I3c351a0227b55c996b645cb76e08976ce36a61e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764187Reviewed-by: default avatarJinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Donn Denman <donnd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689994}
parent e1c87368
......@@ -27,6 +27,7 @@ public class ContextualSearchFieldTrial {
public static final String LONGPRESS_RESOLVE_PARAM_NAME = "longpress_resolve_variation";
public static final String LONGPRESS_RESOLVE_HIDE_ON_SCROLL = "1";
public static final String LONGPRESS_RESOLVE_PRIVACY_AGGRESSIVE = "2";
public static final String LONGPRESS_RESOLVE_PRESERVE_TAP = "3";
//==========================================================================================
private static final String FIELD_TRIAL_NAME = "ContextualSearch";
......
......@@ -1611,7 +1611,9 @@ public class ContextualSearchManager
@Override
public void tapGestureCommit() {
mInternalStateController.notifyStartingWorkOn(InternalState.TAP_GESTURE_COMMIT);
if (!mPolicy.isTapSupported()) {
if (!mPolicy.isTapSupported()
|| mSelectionController.getSelectionType()
== SelectionType.RESOLVING_LONG_PRESS) {
hideContextualSearch(StateChangeReason.UNKNOWN);
return;
}
......@@ -1715,10 +1717,12 @@ public class ContextualSearchManager
@Override
public void showContextualSearchResolvingUi() {
mInternalStateController.notifyStartingWorkOn(InternalState.SHOW_RESOLVING_UI);
boolean isTap = mSelectionController.getSelectionType() == SelectionType.TAP;
showContextualSearch(isTap ? StateChangeReason.TEXT_SELECT_TAP
: StateChangeReason.TEXT_SELECT_LONG_PRESS);
if (isTap) ContextualSearchUma.logRankerFeaturesAvailable(true);
if (mSelectionController.getSelectionType() != SelectionType.UNDETERMINED) {
boolean isTap = mSelectionController.getSelectionType() == SelectionType.TAP;
showContextualSearch(isTap ? StateChangeReason.TEXT_SELECT_TAP
: StateChangeReason.TEXT_SELECT_LONG_PRESS);
if (isTap) ContextualSearchUma.logRankerFeaturesAvailable(true);
}
mInternalStateController.notifyFinishedWorkOn(InternalState.SHOW_RESOLVING_UI);
}
......
......@@ -120,7 +120,7 @@ class ContextualSearchPolicy {
|| ContextualSearchFieldTrial.getSwitch(
ContextualSearchSwitch
.IS_CONTEXTUAL_SEARCH_TAP_DISABLE_OVERRIDE_ENABLED))
? true
? !isTapDisabledDueToLongpress()
: (getPromoTapsRemaining() != 0);
}
......@@ -284,6 +284,15 @@ class ContextualSearchPolicy {
ContextualSearchFieldTrial.LONGPRESS_RESOLVE_PARAM_NAME));
}
/** @return whether Tap is disabled due to the longpress experiment. */
private boolean isTapDisabledDueToLongpress() {
return ChromeFeatureList.isEnabled(ChromeFeatureList.CONTEXTUAL_SEARCH_LONGPRESS_RESOLVE)
&& !ContextualSearchFieldTrial.LONGPRESS_RESOLVE_PRESERVE_TAP.equals(
ChromeFeatureList.getFieldTrialParamByFeature(
ChromeFeatureList.CONTEXTUAL_SEARCH_LONGPRESS_RESOLVE,
ContextualSearchFieldTrial.LONGPRESS_RESOLVE_PARAM_NAME));
}
/**
* Determines whether an error from a search term resolution request should
* be shown to the user, or not.
......
......@@ -80,6 +80,7 @@ import org.chromium.chrome.test.util.ChromeTabUtils;
import org.chromium.chrome.test.util.FullscreenTestUtils;
import org.chromium.chrome.test.util.MenuUtils;
import org.chromium.chrome.test.util.OmniboxTestUtils;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.navigation_interception.NavigationParams;
import org.chromium.content_public.browser.SelectionClient;
import org.chromium.content_public.browser.SelectionPopupController;
......@@ -1535,6 +1536,7 @@ public class ContextualSearchManagerTest {
@Test
@SmallTest
@Feature({"ContextualSearch"})
@Features.DisableFeatures({ChromeFeatureList.CONTEXTUAL_SEARCH_LONGPRESS_RESOLVE})
public void testLongPressGestureFollowedByScrollMaintainsSelection()
throws InterruptedException, TimeoutException {
longPressNode("intelligence");
......@@ -2220,6 +2222,7 @@ public class ContextualSearchManagerTest {
@Test
@SmallTest
@Feature({"ContextualSearch"})
@Features.DisableFeatures({ChromeFeatureList.CONTEXTUAL_SEARCH_LONGPRESS_RESOLVE})
public void testPreventHandlingCurrentSelectionModification()
throws InterruptedException, TimeoutException {
simulateLongPressSearch("search");
......
......@@ -1079,10 +1079,14 @@ const FeatureEntry::FeatureParam kLongpressResolveHideOnScroll = {
const FeatureEntry::FeatureParam kLongpressResolvePrivacyAggressive = {
contextual_search::kLongpressResolveParamName,
contextual_search::kLongpressResolvePrivacyAggressive};
const FeatureEntry::FeatureParam kLongpressResolvePreserveTap = {
contextual_search::kLongpressResolveParamName,
contextual_search::kLongpressResolvePreserveTap};
const FeatureEntry::FeatureVariation kLongpressResolveVariations[] = {
{"and hide on scroll", &kLongpressResolveHideOnScroll, 1, nullptr},
{"and allow privacy-aggressive behavior",
&kLongpressResolvePrivacyAggressive, 1, nullptr},
{"and preserve Tap behavior", &kLongpressResolvePreserveTap, 1, nullptr},
};
#endif // defined(OS_ANDROID)
......
......@@ -12,6 +12,7 @@ const char kContextualSearchFieldTrialName[] = "ContextualSearch";
const char kLongpressResolveParamName[] = "longpress_resolve_variation";
const char kLongpressResolveHideOnScroll[] = "1";
const char kLongpressResolvePrivacyAggressive[] = "2";
const char kLongpressResolvePreserveTap[] = "3";
// Contextual Cards variations and integration Api settings.
const char kContextualCardsVersionParamName[] = "contextual_cards_version";
......
......@@ -42,6 +42,7 @@ extern const char kContextualCardsSimplifiedServerWithDiagnosticChar[];
extern const char kLongpressResolveParamName[];
extern const char kLongpressResolveHideOnScroll[];
extern const char kLongpressResolvePrivacyAggressive[];
extern const char kLongpressResolvePreserveTap[];
} // namespace contextual_search
......
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