Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(115)

Issue 2863573004: Explicitly tell Smart Select whether or not to suggest (Closed)

Created:
3 years, 7 months ago by amaralp
Modified:
3 years, 7 months ago
CC:
agrieve+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, donnd+watch_chromium.org, jam, twellington+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Explicitly tell Smart Select whether or not to suggest This is a piece of the larger CL crrev.com/2785853002. Since the goal is to have the select action mode be triggered by contextmenu events instead of selection changes we need to have Smart Selection be triggered explicitly instead of through selection events. BUG=627234 Review-Url: https://codereview.chromium.org/2863573004 Cr-Commit-Position: refs/heads/master@{#469887} Committed: https://chromium.googlesource.com/chromium/src/+/3ba7b83974443ba8324d704117743c7ae463d4ed

Patch Set 1 #

Patch Set 2 : Make cancel requests explicit #

Total comments: 8

Patch Set 3 : fixing nits #

Messages

Total messages: 27 (18 generated)
amaralp
PTAL boliu@ for content stuff donnd@ for ContextualSearchManager.java timav@ for Smart Selection
3 years, 7 months ago (2017-05-05 01:06:14 UTC) #9
Donn Denman
LGTM for Contextual Search. https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/SelectionClient.java File content/public/android/java/src/org/chromium/content/browser/SelectionClient.java (right): https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/SelectionClient.java#newcode36 content/public/android/java/src/org/chromium/content/browser/SelectionClient.java:36: * @param shouldSuggest Whether SelectionClient ...
3 years, 7 months ago (2017-05-05 01:23:24 UTC) #12
Tima Vaisburd
https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java File content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java (right): https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java#newcode930 content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java:930: if (mSelectionClient == null Nice, I think another option ...
3 years, 7 months ago (2017-05-05 01:57:07 UTC) #13
boliu
lgtm \o/ https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java File content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java (right): https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java#newcode24 content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java:24: public class ContextSelectionClient implements SelectionClient { TODO ...
3 years, 7 months ago (2017-05-05 02:11:50 UTC) #14
Tima Vaisburd
https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java File content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java (right): https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java#newcode24 content/public/android/java/src/org/chromium/content/browser/ContextSelectionClient.java:24: public class ContextSelectionClient implements SelectionClient { On 2017/05/05 02:11:50, ...
3 years, 7 months ago (2017-05-05 16:54:41 UTC) #15
amaralp
https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/SelectionClient.java File content/public/android/java/src/org/chromium/content/browser/SelectionClient.java (right): https://codereview.chromium.org/2863573004/diff/20001/content/public/android/java/src/org/chromium/content/browser/SelectionClient.java#newcode36 content/public/android/java/src/org/chromium/content/browser/SelectionClient.java:36: * @param shouldSuggest Whether SelectionClient should suggest and classify ...
3 years, 7 months ago (2017-05-05 21:18:31 UTC) #18
Tima Vaisburd
On 2017/05/05 21:18:31, amaralp wrote: > Do you know if Android has already implemented the ...
3 years, 7 months ago (2017-05-05 21:22:33 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2863573004/40001
3 years, 7 months ago (2017-05-06 00:25:46 UTC) #24
commit-bot: I haz the power
3 years, 7 months ago (2017-05-06 19:41:49 UTC) #27
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/3ba7b83974443ba8324d70411774...

Powered by Google App Engine
This is Rietveld 408576698