|
|
Created:
3 years, 8 months ago by jiahuiguo Modified:
3 years, 7 months ago CC:
chromium-reviews, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, gogerald+paymentswatch_chromium.org, rouslan+payments_chromium.org, mathp+autofillwatch_chromium.org, mahmadi+paymentswatch_chromium.org, estade+watch_chromium.org, vabr+watchlistautofill_chromium.org, sebsg+paymentswatch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionThis cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done.
Files touched are based on this filter:
"MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html
and
"Master Card" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html
TEST=components:components_unittests
Review-Url: https://codereview.chromium.org/2819183004
Cr-Commit-Position: refs/heads/master@{#473641}
Committed: https://chromium.googlesource.com/chromium/src/+/b65043769fd66a4a1baf6dbb811127f67ed6beae
Patch Set 1 #Patch Set 2 #
Total comments: 4
Patch Set 3 : Change some in comments #Patch Set 4 #
Total comments: 2
Patch Set 5 #
Total comments: 3
Patch Set 6 : Address comments. #Patch Set 7 : Resolve conflict #Patch Set 8 : Resolved conflict #Patch Set 9 : Resolved conflict #Messages
Total messages: 81 (52 generated)
Description was changed from ========== Change MasterCard to Mastercard BUG= ========== to ========== Change "MasterCard" to "Mastercard" since they switched to lowercase 'c' last year, Payments side also had such an effort done. TEST=components:components_unittests ==========
Description was changed from ========== Change "MasterCard" to "Mastercard" since they switched to lowercase 'c' last year, Payments side also had such an effort done. TEST=components:components_unittests ========== to ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html TEST=components:components_unittests ==========
The CQ bit was checked by jiahuiguo@google.com 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_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_tsan_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 jiahuiguo@google.com 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...
jiahuiguo@google.com changed reviewers: + jsaul@google.com, mathp@google.com
Hi Mathieu and Jared, PTAL.
https://codereview.chromium.org/2819183004/diff/20001/components/autofill/cor... File components/autofill/core/browser/credit_card_field.cc (right): https://codereview.chromium.org/2819183004/diff/20001/components/autofill/cor... components/autofill/core/browser/credit_card_field.cc:315: // a pretty common mistake; e.g., "Master Card" instead of "Mastercard". Missed a "Master Card" even though it looks like you're fixing them elsewhere. Maybe run a search for those too? https://codereview.chromium.org/2819183004/diff/20001/components/autofill_str... File components/autofill_strings.grdp (right): https://codereview.chromium.org/2819183004/diff/20001/components/autofill_str... components/autofill_strings.grdp:69: <message name="IDS_AUTOFILL_CC_MASTERCARD" desc="MasterCard credit card name." formatter_data="android_java"> Missed this one
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html TEST=components:components_unittests ========== to ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html and "Master Card" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html package:^chromium$ TEST=components:components_unittests ==========
The CQ bit was checked by jiahuiguo@google.com 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_tsan_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 jiahuiguo@google.com 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...
Description was changed from ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html and "Master Card" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html package:^chromium$ TEST=components:components_unittests ========== to ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html and "Master Card" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html TEST=components:components_unittests ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks. https://codereview.chromium.org/2819183004/diff/20001/components/autofill/cor... File components/autofill/core/browser/credit_card_field.cc (right): https://codereview.chromium.org/2819183004/diff/20001/components/autofill/cor... components/autofill/core/browser/credit_card_field.cc:315: // a pretty common mistake; e.g., "Master Card" instead of "Mastercard". On 2017/04/18 01:21:57, Jared Saul wrote: > Missed a "Master Card" even though it looks like you're fixing them elsewhere. > Maybe run a search for those too? Done. https://codereview.chromium.org/2819183004/diff/20001/components/autofill_str... File components/autofill_strings.grdp (right): https://codereview.chromium.org/2819183004/diff/20001/components/autofill_str... components/autofill_strings.grdp:69: <message name="IDS_AUTOFILL_CC_MASTERCARD" desc="MasterCard credit card name." formatter_data="android_java"> On 2017/04/18 01:21:57, Jared Saul wrote: > Missed this one Done.
lgtm https://codereview.chromium.org/2819183004/diff/60001/components/autofill/cor... File components/autofill/core/browser/autofill_manager_unittest.cc (right): https://codereview.chromium.org/2819183004/diff/60001/components/autofill/cor... components/autofill/core/browser/autofill_manager_unittest.cc:2651: std::vector<const char*> kCreditCardTypes = {"Visa", "Mastercard", "AmEx", Should this stay "Master card"? Looks like "discover" is testing capitalization so it's fine to change C->c here, I think
jiahuiguo@google.com changed reviewers: + mathp@chromium.org - mathp@google.com
https://codereview.chromium.org/2819183004/diff/60001/components/autofill/cor... File components/autofill/core/browser/autofill_manager_unittest.cc (right): https://codereview.chromium.org/2819183004/diff/60001/components/autofill/cor... components/autofill/core/browser/autofill_manager_unittest.cc:2651: std::vector<const char*> kCreditCardTypes = {"Visa", "Mastercard", "AmEx", On 2017/04/18 18:24:19, Jared Saul wrote: > Should this stay "Master card"? Looks like "discover" is testing capitalization > so it's fine to change C->c here, I think Done.
The CQ bit was checked by jiahuiguo@google.com 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: This issue passed the CQ dry run.
lgtm with nit Thanks for the fix! https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... File components/autofill/core/browser/autofill_field_unittest.cc (right): https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... components/autofill/core/browser/autofill_field_unittest.cc:815: field, ASCIIToUTF16("Mastercard"), "en-US", "en-US", &field); Could we add cases for variations of Mastercard such as MasterCard, Master Card?
https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... File components/autofill/core/browser/autofill_field_unittest.cc (right): https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... components/autofill/core/browser/autofill_field_unittest.cc:815: field, ASCIIToUTF16("Mastercard"), "en-US", "en-US", &field); On 2017/04/23 08:23:48, Mathieu wrote: > Could we add cases for variations of Mastercard such as MasterCard, Master Card? I was under the impression while reviewing that the "Case insensitivity: Discover vs. discover" test below accounted for that. These tests don't seem linked to the card types themselves but rather just using them for spacing/capitalization unit tests?
The CQ bit was checked by jiahuiguo@google.com 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...
PTAL. Thanks. https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... File components/autofill/core/browser/autofill_field_unittest.cc (right): https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... components/autofill/core/browser/autofill_field_unittest.cc:815: field, ASCIIToUTF16("Mastercard"), "en-US", "en-US", &field); On 2017/04/23 08:23:48, Mathieu wrote: > Could we add cases for variations of Mastercard such as MasterCard, Master Card? Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/04/25 20:20:32, jiahuiguo wrote: > PTAL. Thanks. > > https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... > File components/autofill/core/browser/autofill_field_unittest.cc (right): > > https://codereview.chromium.org/2819183004/diff/80001/components/autofill/cor... > components/autofill/core/browser/autofill_field_unittest.cc:815: field, > ASCIIToUTF16("Mastercard"), "en-US", "en-US", &field); > On 2017/04/23 08:23:48, Mathieu wrote: > > Could we add cases for variations of Mastercard such as MasterCard, Master > Card? > > Done. Can this be submitted then?
The CQ bit was checked by jiahuiguo@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from jsaul@google.com, mathp@chromium.org Link to the patchset: https://codereview.chromium.org/2819183004/#ps120001 (title: "Resolve conflict")
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: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?)) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
The CQ bit was checked by jiahuiguo@google.com
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: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
jiahuiguo@google.com changed reviewers: + jochen@chromium.org
jiahuiguo@google.com changed required reviewers: + jochen@chromium.org
Hi jochen@, PTAL chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java Thanks
On 2017/05/10 at 18:56:46, jiahuiguo wrote: > Hi jochen@, > > PTAL chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java > > Thanks please ask an owner that is closer to this file to review it.
jiahuiguo@google.com changed reviewers: + rouslan@chromium.org - jochen@chromium.org
jiahuiguo@google.com changed required reviewers: - jochen@chromium.org
Hi rouslan@, PTAL chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java Thanks
PersonalDataManagerTest.java LGTM
The CQ bit was checked by jiahuiguo@google.com
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: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...) 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 jiahuiguo@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, rouslan@chromium.org, jsaul@google.com Link to the patchset: https://codereview.chromium.org/2819183004/#ps140001 (title: "Resolved conflict")
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: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
jiahuiguo@google.com changed reviewers: + brettw@chromium.org
Hi brettw@, Needs your approval for chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java. Thanks.
On 2017/05/15 19:28:47, jiahuiguo wrote: > Hi brettw@, > > Needs your approval for > chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java. Can you please pick an owner in one of the more specific directories?
jiahuiguo@google.com changed reviewers: + tedchoc@chromium.org - brettw@chromium.org
Hi Tedchoc@, Needs your approval for chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java. Thanks.
On 2017/05/19 20:52:06, jiahuiguo wrote: > Hi Tedchoc@, > > Needs your approval for > chrome/android/javatests/src/org/chromium/chrome/browser/autofill/PersonalDataManagerTest.java. > > Thanks. PersonalDataManagerTest - lgtm
The CQ bit was checked by jiahuiguo@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org, tedchoc@chromium.org, rouslan@chromium.org, jsaul@google.com Link to the patchset: https://codereview.chromium.org/2819183004/#ps160001 (title: "Resolved conflict")
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_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by jiahuiguo@google.com
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": 160001, "attempt_start_ts": 1495475302340250, "parent_rev": "86c54d1018d43154254a9b479f43621bd8f6cad6", "commit_rev": "b65043769fd66a4a1baf6dbb811127f67ed6beae"}
Message was sent while issue was closed.
Description was changed from ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html and "Master Card" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html TEST=components:components_unittests ========== to ========== This cl changed "MasterCard" to "Mastercard" in the UI and related tests, since the brand is switched to lowercase 'c' last year, Payments side also had such an effort done. Files touched are based on this filter: "MasterCard" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html and "Master Card" package:^chromium$ case:yes exact:yes -f:xtb -f:.dic -f:.heuristics -f:.js -f:third_party -f:.html TEST=components:components_unittests Review-Url: https://codereview.chromium.org/2819183004 Cr-Commit-Position: refs/heads/master@{#473641} Committed: https://chromium.googlesource.com/chromium/src/+/b65043769fd66a4a1baf6dbb8111... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://chromium.googlesource.com/chromium/src/+/b65043769fd66a4a1baf6dbb8111... |