1638187 - Update new password heuristics model to version 4.0 at c1e5136

Status: VERIFIED FIXED

(Toolkit :: Password Manager, enhancement, P1)

Description

[:severin Rudie]

+++ This bug was initially created as a clone of Bug #1629132 +++

Bug 1595244 introduced a machine learning model using Fathom to improve detection of new password fields for password generation (v1) which was updated in Bug 1625601 (v2) mostly for accuracy improvements on password change forms and a slight performance bump.

The upstream model has since been updated with the following changes:

• Add nextInputIsConfirmy function to improve accuracy in forms with "password" & "confirm password" inputs.

Comment 1

[:severin Rudie]

Attachment: Bug 1638187 - Update NewPasswordModel.jsm to c1e5136. r=bdanforth

Updates the Fathom model to improve recognition of password input fields for the password manager.

Testing results:

Site	Training Result*	My Result
icoadm.in	Partial	Success
sogou.com		Success
nfpa.org	Partial	Success
facebook.com recovery form	Success	Fail (Fathom not called)
vk.com		Fail (Fathom not called)
perfumania.com	Partial	Fail (Fathom not called)
zoom.us		Partial (Password: .5207, Confirm: .9996)
sohu.com		Fail (.5207)
weibo.com		Fail (.5207)
facebook.com change form	Success	Partial (Password: .3350, Confirm: .7748)
DigiFinex	Partial	Partial (Password: .3451, Confirm: .8146)
zhanqi.tv		Fail (.2148)
twitter.com		Fail (.0426)

• taken from the commit message of https://github.com/mozilla-services/fathom-login-forms/commit/368cbce234fcd2804f82d88d10ac940b8243c496

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b903c581d871
Update NewPasswordModel.jsm to c1e5136. r=bdanforth

Comment 3

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/b903c581d871

Comment 4

[Matthew N. [:MattN]]

Please verify the pwgen now works on the sites in the bugs that this bug blocks.

Comment 5

[Timea Cernea [:tbabos]]

Bug 1629899 and Bug 1629909 are verified fixed, but Bug 1629901 results are inconsistent on QA side, commented on that bug for more info.
Let me know if we will treat that as an edge case (thus mark this bug as verified-fixed) or wait for further improvements on the heuristics model.

Comment 6

[Matthew N. [:MattN]]

Erik, does comment 1 align with your expectations of the model update? I thought it was going to address most of the P1 bugs.

(In reply to Timea Cernea [:tbabos] from comment #5)

Let me know if we will treat that as an edge case (thus mark this bug as verified-fixed) or wait for further improvements on the heuristics model.

I think this bug can be verified and we will have to handle the other site fixes in another model update.

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 9149222 [details]
Bug 1638187 - Update NewPasswordModel.jsm to c1e5136. r=bdanforth

Beta/Release Uplift Approval Request

• User impact if declined: Password generation won't appear on as many websites. This was something we promoted in Fx76.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Model update like we've done multiple times. Somewhat self-contained code.
• String changes made/needed: None

Comment 8

[Matthew N. [:MattN]]

Telemetry data is also good: https://sql.telemetry.mozilla.org/dashboard/pwmgr-password-generation-field-detection?p_channel=nightly

Comment 9

[Timea Cernea [:tbabos]]

Marking this as verified-fixed for Nightly 78 based on Comment 5 and Comment 6. Waiting for uplift to Beta 77.

Comment 10

[Erik Rose [:erik][:erikrose]]

Erik, does comment 1 align with your expectations of the model update?

No. I suspect Severin hasn't updated to the version of Fathom with the fixed isVisible() routine, 3.4. Run pip install -U fathom-web to update. It should also make his job a lot easier in general, as vectorization is now fully automatic. See https://mozilla.github.io/fathom/versions.html#id2. I re-ran the numbers on my machine and confirmed they are as I claimed once you get Fathom up to date.

Note that you need update Fathom only on Severin's machine; there is no need to update the version in Firefox, since isVisible() isn't called from the code that runs in production. All you should have to do is paste new coefficients into the model.

I thought it was going to address most of the P1 bugs.

I'm not sure if it's the majority of the current P1s or not. What I said (on April 30 in #fathom on Matrix) was "out of 9 QA failures we were able to collect, I fixed 6 with one rule added".

For completeness, my differences from the table in https://bugzilla.mozilla.org/show_bug.cgi?id=1638187#c1 are:

• icoadm.in works 100%.
• facebook.com recovery form works 100%.
• perfumania.com works 100%.
• facebook.com change form works 100%.
• bitribe.com works 100%. This was one I fixed (https://bugzilla.mozilla.org/show_bug.cgi?id=1629892), but it's missing from the table.
• (nfpa.org was the 6th fix, but it's reflected properly in the table.)

Some of the other ones, like zoom.us, hadn't been filed at the time Vlad did the collection.

Let me know if that doesn't clear it up, and we'll get to the bottom of it. Always happy to have a Zoom session with Severin; I'm impressed how far he's gotten on his own!

Comment 11

[:severin Rudie]

Hey Erik!

I didn't test against any vectorized pages. All of my testing was in an instance of FF with that new rule & updated coefficients (see Phab), so unless I'm mistaken it doesn't sound like that should be the problem. My best guesses at the cause of the disparity were that the structure of the sites could have changed slightly since training, we could have been put into different experimentation buckets, or that vectorization missed some meaningful attribute. But please correct me if I'm wrong, it's entirely possible that I've missed something.

Comment 12

[Erik Rose [:erik][:erikrose]]

Hi, Severin!

Hmm, that is funny. I find it unlikely that those particular sites would have all changed in the past couple of weeks, but it's possible. You could re-capture and diff them; that's one of the first things I'd try. Are there relevant experimentation buckets? That would be an attractively simple explanation if true. I'm not sure what you mean by vectorization missing a meaningful attribute; maybe you just mean there's a delicious new rule we should write, which is always possible.

Comment 13

[:severin Rudie]

Are there relevant experimentation buckets?

Difficult to tell from our side, unfortunately. But it's probably a fair assumption that at least Facebook is constantly running experiments, and not unfathomable (I swear that was unintentional) that it could affect the flow we were testing.

I'm not sure what you mean by vectorization missing a meaningful attribute

I could be confusing terminology. IIRC, when I previously worked on a model the vectorization process created artifacts that were very similar to but not exactly the same as the input pages (e.g., I believe they were locked to a specific size). Is it possible that some information important to the model was lost in that process? For example, locking the size causing it to fall into a different media query, moving where two elements lie relative to one another?

Comment 14

[Erik Rose [:erik][:erikrose]]

Sorry, I thought you meant experimentation buckets in FF itself.

locked to a specific size

We do take care to collect the pages at 1366x768 and vectorize them at the same, so that shouldn't be our issue.

But if you re-freeze the production pages and compare them with the ones in our corpus, you should start to hone in on the answer. Have a great weekend!

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9149222 [details]
Bug 1638187 - Update NewPasswordModel.jsm to c1e5136. r=bdanforth

P1, has tests and verified on nightly, developer is confident that this is low risk, appoved for uplift before RC, thanks.

Comment 16

[Sebastian Hengst [:aryx] (needinfo on intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/df12abb56dd5

Comment 17

[Timea Cernea [:tbabos]]

Verified-fixed on latest RC 70.0 (20200525134724) on Windows 10 x64 for the cases mentioned in Bug 1629899 and Bug 1629909.
Bug 1629901 has the same intermittent results as mentioned in Comment 5.

Aside from the sites in the bugs mentioned above, here are the results of the ones mentioned by Erik:

• facebook.com recovery form - not fixed on latest Nightly and RC.
• perfumania.com - confirmed fixed (updates made in Bug 1629913)
• facebook.com change form - not fixed on latest Nightly and RC.
• bitribe.com - not fixed false positive case