New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 673447 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 714618
issue 770046



Sign in to add a comment

It is too easy to delete a password by accident in settings

Project Member Reported by vabr@chromium.org, Dec 12 2016

Issue description

Both the old about:settings/passwords, and the new about:md-settings/passwords have this issue:

If the user clicks the UI control for deleting a password, it gets deleted immediately and without undo. Clicking the control is very easy:
* In the old settings, it is right next to the "Show" button in the password field.
* In the new settings, it is right below the "Details" menu.

This is handled better by both the password bubble, and the passwords.google.com website -- both of them offer a reasonable Undo feature.

We should provide the same undo feature in the settings. Or a confirmation prompt. Or at least make it much harder to mis-click and get a password deleted.
 

Comment 1 by vabr@chromium.org, Feb 21 2017

Cc: sabineb@chromium.org
 Issue 694351  has been merged into this issue.

Comment 2 by kolos@chromium.org, Apr 24 2017

 Issue 298759  has been merged into this issue.

Comment 3 by kolos@chromium.org, Apr 24 2017

Blocking: 714618

Comment 4 by battre@chromium.org, Jun 30 2017

Cc: nyerramilli@chromium.org dbeam@chromium.org
 Issue 738306  has been merged into this issue.

Comment 5 by dbeam@chromium.org, Jun 30 2017

Cc: hcarmona@chromium.org

Comment 6 by dpa...@chromium.org, Jun 30 2017

Cc: tbuck...@chromium.org
Labels: Proj-MaterialDesign-WebUI

Comment 7 by bettes@chromium.org, Aug 18 2017

Let's just commit the action as we do today, and use toasts to handle the confirmation and undo.

Toasts already exist in MD-Bookmarks and should generally be applied to all areas of Settings that deal with sensitive information, such as Site Details. 

https://docs.google.com/presentation/d/1GILSCx1IFUHC8WHyrP2Iymjq9_swPw4xB5D_2PbiBcE/edit#slide=id.g24381d27bb_3_37

I'd imagine just 1 toast (not 2 as in passwords.google.com) in the bottom left, reading:

[ Password deleted             UNDO  ]

The same timing logic and color styling used in Bookmarks is applicable here as well. 
Owner: jdoerrie@chromium.org
Status: Started (was: Available)
Cc: jdoerrie@chromium.org
 Issue 755469  has been merged into this issue.
Cc: calamity@chromium.org tsergeant@chromium.org
+tost_manager.html authors, tsergeant@, calamity@

To summarize some discussion in the CLs and among the Settings team, we should move bookmarks-toast-manager from md_bookmarks/ to a shared location and use it in Settings.

The correct location for the shared element is ui/webui/resources/elements/ because 'cr-toast-manager' is a single element that should not require any strings.

The current implementation includes an 'Undo' button, but that should either be part of the toast <content>, or a more generic 'button-label' property if we think 'toast' will always include a button (which seems reasonable to assume until / unless a scenario comes up where we don't need a button).

To make sure it's not forgotten: we need to have keyboard support for undo CTRL + Z (COMMAND + Z on mac) for accessibility.

Timeouts on toast may not last long enough for someone with mobility issues or someone who is trying to use a screen reader. 

If the toast will always have a button like Steven suggested in #10, then we should make the toast handle the keyboard shortcut so it's not forgotten.
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a6fd9b262f68dae95a3aa100fde287959d865bf

commit 7a6fd9b262f68dae95a3aa100fde287959d865bf
Author: jdoerrie <jdoerrie@chromium.org>
Date: Mon Sep 18 13:48:04 2017

Introduce cr-toast

This change extracts the toast handling logic from MD Bookmarks into its
own CR element called cr-toast.

Bug:  673447 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I85bc9ed62b5866b4590d07d665b9e8d1711b912c
Reviewed-on: https://chromium-review.googlesource.com/660702
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502568}
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/chrome/browser/resources/md_bookmarks/compiled_resources2.gyp
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/chrome/browser/resources/md_bookmarks/toast_manager.html
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/chrome/browser/resources/md_bookmarks/toast_manager.js
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/chrome/test/data/webui/cr_elements/cr_elements_browsertest.js
[add] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/chrome/test/data/webui/cr_elements/cr_toast_test.js
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/chrome/test/data/webui/md_bookmarks/toast_manager_test.js
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/ui/webui/resources/cr_elements/compiled_resources2.gyp
[add] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/ui/webui/resources/cr_elements/cr_toast/compiled_resources2.gyp
[add] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/ui/webui/resources/cr_elements/cr_toast/cr_toast.html
[add] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/ui/webui/resources/cr_elements/cr_toast/cr_toast.js
[modify] https://crrev.com/7a6fd9b262f68dae95a3aa100fde287959d865bf/ui/webui/resources/cr_elements_resources.grdp

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e

commit e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e
Author: jdoerrie <jdoerrie@chromium.org>
Date: Fri Sep 22 10:38:51 2017

Remove TimerProxy from MD Bookmarks

This change removes TimerProxy from MD Bookmarks in favor of using window
directly and deprecates TestTimerProxy in favor of MockTimer.

Bug:  673447 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I488f4d73753c056f85a7a1a51c1606879551f3b7
Reviewed-on: https://chromium-review.googlesource.com/664759
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503708}
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/browser_resources.grd
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/resources/md_bookmarks/api_listener.html
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/resources/md_bookmarks/compiled_resources2.gyp
[add] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/resources/md_bookmarks/debouncer.html
[add] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/resources/md_bookmarks/debouncer.js
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/resources/md_bookmarks/dnd_manager.html
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/resources/md_bookmarks/dnd_manager.js
[delete] https://crrev.com/97521318f7a23ae5eebccc33b724966299c047a5/chrome/browser/resources/md_bookmarks/timer_proxy.html
[delete] https://crrev.com/97521318f7a23ae5eebccc33b724966299c047a5/chrome/browser/resources/md_bookmarks/timer_proxy.js
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc
[modify] https://crrev.com/e3378cdc1a4aa1625ad24ddb1c3f0bc5cee12a7e/chrome/test/data/webui/md_bookmarks/test_timer_proxy.js

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3bc5e2682778f84572efc5bf4383b34d5c48e699

commit 3bc5e2682778f84572efc5bf4383b34d5c48e699
Author: jdoerrie <jdoerrie@chromium.org>
Date: Wed Sep 27 14:19:58 2017

Implement "Undo Delete Password" in Settings Page

This change implements an UNDO toast following a deletion of a saved password or
password exception entry. It is similar in looks and functionality to the
already exising UNDO toast for MD Bookmarks.

Bug:  673447 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I10ac58afc9e10bb48d01a5a9513e74f247057879
Reviewed-on: https://chromium-review.googlesource.com/643296
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504659}
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/app/settings_strings.grdp
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/extensions/api/passwords_private/passwords_private_api.cc
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/extensions/api/passwords_private/passwords_private_api.h
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/resources/settings/passwords_and_forms_page/compiled_resources2.gyp
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.html
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/resources/settings/settings_resources.grd
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/ui/passwords/password_manager_presenter.cc
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/ui/passwords/password_manager_presenter.h
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/common/extensions/api/passwords_private.idl
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/chrome/test/data/extensions/api_test/passwords_private/test.js
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/extensions/browser/extension_function_histogram_value.h
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/third_party/closure_compiler/externs/passwords_private.js
[modify] https://crrev.com/3bc5e2682778f84572efc5bf4383b34d5c48e699/tools/metrics/histograms/enums.xml

Comment 15 by kolos@chromium.org, Sep 29 2017

Blocking: 770046
Status: Fixed (was: Started)

Sign in to add a comment