Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2506203002:
Fix language switching (Closed)
|
Created:
4 years, 1 month ago by David Tseng Modified:
4 years, 1 month ago Reviewers:
dmazzoni CC:
chromium-reviews, alemate+watch_chromium.org, oshima+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, yuzo+watch_chromium.org, je_julie, arv+watch_chromium.org, dtseng+watch_chromium.org, dmazzoni+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix language switching
Ensure we're
- querying for the user's preferred languages via
chrome.i18n.getAcceptLanguages
- matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es.
TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish.
Previously, we would have picked German (the last clause of the our matching predicates which just picks the first voice).
BUG=591453
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/c1cab294711bca021f9e21a06e9183ca88b8c62a
Cr-Commit-Position: refs/heads/master@{#433747}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Fix the test. #Patch Set 3 : No tts lang. #
Messages
Total messages: 33 (21 generated)
Description was changed from ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. BUG= ========== to ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. Previously, we would have picked German (the last clause of the our matching predicates which just picks the first voice). BUG=591453 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
dtseng@chromium.org changed reviewers: + dmazzoni@chromium.org
lgtm https://codereview.chromium.org/2506203002/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js (right): https://codereview.chromium.org/2506203002/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:60: chrome.i18n.getUILanguage(); Did you confirm that this is in the right format, i.e. with hyphens and not underscores, and the same capitalization? https://codereview.chromium.org/2506203002/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:688: var currentLocale = acceptLanguages[0] || Ideally we should use all acceptLanguages? Maybe add a TODO
https://codereview.chromium.org/2506203002/diff/1/chrome/browser/resources/ch... File chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js (right): https://codereview.chromium.org/2506203002/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:60: chrome.i18n.getUILanguage(); On 2016/11/17 22:49:38, dmazzoni wrote: > Did you confirm that this is in the right > format, i.e. with hyphens and not underscores, > and the same capitalization? Yes, getMessage seems to be the outlier (xx_XX) getUILanguage and getAcceptLanguage both return xx-XX or xx https://codereview.chromium.org/2506203002/diff/1/chrome/browser/resources/ch... chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:688: var currentLocale = acceptLanguages[0] || On 2016/11/17 22:49:38, dmazzoni wrote: > Ideally we should use all acceptLanguages? > Maybe add a TODO > It's all kind of guess work at this point because I see the browser UI language is different from the user's accept language and ChromeVox localized strings are all based upon the ui locale (i.e. the extension resources). We should definitely re-visit. Note the prsence of chrome.i18n.detectLanguage which we could use on a page to figure out which tts voice to use. I tried it out and our engine is just too slow in switching to make it practical (not to even begin considering how reliable the detection is).
The CQ bit was checked by dtseng@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dtseng@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org Link to the patchset: https://codereview.chromium.org/2506203002/#ps20001 (title: "Fix the test.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by dtseng@chromium.org
The CQ bit was checked by dtseng@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by dtseng@chromium.org
The CQ bit was checked by dtseng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dtseng@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org Link to the patchset: https://codereview.chromium.org/2506203002/#ps40001 (title: "No tts lang.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dtseng@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479775196936210,
"parent_rev": "d5dfe499f85a870e807670f01c41199d863f55fe", "commit_rev":
"6d1bcbcade2c947773b8a2fedf8d15729a9f4abb"}
Message was sent while issue was closed.
Description was changed from ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. Previously, we would have picked German (the last clause of the our matching predicates which just picks the first voice). BUG=591453 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. Previously, we would have picked German (the last clause of the our matching predicates which just picks the first voice). BUG=591453 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. Previously, we would have picked German (the last clause of the our matching predicates which just picks the first voice). BUG=591453 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Fix language switching Ensure we're - querying for the user's preferred languages via chrome.i18n.getAcceptLanguages - matching in both directions i.e. voice.lang starts with of ui locale and ui locale starts with voice lang. For example, es, es-ES vs es-ES, es. TEST=add Spanish as a voice and get the voice to be first in the languages and input screen (UI is somewhat inaccessible). Verify that ChromeVox, when re-started, speaks in Spanish. Previously, we would have picked German (the last clause of the our matching predicates which just picks the first voice). BUG=591453 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/c1cab294711bca021f9e21a06e9183ca88b8c62a Cr-Commit-Position: refs/heads/master@{#433747} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/c1cab294711bca021f9e21a06e9183ca88b8c62a Cr-Commit-Position: refs/heads/master@{#433747} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
