1551730 - Use HTMLInputElement.hasBeenTypePassword instead of checking `type` manually

Status: VERIFIED FIXED

(Toolkit :: Password Manager, enhancement, P2)

Description

[Matthew N. [:MattN]]

In order to proper handle password fields where the user is able to toggle @type to reveal the password, we should use the ChromeOnly HTMLInputElement.hasBeenTypePassword API to determine whether a field is a "password" field.

We may want to be careful about filling into an unmask password field but I'm not sure there is a good solution to that since we can't be sure the user still has a visible toggle.

Comment 3

[Matthew N. [:MattN]]

This affects https://accounts.google.com (bug 1595414) and other popular sites (causing us not to save or autocomplete) so I think we should address this in the near-term.

Comment 4

[Matthew N. [:MattN]]

We may want to also update the DOMInputPasswordAdded event code to use this property too.

Comment 6

[Matthew N. [:MattN]]

…and nsFormFillController.cpp

Comment 7

[Matthew N. [:MattN]]

I've rebased some old patches I had for this.

Comment 8

[Matthew N. [:MattN]]

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

We may want to be careful about filling into an unmask password field but I'm not sure there is a good solution to that since we can't be sure the user still has a visible toggle.

I wonder if we should start by only filling into these when userTriggered?

Comment 9

[Matthew N. [:MattN]]

Context menu code using this.onPassword also should change.

Comment 10

[Matthew N. [:MattN]]

Attachment: Bug 1551730 - Rename some _autocomplete.html mochitests to the test_autocomplete_*.html prefix. r=sfoster

Comment 11

[Matthew N. [:MattN]]

Attachment: Bug 1551730 - Use hasBeenTypePassword to determine if we're on a password field for autocomplete. r=sfoster

Depends on D68322

Comment 12

[Matthew N. [:MattN]]

Attachment: Bug 1551730 - Use hasBeenTypePassword for login filling. r=sfoster

Depends on D68323

Comment 13

[Matthew N. [:MattN]]

Attachment: Bug 1551730 - Don't autofill passwords when hasBeenTypePassword but type!=password. r=sfoster

Depends on D68324

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97360a5f3b85
Rename some _autocomplete.html mochitests to the test_autocomplete_*.html prefix. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/7e4f51820b4a
Use hasBeenTypePassword to determine if we're on a password field for autocomplete. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/27a026da4531
Use hasBeenTypePassword for login filling. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/71e4739111e7
Don't autofill passwords when hasBeenTypePassword but type!=password. r=sfoster

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/97360a5f3b85
https://hg.mozilla.org/mozilla-central/rev/7e4f51820b4a
https://hg.mozilla.org/mozilla-central/rev/27a026da4531
https://hg.mozilla.org/mozilla-central/rev/71e4739111e7

Comment 16

[Timea Cernea [:tbabos]]

Hey Matt,
Verified the scenarios described in Bug 1570638 and Bug 1616523 and they are both fixed with this on Windows 10.
Is there anything else to check for this bug fix?

Comment 17

[Matthew N. [:MattN]]

I already tested bug 1595414 so I don't think you need to again. That's probably good enough. Thanks

Comment 18

[Timea Cernea [:tbabos]]

Thanks Matt!
Verified-Fixed on latest Nightly 76.0a1 (2020-04-01) on Windows 10, MacOS 10.13 and Ubuntu 16.04 based on Comment 16.