Chromium Code Reviews
Help | Chromium Project | Sign in
(9)

Issue 2703643004: [TTS] Add an ACK message to SelectWordAroundCaret.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 4 days ago by Donn Denman
Modified:
8 hours, 37 minutes ago
Reviewers:
aelias
CC:
agrieve+watch_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, donnd+watch_chromium.org, jam, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, nona+watch_chromium.org, shuchen+watch_chromium.org, James Su, twellington+watch_chromium.org, Theresa, yusukes+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[TTS] Add an ACK message to SelectWordAroundCaret. Adds an ACK message so callers of SelectWordAroundCaret can know if the selection actually happened and how much the selection was adjusted. BUG=692824, 435778

Patch Set 1 #

Patch Set 2 : Clean up the logic in render_view_impl.cc #

Total comments: 2

Patch Set 3 : Just fixed a typo in a comment. #

Total comments: 10

Patch Set 4 : Changed to send the message on all platforms, not just Android. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -1 line) Patch
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java View 1 2 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java View 1 2 1 chunk +13 lines, -0 lines 0 comments Download
M content/browser/android/content_view_core_impl.h View 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/android/content_view_core_impl.cc View 1 chunk +11 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.h View 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_view_android.cc View 1 2 3 2 chunks +11 lines, -0 lines 0 comments Download
M content/common/view_messages.h View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/SelectionPopupController.java View 1 chunk +6 lines, -0 lines 0 comments Download
M content/renderer/render_view_impl.cc View 1 2 3 1 chunk +12 lines, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 20 (12 generated)
Donn Denman
Theresa, Please take a first quick-look at this CL. I'm not sure who will need ...
1 week, 3 days ago (2017-02-17 16:13:46 UTC) #7
Theresa
The approach generally looks good to me. We should discuss with aelias@. https://codereview.chromium.org/2703643004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java File content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java ...
1 week, 3 days ago (2017-02-17 18:36:55 UTC) #8
Donn Denman
Thanks Theresa!
1 week, 3 days ago (2017-02-17 19:08:17 UTC) #9
Donn Denman
https://codereview.chromium.org/2703643004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java File content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java (right): https://codereview.chromium.org/2703643004/diff/20001/content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java#newcode33 content/public/android/java/src/org/chromium/content/browser/ContextualSearchClient.java:33: * Acknowledges that a selectWordAroundCaret action has completed with ...
1 week, 3 days ago (2017-02-17 19:09:14 UTC) #11
Donn Denman
Alex, PTAL at this addition of an ACK for SelectWordAroundCaret. I'm unsure if it should ...
1 week, 3 days ago (2017-02-17 19:11:02 UTC) #13
aelias
https://codereview.chromium.org/2703643004/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java File chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java (right): https://codereview.chromium.org/2703643004/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java#newcode442 chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java:442: // TODO(donnd): use startAdjust and endAdjust in upcoming text-extraction ...
5 days, 23 hours ago (2017-02-22 03:36:01 UTC) #14
Donn Denman
aelias, PTAL. Sorry for the lack of context with this CL. Here's a summary and ...
5 days, 4 hours ago (2017-02-22 23:13:02 UTC) #19
Donn Denman
8 hours, 37 minutes ago (2017-02-27 18:53:42 UTC) #20
FYI Alex and I talked f2f about this CL and we're going to wait until the
dependent CL is ready for a full review.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f8e48bd