New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 713850 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

"Start Speaking" in context menu not working

Project Member Reported by ekaramad@chromium.org, Apr 20 2017

Issue description

Chrome Version: 58.0.3029.81 (Official Build) (64-bit)
OS: Mac

What steps will reproduce the problem?
(1) Go to any page with some text.
(2) Right click a word to highlight it and get the context menu.
(3) Select "Speech>Start Speaking".

What is the expected result?
The word should be read.

What happens instead?
Nothing.

This is a regression from previous stable.
 
Status: Started (was: Assigned)
Result of the bisect I did locally:
You are probably looking for a change made after 454340 (known good), but no later than 454367 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/08eddf09b057e24f14e20ed473836970d0d8021b..374b2669b0af7cd88882cff28555f5aaa04c52d7

Pointing at my own refactoring CL:
https://codereview.chromium.org/2694543002/

And the bug being due to the fact that after moving |selected_text_| to TextInputManager, I did not remove the variable form RenderWidgetHostViewMac and SpeakSelection still uses that value which is always empty now. Uploading a fix very soon.
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 24 2017

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

commit b69ac7cd2ccc200ff53b95b30cbbea962a27e0d3
Author: ekaramad <ekaramad@chromium.org>
Date: Mon Apr 24 19:26:31 2017

Fix a recent regression in Speech->Start Speaking on Mac

After a recent refactoring, all RenderWidgetHostViews use TextInputManager in
order to get the value of |selected_text_|. Unfortunately this was not applied
to SpeakText() call inside RenderWidgetHostViewMac and furthermore, the local
variable |selected_text_| was not removed from the view.

BUG=713850

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

[modify] https://crrev.com/b69ac7cd2ccc200ff53b95b30cbbea962a27e0d3/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/b69ac7cd2ccc200ff53b95b30cbbea962a27e0d3/content/browser/renderer_host/render_widget_host_view_mac.mm

Labels: Merge-Request-59 M-59 M-58
Status: Fixed (was: Started)
Verified on Canary 60.0.3080.5. Going to chrome://version and double tapping "Official" reads the word as expected. Marking as fixed and adding merge request for 59.
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 26 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 27 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/50b0fc38c86e7b9994db4d00c5e5e2247b972a2c

commit 50b0fc38c86e7b9994db4d00c5e5e2247b972a2c
Author: EhsanK <ekaramad@chromium.org>
Date: Thu Apr 27 00:04:03 2017

Fix a recent regression in Speech->Start Speaking on Mac

After a recent refactoring, all RenderWidgetHostViews use TextInputManager in
order to get the value of |selected_text_|. Unfortunately this was not applied
to SpeakText() call inside RenderWidgetHostViewMac and furthermore, the local
variable |selected_text_| was not removed from the view.

BUG=713850

Review-Url: https://codereview.chromium.org/2831183002
Cr-Commit-Position: refs/heads/master@{#466715}
(cherry picked from commit b69ac7cd2ccc200ff53b95b30cbbea962a27e0d3)

Review-Url: https://codereview.chromium.org/2842383002 .
Cr-Commit-Position: refs/branch-heads/3071@{#243}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/50b0fc38c86e7b9994db4d00c5e5e2247b972a2c/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/50b0fc38c86e7b9994db4d00c5e5e2247b972a2c/content/browser/renderer_host/render_widget_host_view_mac.mm

Labels: TE-Verified-M59 TE-Verified-59.0.3071.29
Verified this issue on Mac OS 10.12.4 using chrome latest dev M59 #59.0.3071.29 by following steps mentioned in the original comment. 

Observed the words are reading out on enabling start speaking. Hence adding TE-Verified label.

Please refer the screen cast
Apr 27 2017 12-56 PM.webm
3.3 MB View Download
Cc: linds...@chromium.org
 Issue 729406  has been merged into this issue.
Cc: shrike@chromium.org
Status: Started (was: Fixed)
If we're going to merge  Issue 729406  into this one then we need to reopen this bug. This regression occurred because we apparently don't have any testing to make sure text-to-speech works. We need to create an automated test for that, or a manual test, but as it stands there's nothing currently stopping this regression from recurring at any time.

Sign in to add a comment