Username correction for the Save password bubble |
|||||||
Issue descriptionThis is bug for tracking implementation progress for the username correction feature. Give the user the ability to edit the username being saved from the prompt. This would handle cases in which the Password Manager misdetect (or don’t detect at all) the username field, helping ensure a clean set of saved credentials The short doc, which helps to understand the feature: https://docs.google.com/a/google.com/document/d/1zTXXiiyJ5NdYSNEUNUjPUT6jyzCqv52rqjsY8GXZ6II/edit?usp=sharing
,
Jun 22 2017
Thanks Irmak for testing various cases. Please also check how it looks for Right-To-Left languages. FYI, you can run Chrome with the following command to redefine the language "export LANGUAGE=he_IL && out/Release/chrome"
,
Jul 4 2017
To comment #5: could you please explain why you changed UI? Feedback from UI team? Thx.
,
Jul 4 2017
Currently we are using the mocks from this slide; https://docs.google.com/presentation/d/10za0hvOXmZNjjTcViMFjMJIoOU8LJKJlaE6iQ2LtTgY/edit#slide=id.g2266fb2e3c_2_0 Though the design here is not exactly the same with what is implemented (the explanation texts are not in the current design), we want to have something close and reasonable enough before we send the feature to ui review. After the ui review, there might be another update depending on the feedback we get from them.
,
Jul 5 2017
Peter, could you please take a look whether the plans here run afoul of your work on Harmony? Thanks!
,
Jul 5 2017
You mean changing the username label to a textfield? I don't think that causes inherent problems with Harmonizing the dialog.
,
Jul 5 2017
There will be a new edit button on the bottom left (in LTR languages) of the dialog.
,
Jul 5 2017
I think that's still probably OK, but I would suggest you ping hwi@, the interaction designer for Harmony, rather than me on this one, to ensure that's a pattern we actually want in Harmony. I can't remember if we were able to eliminate the other cases of having a third button (that wasn't a Learn More icon). If not, I can't remember if we wanted to collapse three-button dialogs into "all three on the bottom left" or not. Hwi would know the answers to those better.
,
Jul 5 2017
(Removing myself because I will be soon on leave.)
,
Jul 5 2017
3 buttons (C#5) is not against Harmony pattern, but recommended to avoid. Similar to the pencil button approach on c#3 and also delete icons on Saved passwords (https://screenshot.googleplex.com/QaKUPR12h1L.png, w/ Harmony - https://screenshot.googleplex.com/SmnWSFNQq3X.png), a non-blocking suggestion I have is trying the pencil icon on the same row *after* password(****). Another 3-button case is Bookmark, which we haven't found an alternative yet. With translate UI, the current proposal is to use an inline labeled button (https://screenshot.googleplex.com/Ux5SsJTV2vN.png). We continue to seek ideas to make these cases simpler, please keep looking for future improvements.
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2fd1706822e4bd350826788f822d9a738ffca034 commit 2fd1706822e4bd350826788f822d9a738ffca034 Author: irmakk <irmakk@google.com> Date: Thu Jul 06 16:45:01 2017 Edit button makes username editable in the password manager bubble. When user clicks on the edit button in the password manager bubble; - Username becomes editable - Focus is on editable field - Edit button becomes disabled While username is editable, if user hits tab/enter/esc keys; - Username field becomes a label again - Focus is on save button - Edit button is enabled again Known issue: Since this cl doesn't contain any communication with the username in the background and currently only has the ui part of the project, edited username will be restored to its original state after editing is done. This problem will be solved with the next cl sending the information to back-end and communication with the manager itself. BUG= 734965 Review-Url: https://codereview.chromium.org/2960843002 Cr-Commit-Position: refs/heads/master@{#484635} [modify] https://crrev.com/2fd1706822e4bd350826788f822d9a738ffca034/chrome/browser/ui/views/passwords/manage_password_items_view.cc [modify] https://crrev.com/2fd1706822e4bd350826788f822d9a738ffca034/chrome/browser/ui/views/passwords/manage_password_items_view.h [modify] https://crrev.com/2fd1706822e4bd350826788f822d9a738ffca034/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
,
Jul 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a73d254af7d4cec0aeb8fe0e343f2aca0278a7c3 commit a73d254af7d4cec0aeb8fe0e343f2aca0278a7c3 Author: Irmak Kavasoglu <irmakk@google.com> Date: Fri Jul 07 17:49:59 2017 When editable field is replaced by a label upon losing focus, new username is kept. In the current implementation, when user changes the username in the editable field and then loses focus on the field; the editable field is replaced by a label which still has the old username on it. This cl helps keeping the user's changes in the username field. This cl does NOT save the new username to the password manager. UI-wise everything looks okay, but save button will still save the old username. Bug: 734965 Change-Id: I61bef70894de4e2e71e9e314f30915d011795ccc Reviewed-on: https://chromium-review.googlesource.com/563657 Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#484977} [modify] https://crrev.com/a73d254af7d4cec0aeb8fe0e343f2aca0278a7c3/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc [modify] https://crrev.com/a73d254af7d4cec0aeb8fe0e343f2aca0278a7c3/chrome/browser/ui/passwords/manage_passwords_bubble_model.h [modify] https://crrev.com/a73d254af7d4cec0aeb8fe0e343f2aca0278a7c3/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
,
Jul 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dc3c5d94b78ed1958216b8426f57a37bcd71b35a commit dc3c5d94b78ed1958216b8426f57a37bcd71b35a Author: Irmak Kavasoglu <irmakk@google.com> Date: Tue Jul 18 08:11:19 2017 Username correction is working. After user edits the username; - Losing focus by pressing enter or tab will keep the new username - Losing focus by pressing esc will discard the change and keep the old username Clicking the save button will save the form with the edited username (if the changes were not discarded). Bug: 734965 Change-Id: I35301f57ebb11d58b4545e1f31a272e7bff9ea65 Reviewed-on: https://chromium-review.googlesource.com/563672 Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#487417} [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/password_manager/password_manager_test_base.cc [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller_unittest.mm [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/manage_passwords_ui_controller_mock.h [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/passwords_model_delegate.h [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/chrome/browser/ui/passwords/passwords_model_delegate_mock.h [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/components/password_manager/core/browser/password_form_manager.cc [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/components/password_manager/core/browser/password_form_manager.h [modify] https://crrev.com/dc3c5d94b78ed1958216b8426f57a37bcd71b35a/components/password_manager/core/browser/password_form_manager_unittest.cc
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7e16af080f37a36bb814a159fdb4332c2afa2f9f commit 7e16af080f37a36bb814a159fdb4332c2afa2f9f Author: Irmak Kavasoglu <irmakk@google.com> Date: Wed Jul 19 09:52:47 2017 Added the flag for username correction. Bug: 734965 Change-Id: I18f2c03539774b7aa71e2357f54220f5de5d8b5f Reviewed-on: https://chromium-review.googlesource.com/575236 Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org> Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Cr-Commit-Position: refs/heads/master@{#487808} [modify] https://crrev.com/7e16af080f37a36bb814a159fdb4332c2afa2f9f/chrome/browser/about_flags.cc [modify] https://crrev.com/7e16af080f37a36bb814a159fdb4332c2afa2f9f/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/7e16af080f37a36bb814a159fdb4332c2afa2f9f/chrome/browser/flag_descriptions.h [modify] https://crrev.com/7e16af080f37a36bb814a159fdb4332c2afa2f9f/tools/metrics/histograms/enums.xml
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/960a5df52ff2169d689d4af6b2684e3568e0e1a3 commit 960a5df52ff2169d689d4af6b2684e3568e0e1a3 Author: Irmak Kavasoğlu <irmakk@google.com> Date: Fri Jul 21 08:22:18 2017 Added the edit button to pending password bubble for mac platform. Created the edit button and added it to the pending password bubble for mac platform. Clicking the button does not do anything. Screenshot is added to the bug as comment #19. Bug: 734965 Change-Id: Iaf424fc8a9964543995a6ef321b9a33af596a594 Reviewed-on: https://chromium-review.googlesource.com/570440 Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#488619} [modify] https://crrev.com/960a5df52ff2169d689d4af6b2684e3568e0e1a3/chrome/browser/ui/cocoa/passwords/pending_password_view_controller.mm [modify] https://crrev.com/960a5df52ff2169d689d4af6b2684e3568e0e1a3/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller.h [modify] https://crrev.com/960a5df52ff2169d689d4af6b2684e3568e0e1a3/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller.mm [modify] https://crrev.com/960a5df52ff2169d689d4af6b2684e3568e0e1a3/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller_unittest.mm
,
Jul 31 2017
,
Aug 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44 commit ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44 Author: Irmak Kavasoğlu <irmakk@google.com> Date: Mon Aug 07 10:24:51 2017 Edit button makes username field editable on save password bubble. This cl is a partial implementation of the username correction feature for mac platform. When user clicks edit button; - The username field becomes editable - The focus is on the editable field - Edit button is disabled When user hits enter/tab/esc; - Username field loses focus and becomes a label again - Focus is on the save button - Edit button is clickable again Known issue: The changes made in the username field are not kept. When editable field loses focus, the old username is restored. Bug: 734965 Change-Id: I49d9e8167b9fca381c230236143d4a6304f3057a Reviewed-on: https://chromium-review.googlesource.com/593609 Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Cr-Commit-Position: refs/heads/master@{#492289} [modify] https://crrev.com/ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44/chrome/browser/ui/cocoa/passwords/password_item_views.h [modify] https://crrev.com/ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44/chrome/browser/ui/cocoa/passwords/passwords_list_view_controller.mm [modify] https://crrev.com/ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller.h [modify] https://crrev.com/ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller.mm [modify] https://crrev.com/ed6f7a2248a482ac3fdc3ea71a5571c8dd909b44/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller_unittest.mm
,
Aug 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aa6879b08b08b7dab43edee853a84970d0673682 commit aa6879b08b08b7dab43edee853a84970d0673682 Author: Irmak Kavasoğlu <irmakk@google.com> Date: Mon Aug 07 12:21:21 2017 Username correction works in mac platform With this cl the username correction feature fully works in mac platform. The edited username is kept when user loses focus of the editable field. The initial username is restored if user loses focus of the editable field by pressing escape. If edited username is an already existing one, the password is overwritten. Bug: 734965 Change-Id: Ifff3447cffe799c093e7dd679eca46cd7afe60c3 Reviewed-on: https://chromium-review.googlesource.com/603311 Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Cr-Commit-Position: refs/heads/master@{#492302} [modify] https://crrev.com/aa6879b08b08b7dab43edee853a84970d0673682/chrome/browser/about_flags.cc [modify] https://crrev.com/aa6879b08b08b7dab43edee853a84970d0673682/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller.mm [modify] https://crrev.com/aa6879b08b08b7dab43edee853a84970d0673682/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller_unittest.mm
,
Aug 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96d39c792665ec619a4cfb0d05da47420100661f commit 96d39c792665ec619a4cfb0d05da47420100661f Author: Irmak Kavasoğlu <irmakk@google.com> Date: Thu Aug 10 15:21:25 2017 Enabled username correction by default Enabled username correction feature by default and updated tests accordingly. Bug: 734965 Change-Id: I2c4a61b2d791075193650e49179e24ee6d0c8969 Reviewed-on: https://chromium-review.googlesource.com/610162 Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Commit-Queue: Irmak Kavasoğlu <irmakk@google.com> Cr-Commit-Position: refs/heads/master@{#493394} [modify] https://crrev.com/96d39c792665ec619a4cfb0d05da47420100661f/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller_unittest.mm [modify] https://crrev.com/96d39c792665ec619a4cfb0d05da47420100661f/components/password_manager/core/common/password_manager_features.cc
,
Aug 18 2017
,
Sep 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b2c42f478aabe786612026f8f7aa3146057c380 commit 4b2c42f478aabe786612026f8f7aa3146057c380 Author: Vasilii Sukhanov <vasilii@chromium.org> Date: Thu Sep 21 09:30:11 2017 Drop the edit button in the password save bubble for Views. Now the bubble is always in the editing mode. Bug: 734965 Change-Id: I8600b983c5a06170f554bfbd012aea72b04a31f4 Reviewed-on: https://chromium-review.googlesource.com/675365 Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org> Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#503400} [modify] https://crrev.com/4b2c42f478aabe786612026f8f7aa3146057c380/chrome/browser/ui/cocoa/passwords/save_pending_password_view_controller.mm [modify] https://crrev.com/4b2c42f478aabe786612026f8f7aa3146057c380/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc [modify] https://crrev.com/4b2c42f478aabe786612026f8f7aa3146057c380/chrome/browser/ui/passwords/manage_passwords_bubble_model.h [modify] https://crrev.com/4b2c42f478aabe786612026f8f7aa3146057c380/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc [modify] https://crrev.com/4b2c42f478aabe786612026f8f7aa3146057c380/chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc
,
Nov 17 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by irmakk@google.com
, Jun 21 201734.0 KB
34.0 KB View Download
34.0 KB
34.0 KB View Download