New issue
Advanced search Search tips

Issue 634136 link

Starred by 2 users
Status: Started
Owner:
donnd@chromium.org
Cc:
marq@chromium.org
twelling...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Feature

Blocking:
issue 642897



Sign in to add a comment

[TTS] Add basic "searchyness" for Tap Suppression

Project Member Reported by donnd@chromium.org, Aug 3 2016 
We want to gauge the "searchyness" of the text tapped in order to better decide whether to trigger or not.  

The first step in doing this is to gather the text surrounding the point where a Tap gesture occurs so it can be examined before doing any UX (such as selecting the word).  Pedro's refactoring CL does this, so I'm thinking of importing that part of the refactoring into the mainstream code.

Once we have access to the tap-text we can use it to build a rudimentary searchyness value, probably based on the length of the word tapped and whether it's capitalized.

See go/cs-suppression for background on Tap Suppression.

CC'ing marq@ so we can try to coordinate an iOS implementation.

Comment 1 by donnd@chromium.org, Aug 17 2016 

We also need to build a signal mixer to be able to mix various signals in order to get a searchyness result, or more likely to get a final suppress outcome from various searchyness signals.  There's an design now at go/cs-suppression.

Comment 2 by twelling...@chromium.org, Aug 31 2016

Blocking: 642897
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f11e42880b1b7b6608e2d57fa538552318367ae

commit 3f11e42880b1b7b6608e2d57fa538552318367ae
Author: donnd <donnd@chromium.org>
Date: Wed Sep 14 02:55:27 2016

[TTS] Gather surrounding text on Tap before any UX.

Extract the text tapped on to use as a signal in Tap Suppression.
The text is extracted before any UX is displayed in order to allow the
tap to be totally ignored when appropriate.  Feeding the surrounding
text into the logic of TTS will be done separately.

This adds several files that are part of the 2016-refactoring.
See crbug.com/624609 and go/cs-refactoring-2016.

This CL is part of the refactoring-2016 effort, see go/cs-refactoring-2016
for details.

BUG=634136, 624609

Review-Url: https://codereview.chromium.org/2211353002
Cr-Commit-Position: refs/heads/master@{#418464}

[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/ResolvedSearchAction.java
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchAction.java
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchActionListener.java
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/gesture/SearchGestureHost.java
[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/java_sources.gni
[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/BUILD.gn
[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/android/contextualsearch/contextual_search_context.cc
[modify] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/android/contextualsearch/contextual_search_context.h
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/android/contextualsearch/search_action.cc
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/android/contextualsearch/search_action.h
[add] https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae/chrome/browser/android/contextualsearch/search_action_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80

commit 1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80
Author: jbroman <jbroman@chromium.org>
Date: Thu Sep 15 15:39:39 2016

Revert of [TTS] Gather surrounding text on Tap before any UX. (patchset #13 id:240001 of https://codereview.chromium.org/2211353002/ )

Reason for revert:
Suspected of causing try flakes in ContextualSearchManagerTest#testPromoTapCount:

https://bugs.chromium.org/p/chromium/issues/detail?id=647210

Original issue's description:
> [TTS] Gather surrounding text on Tap before any UX.
>
> Extract the text tapped on to use as a signal in Tap Suppression.
> The text is extracted before any UX is displayed in order to allow the
> tap to be totally ignored when appropriate.  Feeding the surrounding
> text into the logic of TTS will be done separately.
>
> This adds several files that are part of the 2016-refactoring.
> See crbug.com/624609 and go/cs-refactoring-2016.
>
> This CL is part of the refactoring-2016 effort, see go/cs-refactoring-2016
> for details.
>
> BUG=634136, 624609
>
> Committed: https://crrev.com/3f11e42880b1b7b6608e2d57fa538552318367ae
> Cr-Commit-Position: refs/heads/master@{#418464}

TBR=twellington@chromium.org,pedrosimonetti@chromium.org,tedchoc@chromium.org,donnd@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=634136, 624609

Review-Url: https://codereview.chromium.org/2348443002
Cr-Commit-Position: refs/heads/master@{#418873}

[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/ResolvedSearchAction.java
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchAction.java
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/action/SearchActionListener.java
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/gesture/SearchGestureHost.java
[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/android/java_sources.gni
[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/browser/BUILD.gn
[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/browser/android/chrome_jni_registrar.cc
[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/browser/android/contextualsearch/contextual_search_context.cc
[modify] https://crrev.com/1dbb5e84a2682cdbc17093d9e1f5d1c40de46e80/chrome/browser/android/contextualsearch/contextual_search_context.h
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/browser/android/contextualsearch/search_action.cc
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/browser/android/contextualsearch/search_action.h
[delete] https://crrev.com/925b8063d1de2006147e1834894b82767ef227cd/chrome/browser/android/contextualsearch/search_action_unittest.cc

Comment 5 by donnd@chromium.org, Sep 20 2016 

FYI I'll need to fix issue 647128 before relanding the change in #3.

Comment 6 by donnd@chromium.org, Sep 26 2016 

Will need to check issue 650087 also.
►

Sign in to add a comment