Password Page forward navigation should be disabled while attempt in progress |
|||||
Issue descriptionIf the user clicks the 'Done' button while waiting for an IPC response for the password check, each click adds another check to the queue causing bad UX. The simplest solution is just to disable the 'Done' button while waiting for a response.
,
Oct 1
The overall idea SGTM, but we should be careful to disable forward navigation (rather than the "Done" button itself) while a forward navigation attempt is in progress. The reason for this distinction is that forward navigation can also be attempted by hitting the "Enter" keyboard key instead of by clicking the "Done" button. See https://cs.chromium.org/chromium/src/ui/webui/resources/cr_components/chromeos/multidevice_setup/multidevice_setup.js?q=onForwardNavigationRequested_.
,
Oct 1
Good call
,
Oct 2
When you say "causing bad UX", can you be more specific? What user-facing consequence does this have?
,
Oct 2
The issue is that it will queue up multiple attempts. If I understand what's happening under the hood correctly, if the user clicks 'Done' 2 times, the UI will try to get an auth token with whatever is in the text field and then, if the password was wrong, as soon as it receives that 'wrong password' response, it will attempt to get an auth token with whatever is in the text field at that point. From the user's perspective, if the accidentally or impatiently click the button a few times, they'll keep seeing the message 'Wrong password' reappear.
,
Oct 2
I think it's actually slightly different from that. It queues up an attempt for the next time you add text to the field so that, once you add it, it tries to submit again.
,
Oct 2
Thanks, Jordy. From your description, it doesn't seem like this warrants being considered a P1. We should sill try to fix it before launch, but P1 should be reserved for worse issues than this. If there are other, more dangerous or problematic issues that result from this bug, let me know.
,
Oct 3
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac6fb40d1a553232eb8847369e33f3f997d42d53 commit ac6fb40d1a553232eb8847369e33f3f997d42d53 Author: Jordy Greenblatt <jordynass@chromium.org> Date: Thu Oct 04 03:13:16 2018 [CrOS MultiDevice] Stop Setup Flow from accumulating password attempts The current HEAD behavior of the Setup Flow password page is to allow the user to repeatedly request forward navigation while the IPC for a password check is in progress. Then these requests for a queue so that the next time the user changes the contexts of the password input field, the next request in the queue is sent causing an accidental password check/setup attempt. This CL adds a field to the password page to allow it to determine whether it is in the middle of waiting for a password check to complete and if so, prevent any forward navigation attempt (including from the 'enter' key) and disable the button so the user knows it is out of commission while waiting for the IPC response. The CL also removes some now unused code surrounding expired auth tokens which made it harder to follow the logic flow for the IPC and callback: In the original design, the user would enter their password before they got to the start setup page and would therefore have to hold onto an auth token throughout the flow. In particular, it was necessary to set a timeout leading to a callback to erase the token once it was expired. In the current design, however, the auth token is used to complete the setup flow as soon as it is successfully obtained so there is no case in which a user would have time to let their token expire. Bug: 890958 Change-Id: Ia82af98ebaeeed0429c2da114d6e667130100c99 Reviewed-on: https://chromium-review.googlesource.com/c/1260215 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Commit-Queue: Jordy Greenblatt <jordynass@chromium.org> Cr-Commit-Position: refs/heads/master@{#596491} [modify] https://crrev.com/ac6fb40d1a553232eb8847369e33f3f997d42d53/ui/webui/resources/cr_components/chromeos/multidevice_setup/password_page.js
,
Oct 4
,
Oct 4
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by hansberry@chromium.org
, Oct 1