Use title case for headers of saved and blacklisted passwords list in settings on iOS |
||||
Issue description(1) Open Settings > Save Passwords. (2) Look at the headings for the list of saved passwords and for blacklisted passwords below. Actual: They are "Saved passwords" and "Never saved". Expected: As pschaffner@ suggests, those should be "Saved Passwords" and "Never Saved" instead.
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd8b83c8aba9791af6e6116b5db2e0a3d034872c commit fd8b83c8aba9791af6e6116b5db2e0a3d034872c Author: Vaclav Brozek <vabr@chromium.org> Date: Wed Jul 12 10:15:58 2017 Password settings on iOS: use Title Case Most of setting headers in the passwords seciton on iOS already use Title Case, except for the headers of the saved and blacklisted lists of passwords. This CL fixes that by creating new strings for iOS settings inside ios/chrome/app/strings/ios_strings.grd. Previously the code used shared strings from the password_manager component (components/password_manager_strings.grdp). That string was also used by the old (non-MD) desktop settings, which have already been deprecated. The MD settings on desktop use the Title Case as iOS should as well, but the MD settings strings are consistently saved in chrome/app/settings_strings.grdp, where they cannot be shared with iOS. The CL creates the new string instead of sharing the one from MD settings, because sharing has no benefit (on no build both strings are included, so no storage spared) and has the drawback of binding the style of both strings forever together. The CL does not remove the component string yet. The old desktop settings code is still in the codebase, and the shared string (used only by the old settings) should be deleted once those old settings are deleted. The CL was approved in https://chromium-review.googlesource.com/c/567181. This is a verbatim copy of that CL, but re-uploaded after Gerrit issues prevented me to actually land the original one. Hence I'm TBR-in lpromero@ who approved the original. TBR=lpromero@chromium.org Bug: 740952 Change-Id: I455b8f2a5edd1089916a7b87f20eddc3b274a2b7 Reviewed-on: https://chromium-review.googlesource.com/567930 Reviewed-by: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Vaclav Brozek <vabr@chromium.org> Cr-Commit-Position: refs/heads/master@{#485912} [modify] https://crrev.com/fd8b83c8aba9791af6e6116b5db2e0a3d034872c/ios/chrome/app/strings/ios_strings.grd [modify] https://crrev.com/fd8b83c8aba9791af6e6116b5db2e0a3d034872c/ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff9611dd2d5858f3ff65b3df37777e6c0cc3ab58 commit ff9611dd2d5858f3ff65b3df37777e6c0cc3ab58 Author: Sergio Collazos <sczs@chromium.org> Date: Wed Jul 12 16:51:58 2017 Revert "Password settings on iOS: use Title Case" This reverts commit fd8b83c8aba9791af6e6116b5db2e0a3d034872c. Reason for revert: This CL breaks password_settings_eg_test, we might need to update the tests so they use the new A11y labels. Though I'm not sure if there's more than that. Original change's description: > Password settings on iOS: use Title Case > > Most of setting headers in the passwords seciton on iOS already use Title Case, > except for the headers of the saved and blacklisted lists of passwords. > > This CL fixes that by creating new strings for iOS settings inside > ios/chrome/app/strings/ios_strings.grd. Previously the code used shared strings > from the password_manager component (components/password_manager_strings.grdp). > That string was also used by the old (non-MD) desktop settings, which have > already been deprecated. The MD settings on desktop use the Title Case as iOS > should as well, but the MD settings strings are consistently saved in > chrome/app/settings_strings.grdp, where they cannot be shared with iOS. > > The CL creates the new string instead of sharing the one from MD settings, > because sharing has no benefit (on no build both strings are included, so no > storage spared) and has the drawback of binding the style of both strings > forever together. > > The CL does not remove the component string yet. The old desktop settings code > is still in the codebase, and the shared string (used only by the old settings) > should be deleted once those old settings are deleted. > > The CL was approved in https://chromium-review.googlesource.com/c/567181. This > is a verbatim copy of that CL, but re-uploaded after Gerrit issues prevented me > to actually land the original one. Hence I'm TBR-in lpromero@ who approved the > original. > TBR=lpromero@chromium.org > > Bug: 740952 > Change-Id: I455b8f2a5edd1089916a7b87f20eddc3b274a2b7 > Reviewed-on: https://chromium-review.googlesource.com/567930 > Reviewed-by: Vaclav Brozek <vabr@chromium.org> > Commit-Queue: Vaclav Brozek <vabr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#485912} TBR=vabr@chromium.org,lpromero@chromium.org Change-Id: I41a5e01b086a1643a6f72f3934d5cfea6ceb57b6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 740952 Reviewed-on: https://chromium-review.googlesource.com/568387 Reviewed-by: Sergio Collazos <sczs@chromium.org> Commit-Queue: Sergio Collazos <sczs@chromium.org> Cr-Commit-Position: refs/heads/master@{#485997} [modify] https://crrev.com/ff9611dd2d5858f3ff65b3df37777e6c0cc3ab58/ios/chrome/app/strings/ios_strings.grd [modify] https://crrev.com/ff9611dd2d5858f3ff65b3df37777e6c0cc3ab58/ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm
,
Jul 12 2017
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/31b2b6fb8ebda8f9584443a6f7a6fb8e64c4a529 commit 31b2b6fb8ebda8f9584443a6f7a6fb8e64c4a529 Author: Vaclav Brozek <vabr@chromium.org> Date: Wed Jul 12 21:43:30 2017 Reland: Password settings on iOS: use Title Case The original CL https://chromium-review.googlesource.com/567930 was reverted because EG tests were not synchronised with the string change. The reverted CL is patch set 1 here. The fix consists of updating the tests, those are all subsequent patch sets here. Original description: --------------------- Most of setting headers in the passwords seciton on iOS already use Title Case, except for the headers of the saved and blacklisted lists of passwords. This CL fixes that by creating new strings for iOS settings inside ios/chrome/app/strings/ios_strings.grd. Previously the code used shared strings from the password_manager component (components/password_manager_strings.grdp). That string was also used by the old (non-MD) desktop settings, which have already been deprecated. The MD settings on desktop use the Title Case as iOS should as well, but the MD settings strings are consistently saved in chrome/app/settings_strings.grdp, where they cannot be shared with iOS. The CL creates the new string instead of sharing the one from MD settings, because sharing has no benefit (on no build both strings are included, so no storage spared) and has the drawback of binding the style of both strings forever together. The CL does not remove the component string yet. The old desktop settings code is still in the codebase, and the shared string (used only by the old settings) should be deleted once those old settings are deleted. --------------------- Bug: 740952 Change-Id: I1b166a4961235fdaa5a122795cc016d9b5438612 Reviewed-on: https://chromium-review.googlesource.com/568497 Commit-Queue: Vaclav Brozek <vabr@chromium.org> Commit-Queue: Louis Romero <lpromero@chromium.org> Reviewed-by: Louis Romero <lpromero@chromium.org> Cr-Commit-Position: refs/heads/master@{#486089} [modify] https://crrev.com/31b2b6fb8ebda8f9584443a6f7a6fb8e64c4a529/ios/chrome/app/strings/ios_strings.grd [modify] https://crrev.com/31b2b6fb8ebda8f9584443a6f7a6fb8e64c4a529/ios/chrome/browser/ui/settings/passwords_settings_egtest.mm [modify] https://crrev.com/31b2b6fb8ebda8f9584443a6f7a6fb8e64c4a529/ios/chrome/browser/ui/settings/save_passwords_collection_view_controller.mm [modify] https://crrev.com/31b2b6fb8ebda8f9584443a6f7a6fb8e64c4a529/ios/chrome/browser/ui/settings/settings_egtest.mm
,
Jul 13 2017
,
Jul 18 2017
Verified in 61.0.3160.0 Canary, iPhone 7 iOS 10.2, iPad mini 10.3.3 "Saved Passwords" and "Never Saved" are seen. Looks good. |
||||
►
Sign in to add a comment |
||||
Comment 1 by vabr@chromium.org
, Jul 11 2017181 KB
181 KB View Download
168 KB
168 KB View Download
182 KB
182 KB View Download
168 KB
168 KB View Download