Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1022143003:
Removes flag --enable-new-md-input-view and add a opposite flag: --disable-new-md-input-view. (Closed)
|
Created:
5 years, 9 months ago by FengYuan Modified:
5 years, 8 months ago CC:
chromium-reviews, extensions-reviews_chromium.org, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, asvitkine+watch_chromium.org, nona+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRemoves flag --enable-new-md-input-view and add a opposite flag: --disable-new-md-input-view.
BUG=422869, 470405
TBR=asvitkine@chromium.org,bshe@chromium.org
Committed: https://crrev.com/0ad7e3344e09bb78960e3b83a0c7d2197c3c5941
Cr-Commit-Position: refs/heads/master@{#322124}
Patch Set 1 #
Total comments: 4
Patch Set 2 : #
Total comments: 2
Patch Set 3 : #Patch Set 4 : Fixed virtual keyboard browser test broken #
Total comments: 1
Patch Set 5 : #Patch Set 6 : #
Messages
Total messages: 35 (13 generated)
fengyuan@chromium.org changed reviewers: + asvitkine@chromium.org, shuchen@chromium.org
https://codereview.chromium.org/1022143003/diff/1/chrome/browser/chromeos/ext... File chrome/browser/chromeos/extensions/input_method_api.cc (right): https://codereview.chromium.org/1022143003/diff/1/chrome/browser/chromeos/ext... chrome/browser/chromeos/extensions/input_method_api.cc:44: output->SetBoolean("isNewMDInputViewDisabled", please don't change this. https://codereview.chromium.org/1022143003/diff/1/chrome/common/extensions/ap... File chrome/common/extensions/api/input_method_private.json (right): https://codereview.chromium.org/1022143003/diff/1/chrome/common/extensions/ap... chrome/common/extensions/api/input_method_private.json:27: "isNewMDInputViewDisabled": {"type": "boolean"}, please don't change this.
PTAL https://codereview.chromium.org/1022143003/diff/1/chrome/browser/chromeos/ext... File chrome/browser/chromeos/extensions/input_method_api.cc (right): https://codereview.chromium.org/1022143003/diff/1/chrome/browser/chromeos/ext... chrome/browser/chromeos/extensions/input_method_api.cc:44: output->SetBoolean("isNewMDInputViewDisabled", On 2015/03/23 02:16:23, Shu Chen wrote: > please don't change this. Done. https://codereview.chromium.org/1022143003/diff/1/chrome/common/extensions/ap... File chrome/common/extensions/api/input_method_private.json (right): https://codereview.chromium.org/1022143003/diff/1/chrome/common/extensions/ap... chrome/common/extensions/api/input_method_private.json:27: "isNewMDInputViewDisabled": {"type": "boolean"}, On 2015/03/23 02:16:23, Shu Chen wrote: > please don't change this. Done.
lgtm
The CQ bit was checked by fengyuan@chromium.org
The CQ bit was unchecked by fengyuan@chromium.org
The CQ bit was checked by fengyuan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1022143003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fengyuan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1022143003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by fengyuan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1022143003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:53126: - <int value="413081240" label="enable-new-md-input-view"/> Please keep the old flag entry values here, so that data from older versions can be decoded.
Thanks for your review, PTAL. https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (left): https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:53126: - <int value="413081240" label="enable-new-md-input-view"/> On 2015/03/24 13:41:22, Alexei Svitkine (away) wrote: > Please keep the old flag entry values here, so that data from older versions can > be decoded. Done.
On 2015/03/24 17:30:55, FengYuan wrote: > Thanks for your review, PTAL. > > https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... > File tools/metrics/histograms/histograms.xml (left): > > https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... > tools/metrics/histograms/histograms.xml:53126: - <int value="413081240" > label="enable-new-md-input-view"/> > On 2015/03/24 13:41:22, Alexei Svitkine (away) wrote: > > Please keep the old flag entry values here, so that data from older versions > can > > be decoded. > > Done. Did you forget to upload a new patchset?
I forgot it's chromium..take it as critique... The patchset has uploaded, PTAL :) On 2015/03/24 17:34:25, Alexei Svitkine (away) wrote: > On 2015/03/24 17:30:55, FengYuan wrote: > > Thanks for your review, PTAL. > > > > > https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... > > File tools/metrics/histograms/histograms.xml (left): > > > > > https://codereview.chromium.org/1022143003/diff/20001/tools/metrics/histogram... > > tools/metrics/histograms/histograms.xml:53126: - <int value="413081240" > > label="enable-new-md-input-view"/> > > On 2015/03/24 13:41:22, Alexei Svitkine (away) wrote: > > > Please keep the old flag entry values here, so that data from older versions > > can > > > be decoded. > > > > Done. > > Did you forget to upload a new patchset?
fengyuan@chromium.org changed reviewers: + rsadam@chromium.org
Hi, Raheem Could you help review for the tests change? Thanks.
fengyuan@chromium.org changed reviewers: + bshe@chromium.org
The CQ bit was checked by fengyuan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from shuchen@chromium.org Link to the patchset: https://codereview.chromium.org/1022143003/#ps60001 (title: "Fixed virtual keyboard browser test broken")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1022143003/60001
virtual_keyboard/layout_test.js LGTM with nit. https://codereview.chromium.org/1022143003/diff/60001/chrome/test/data/chrome... File chrome/test/data/chromeos/virtual_keyboard/layout_test.js (right): https://codereview.chromium.org/1022143003/diff/60001/chrome/test/data/chrome... chrome/test/data/chromeos/virtual_keyboard/layout_test.js:182: // TODO(fengyuan): bring back this handwriting test once found the cause. Nit: Please file a bug and include the bug id here for bringing back the test.
On 2015/03/25 03:48:16, rsadam wrote: > virtual_keyboard/layout_test.js LGTM with nit. > > https://codereview.chromium.org/1022143003/diff/60001/chrome/test/data/chrome... > File chrome/test/data/chromeos/virtual_keyboard/layout_test.js (right): > > https://codereview.chromium.org/1022143003/diff/60001/chrome/test/data/chrome... > chrome/test/data/chromeos/virtual_keyboard/layout_test.js:182: // > TODO(fengyuan): bring back this handwriting test once found the cause. > Nit: Please file a bug and include the bug id here for bringing back the test. Done. See crbug.com/470405
Thanks for your review.
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/0ad7e3344e09bb78960e3b83a0c7d2197c3c5941 Cr-Commit-Position: refs/heads/master@{#322124} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
