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

Issue 890958 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug


Participants' hotlists:
Better-Together-Launch-Blockers


Sign in to add a comment

Password Page forward navigation should be disabled while attempt in progress

Project Member Reported by jordynass@chromium.org, Oct 1

Issue description

If 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.
 
Cc: khorimoto@chromium.org
Summary: Password Page forward navigation should be disabled while attempt in progress (was: Password Page 'Done' button clicks queue password submission attempts)
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_.
Good call
When you say "causing bad UX", can you be more specific? What user-facing consequence does this have?
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.
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.
Labels: -Pri-1 Pri-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.
Owner: jordynass@chromium.org
Status: Started (was: Available)
Project Member

Comment 9 by bugdroid1@chromium.org, 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

Cc: nohle@chromium.org
 Issue 890903  has been merged into this issue.
Status: Fixed (was: Started)

Sign in to add a comment