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

Status: VERIFIED FIXED

(Toolkit :: Password Manager, enhancement, P2)

Description

[Matthew N. [:MattN]]

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.

Comment 1

[Matthew N. [:MattN]]

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

Comment 2

[Mike Taylor [:miketaylr]]

Here you go, https://docs.google.com/spreadsheets/d/1nCZ8QulQdk_whqNtun2kEL64kjhJfw8IdtdOglrr1_w/edit#gid=1841286414

Comment 3

[Matthew N. [:MattN]]

Awesome, thank you!

Comment 4

[Matthew N. [:MattN]]

(In reply to Mike Taylor [:miketaylr] from comment #2)

Here you go, https://docs.google.com/spreadsheets/d/1nCZ8QulQdk_whqNtun2kEL64kjhJfw8IdtdOglrr1_w/edit#gid=1841286414

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

Comment 5

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Add prefs to release and enable password generation. r=sfoster

Comment 6

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Generate and cache a password for autocomplete="new-password" password fields. r=sfoster

Depends on D31205

Comment 7

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - LoginManagerParent.doAutocompleteSearch/getGeneratedPassword tests. r=sfoster

Depends on D31206

Comment 8

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Simplify login autocomplete footer result to avoid JSON. r=sfoster

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

Comment 9

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Password Generation Autocomplete Result. r=sfoster

Depends on D31208

Comment 10

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Make a generic two-line autocomplete richlistitem element. r=sfoster

Depends on D31209

Comment 11

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Password generation autocomplete UI. r=sfoster

Depends on D31210

Comment 12

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Simplify test_autocomplete_new_password and use more common patterns. r=sfoster

Depends on D31211

Comment 13

[Matthew N. [:MattN]]

Attachment: Bug 1548381 - Tests for the password generation autocomplete UI. r=sfoster

Depends on D31575

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Dorel Luca [:dluca]]

Backed out 9 changesets (bug 1548381) for XPCShell failures in toolkit/components/passwordmgr/test/unit/test_LoginManagerParent_doAutocompleteSearch.js

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=247446655&repo=autoland&lineNumber=2961

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0e7d8f96bf123f5d0f491fe7780223bde509e841

Backout:
https://hg.mozilla.org/integration/autoland/rev/2690e619a493a6730d6318589405aa2e229ba5c3

Comment 16

[Matthew N. [:MattN]]

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

Comment 17

[Pulsebot]

Pushed by [e-mail]:
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

Comment 18

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/7b82d533e37e
https://hg.mozilla.org/mozilla-central/rev/c1beb9ce876d
https://hg.mozilla.org/mozilla-central/rev/e79711ad9ef5
https://hg.mozilla.org/mozilla-central/rev/c5b9ba215f49
https://hg.mozilla.org/mozilla-central/rev/1433d315b1b7
https://hg.mozilla.org/mozilla-central/rev/e92a0032aa2a
https://hg.mozilla.org/mozilla-central/rev/2a70e4e43c5e
https://hg.mozilla.org/mozilla-central/rev/34bbe924602e
https://hg.mozilla.org/mozilla-central/rev/b74e5737da64

Comment 19

[Ben Bucksch (:BenB)]

+1
Great move!

Comment 20

[Daniel Bodea [:danibodea]]

Attachment: 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?

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

Comment 21

[Matthew N. [:MattN]]

(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).

Comment 22

[Ben Bucksch (:BenB)]

FYI:

https://docs.google.com/spreadsheets/d/1nCZ8QulQdk_whqNtun2kEL64kjhJfw8IdtdOglrr1_w/edit#gid=1841286414

Accessible

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

Not accessible without Google login.

Comment 23

[Matthew N. [:MattN]]

(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.

Comment 24

[Daniel Bodea [:danibodea]]

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.

Comment 25

[Matthew N. [:MattN]]

(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]).

Comment 26

[Daniel Bodea [:danibodea]]

(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.