Closed Bug 1548381 Opened 5 years ago Closed 5 years ago

Show password generation UI on `autocomplete="new-password"` fields

Categories

(Toolkit :: Password Manager, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
mozilla69
Tracking Status
firefox69 --- verified
firefox70 --- verified

People

(Reporter: MattN, Assigned: MattN)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [passwords:generation] [skyline])

User Story

* `signon.generation.enabled` is the user pref to enable/disable the feature from about:preferences (not implemented yet).
* `signon.generation.available` controls whether the feature is available for users (e.g. if the about:preferences UI should show).

https://www.facebook.com is a popular site that uses autocomplete="new-password" for its registration form. Thousands of other test sites: https://docs.google.com/spreadsheets/d/1tKNeZh9SP3QBj5-X9O6QA6FddZpHY4qH7rHcn6LkTtI/edit#gid=1841286414

Attachments

(10 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
145.24 KB, image/png
Details

If an <input> has autocomplete="new-password", we have a strong signal that password generation is relevant for the field and so we should offer it to the user.

Mike, could you help us query httparchive data to understand the impact this could have? The query could be the following (in standard SQL):

SELECT page, url FROM `httparchive.response_bodies.2019_04_01_desktop`
WHERE REGEXP_CONTAINS(body, r'autocomplete=[\'"]?new-password[\'"]?')
ORDER BY url;

Thank you

Flags: needinfo?(miket)
See Also: → 1548391

Awesome, thank you!

Blocks: 1548854
Depends on: 1548857
Depends on: 1548874
Flags: qe-verify+
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Depends on: 1550669
Depends on: 1551407

We don't need to use JSON since we now support getCommentAt for extra data.

Also add unit tests that are missing.

Depends on D31207

Attachment #9064992 - Attachment description: Bug 1548381 - Generation Autocomplete Result → Bug 1548381 - Password Generation Autocomplete Result. r=sfoster
Attachment #9064995 - Attachment description: Bug 1548381 - Generation autocomplete UI → Bug 1548381 - Password Generation autocomplete UI
User Story: (updated)
Attachment #9064995 - Attachment description: Bug 1548381 - Password Generation autocomplete UI → Bug 1548381 - Password generation autocomplete UI. r=sfoster
User Story: (updated)
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/426750b88fc2
Add prefs to release and enable password generation. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/fde90ccfb570
Generate and cache a password for autocomplete="new-password" password fields. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/e0cf735bdcf5
LoginManagerParent.doAutocompleteSearch/getGeneratedPassword tests. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/1e2300b95a59
Simplify login autocomplete footer result to avoid JSON. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/60ff6e363acf
Password Generation Autocomplete Result. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/cddbcd92ec10
Make a generic two-line autocomplete richlistitem element. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/38e35b6d8d80
Password generation autocomplete UI. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/738ce5e88e05
Simplify test_autocomplete_new_password and use more common patterns. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/0e7d8f96bf12
Tests for the password generation autocomplete UI. r=sfoster
Flags: needinfo?(MattN+bmo)

I forgot to skip the tests on Android where the generation JSM isn't packaged.

Flags: needinfo?(MattN+bmo)
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/7b82d533e37e
Add prefs to release and enable password generation. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/c1beb9ce876d
Generate and cache a password for autocomplete="new-password" password fields. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/e79711ad9ef5
LoginManagerParent.doAutocompleteSearch/getGeneratedPassword tests. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/c5b9ba215f49
Simplify login autocomplete footer result to avoid JSON. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/1433d315b1b7
Password Generation Autocomplete Result. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/e92a0032aa2a
Make a generic two-line autocomplete richlistitem element. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/2a70e4e43c5e
Password generation autocomplete UI. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/34bbe924602e
Simplify test_autocomplete_new_password and use more common patterns. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/b74e5737da64
Tests for the password generation autocomplete UI. r=sfoster

+1
Great move!

User Story: (updated)

The first real website I have found with the requirements mentioned is www.yahoo.com. It appears that the "New Password" field has the attribute autocomplete="new-password", as the bug title suggests.

I have attempted to click inside the field and to trigger the password generator UI, but I had no luck. Can you explain how I can verify this implementation?

Tests were done on Nightly v69.0a1 (2019-05-23) (64-bit).

Flags: needinfo?(MattN+bmo)

(In reply to Bodea Daniel [:danibodea] from comment #20)

Created attachment 9067057 [details]
how is it supposed to work.png

The first real website I have found with the requirements mentioned is www.yahoo.com. It appears that the "New Password" field has the attribute autocomplete="new-password", as the bug title suggests.

I have attempted to click inside the field and to trigger the password generator UI, but I had no luck. Can you explain how I can verify this implementation?

Did you read the user story? It has 2 prefs to flip and lists Facebook as another test site. Btw. there is a list of thousands of sites you can test in comment 4 (which I now added to the User Story).

User Story: (updated)
Flags: needinfo?(MattN+bmo)

(In reply to Ben Bucksch (:BenB) from comment #22)

My fork with additional columns is here: https://docs.google.com/spreadsheets/d/1tKNeZh9SP3QBj5-X9O6QA6FddZpHY4qH7rHcn6LkTtI/edit#gid=1841286414

Not accessible without Google login.

Thanks, I didn't realize the other one was public so that's why I didn't make mine public. It's fixed now.

Hey, Matt. Sorry if I wasn't professional enough. For some reason, I haven't noticed the information in the user story. If I am to explain, I can't always read every little bit of info because, sometimes, most of it means nothing to me and my filtering skill are still evolving. Please bear with me. Things that I normally deduce in 10-15 minutes probably take you 1 or 2 to read and answer.

In other, more important notes, I have tested the first 15 top sites of 2019 and these are my results:

  • Google/Youtube - the password generation UI appears and functions quite well;

  • Yahoo: the password generation UI appears and functions quite well;

  • Pinterest: the password generation UI appears and functions quite well;

  • Facebook: the password generation UI does not appear (probably because the autocomplete attribute has the value "off", not "new-password")

  • Amazon: the password generation UI does not appear (probably because the autocomplete attribute is missing altogether)

  • Wikipedia: the password generation UI does not appear (probably because the autocomplete attribute is missing altogether)

  • Twitter: the password generation UI does not appear (probably because the autocomplete attribute is missing altogether)

  • Microsoft/Bing/MSN: the password generation UI does not appear (probably because the autocomplete attribute is missing altogether)

  • eBay: the password generation UI does not appear (probably because the autocomplete attribute has the value "off", not "new-password")

  • Linkedin: the password generation UI does not appear (probably because the autocomplete attribute is missing altogether)

On the note of the feature's functionality, this is what I have discovered:
- left clicking in the "New password" or "Confirm new password" fields, opens the Password Manager's suggestions drop-down.
- the drop-down displays: the saved logins for this site, "Use Generated Password" option with the password visible under it and the "View Saved Logins" button.
- selecting the "Use Generated Password" option will fill the field with the password shown on the option.
- the user then has the option to fill the same generated password in the "Confirm New Password" field by the same method.
On the downside:
- after ticking the "Show Password" checkbox, the Password Manager suggestions drop-down will no longer display. (intended?)
- after selecting the generated password and saving the password change, the password manager will prompt to save the newly changed password, but the username is left blank (on websites that the username is entered in the previous screen, like Yahoo).
- if the user then chooses to change his password again, it can be noticed that the same password is displayed by the password generator.

Please verify it if the functionality is as expected, NI me if further testing is needed, along with more information on how to proceed.
Thank you and sorry if I have created any kind of frustration.

Flags: needinfo?(MattN+bmo)
Depends on: 1559997
Depends on: 1559998
Whiteboard: [passwords:generation] [skyline]
Depends on: 1565696
Depends on: 1565697

(In reply to Bodea Daniel [:danibodea] from comment #24)

On the downside:
- after ticking the "Show Password" checkbox, the Password Manager suggestions drop-down will no longer display. (intended?)

You mean that if you toggle the checkbox in the doorhanger, you no longer see suggestions in the webpage? It's not expected to see a dropdown from the doorhanger field itself.

 - after selecting the generated password and saving the password change, the password manager will prompt to save the newly changed password, but the username is left blank (on sides the username is entered in the previous screen, like Yahoo).

This is now improved in Nightly. The auto-saved login is without a username but the doorhanger should include the username and choosing to "Update" should add it in storage.

 - if the user then chooses to change his password again, it can be noticed that the same password is displayed by the password generator.

Yeah, that is expected for now. Bug 1551723 and bug 1569568 will provide a way to request a new generated password. For now they last for the whole session for the same principal (origin + origin attributes [private browsing, container]).

Flags: needinfo?(MattN+bmo)
Depends on: 1570638

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #25)

(In reply to Bodea Daniel [:danibodea] from comment #24)

On the downside:
- after ticking the "Show Password" checkbox, the Password Manager suggestions drop-down will no longer display. (intended?)

You mean that if you toggle the checkbox in the doorhanger, you no longer see suggestions in the webpage? It's not expected to see a dropdown from the doorhanger field itself.

When I say Password Manager drop-down", I mean to talk about the drop-down with the "View Saved Logins" button, the one that drops down from a password field when attempting to manually fill a previously saved password or a generated password. I have logged bug 1570638.

 - after selecting the generated password and saving the password change, the password manager will prompt to save the newly changed password, but the username is left blank (on sites the username is entered in the previous screen, like Yahoo).

This is now improved in Nightly. The auto-saved login is without a username but the doorhanger should include the username and choosing to "Update" should add it in storage.

I can confirm this is true as far as my testing went.

 - if the user then chooses to change his password again, it can be noticed that the same password is displayed by the password generator.

Yeah, that is expected for now. Bug 1551723 and bug 1569568 will provide a way to request a new generated password. For now they last for the whole session for the same principal (origin + origin attributes [private browsing, container]).

Yes, thank you for the feedback.

I will close this bug as verified in Nightly v70.0a1 and in Beta 69.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.