1692980 - Update new password heuristics model

Status: VERIFIED FIXED

(Toolkit :: Password Manager, enhancement, P2)

Description

[Dimi Lee [:dimi]]

Bug 1595244 introduced a machine learning model using Fathom to improve detection of new password fields for password generation (v1). The last time we updated this model was in Bug 1638187 (15 May 2020)

It's time to collect those false-positive and false-negative cases discovered since then to improve the accuracy of the model.

Comment 1

[Dimi Lee [:dimi]]

Attachment: Bug 1692980 - P1. Update NewPasswordModel.jsm to 55407b0. r=sfoster

The improvements include:

• Add three french keywords

  • Add Créer to newStringRegex
  • Add S'inscrire to registerStringRegex
  • Add Créer un compte to registerStringRegex

• Add a formHasMultipleVisibleInput signal.
  This rule returns true when there is more than 3 visible input in a form.
  Since the idea in the signal is that a registration page often has multiple inputs.
  This rule only selects inputs whose type is either email, password, text, tel or empty,
  which are more likely a input field for users to fill their information.

• Support formless password field in formHasMultipleVisibleInput signal.
  For password fields don't have an associated form, signals that require a form always return false.
  This patch adds an additional heuristic in formHasMultipleVisibleInput signal to support formless
  password field. The heuristic works as follow:

  1. Locate the closest preceding input with selector
     "input[type=email],input[type=text],,input[type=tel],input[type=password]".
  2. Find the lowest common ancestor of the password field and the input field found in step1
  3. Use the common ancestor as the "form" element, apply formHasMultipleVisibleInput signal

Comment 2

[Dimi Lee [:dimi]]

Update the result, Precision doesn't have significant difference and Recall is up around 5%

Before:
Training precision  : 1.0000   Recall: 0.8964
Validation precision: 0.9905   Recall: 0.8125
Testing precision   : 0.9926   Recall: 0.8933

After:
Training precision  : 1.0000   Recall: 0.9321
Validation precision: 1.0000   Recall: 0.8828
Testing precision   : 0.9862   Recall: 0.9533

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d1e9ad2dd1d
Update NewPasswordModel.jsm to 78d4bf8. r=sfoster,tgiles

Comment 4

[Atila Butkovits]

Backed out for causing failures at test_isProbablyANewPasswordField.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/5672c4f15bd77ce43f10dba2a1de280a2d63e1c0

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=ccCJW5xVSrKjAPEbd6QSKw.0&revision=6d1e9ad2dd1defed2f06c1baf8f87ef6f6d75e53&searchStr=linux%2C18.04%2Cx64%2Casan%2Copt%2Cxpcshell%2Ctests%2Ctest-linux1804-64-asan%2Fopt-xpcshell-e10s%2Cx2

Failure log: https://treeherder.mozilla.org/logviewer?job_id=336886276&repo=autoland&lineNumber=2268

Comment 5

[Dimi Lee [:dimi]]

okay, there are two problems:

1. isVisible in fathom throws exception in xpcsehll-test because the element doesn't have a window
2. isProbablyANewPassword returns false for "confirmed password" in password change form (score is 0.73, slight lower than the current threshold holder 0.75). I'll check if that also happens in other password change forms in our samples.

Comment 6

[Dimi Lee [:dimi]]

While doing more test, I found out that there is a drawback on the new signal added. The drawback is that it doesn’t work well for password-change forms that have multiple input fields, for example, a password-change form has "current", "new" and "confirm" password fields.

I'll work on the problem and include the fix in this patch.

Comment 7

[Dimi Lee [:dimi]]

Attachment: Bug 1692980 - P2. Fix testcase failures after updating NewPasswordModel r=sfoster

Depends on D111926

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a23cc886289
P1. Update NewPasswordModel.jsm to 55407b0. r=sfoster,tgiles
https://hg.mozilla.org/integration/autoland/rev/1b690d7067c3
P2. Fix testcase failures after updating NewPasswordModel r=sfoster,tgiles

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/2a23cc886289
https://hg.mozilla.org/mozilla-central/rev/1b690d7067c3

Comment 10

[Timea Cernea [:tbabos][inactive]]

Confirming this as verified-fixed after checking and closing all the issues from dependencies. Several other generated password site requirements issues were also raised in the process and mentioned in comments where needed.
The only remaining issue that seems like it is not fixed is Bug 1674499, a comment and NI? was left for that bug as well.

Comment 11

[Masataka Yakura [:myakura]]

the change in https://hg.mozilla.org/mozilla-central/rev/2a23cc886289 might cause regressions in some Japanese and Chinese forms as they somehow started include ">" characters before/inside keywords.

(reported as https://bugzilla.mozilla.org/show_bug.cgi?id=1714776 , in case if missed)