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

Issue 794453002: Updates manifest files to use the new nl_data.js in input tool (Closed)

Created:
6 years ago by bshe
Modified:
6 years ago
Reviewers:
Shu Chen
CC:
chromium-reviews, stevenjb+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, nkostylev+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Updates manifest files to use the new nl_data.js in input tool BUG=NONE TEST= Note: this test step depends on xkb extension update first. It should be soon available in canary channel. 1. In chrome://settings/languages page, add "Dutch" 2. Check if you see two IMES available for this language(netherland and belgian) 3. enable the two imes and switch to them (assume netherland first) 4. verify the default virtual keyboard is a compact qwerty keyboard and can switch to us-intl full layout 4. switch to belgian keyboard, verify it is azerty full keyboard and no compact layout for it Committed: https://crrev.com/fd85200868d990525942516352b12b9da25c6ddd Cr-Commit-Position: refs/heads/master@{#308086}

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Patch Set 3 : #

Total comments: 4

Patch Set 4 : reviews #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -8 lines) Patch
M chrome/app/chromeos_strings.grdp View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/resources/chromeos/input_method/xkb_manifest.json View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 11 (2 generated)
bshe
Hi Shu. This is the chrome side change follow: https://critique.corp.google.com/#review/81727097 Do you mind to take ...
6 years ago (2014-12-10 00:32:50 UTC) #2
Shu Chen
https://codereview.chromium.org/794453002/diff/20001/chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json File chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json (right): https://codereview.chromium.org/794453002/diff/20001/chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json#newcode88 chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json:88: "name": "__MSG_keyboard_netherland__", You need to add definition of MSG_keyboard_netherland ...
6 years ago (2014-12-10 07:14:54 UTC) #3
bshe
https://codereview.chromium.org/794453002/diff/20001/chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json File chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json (right): https://codereview.chromium.org/794453002/diff/20001/chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json#newcode88 chrome/browser/resources/chromeos/input_method/google_xkb_manifest.json:88: "name": "__MSG_keyboard_netherland__", On 2014/12/10 07:14:54, Shu Chen wrote: > ...
6 years ago (2014-12-10 15:18:41 UTC) #4
Shu Chen
https://codereview.chromium.org/794453002/diff/40001/chrome/app/chromeos_strings.grdp File chrome/app/chromeos_strings.grdp (right): https://codereview.chromium.org/794453002/diff/40001/chrome/app/chromeos_strings.grdp#newcode4747 chrome/app/chromeos_strings.grdp:4747: Netherland keyboard s/Netherland/Netherlands https://codereview.chromium.org/794453002/diff/40001/chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc File chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc (right): https://codereview.chromium.org/794453002/diff/40001/chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc#newcode164 chrome/browser/chromeos/input_method/component_extension_ime_manager_impl.cc:164: ...
6 years ago (2014-12-11 03:57:02 UTC) #5
bshe
Thanks for review. I have updated the cl here: https://critique.corp.google.com/#review/81727097 too. https://codereview.chromium.org/794453002/diff/40001/chrome/app/chromeos_strings.grdp File chrome/app/chromeos_strings.grdp (right): ...
6 years ago (2014-12-11 19:05:01 UTC) #6
Shu Chen
lgtm
6 years ago (2014-12-12 01:11:07 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/794453002/60001
6 years ago (2014-12-12 14:31:49 UTC) #9
commit-bot: I haz the power
Committed patchset #4 (id:60001)
6 years ago (2014-12-12 15:15:47 UTC) #10
commit-bot: I haz the power
6 years ago (2014-12-12 15:16:35 UTC) #11
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/fd85200868d990525942516352b12b9da25c6ddd
Cr-Commit-Position: refs/heads/master@{#308086}

Powered by Google App Engine
This is Rietveld 408576698