The change below was incorrect since countLogins would ignore the "" for formSubmitURL but searchLogins does not and therefore this only returns form logins that literally have "" for the saved action. https://hg.mozilla.org/mozilla-central/diff/da82605a18d6/toolkit/components/passwordmgr/LoginManagerParent.jsm#l1.120 The search should specify `httpRealm: null` instead of `formSubmitURL: ""` but this reveals an issue in that the MP prompt appears when a login is found. This is probably a real user-facing issue and is caught by test_master_password.html with this fix.

It's possible to write a test for this but that's already covered by bug 1164018 and this code should be removed in bug 1286718. Review commit: https://reviewboard.mozilla.org/r/68690/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68690/

Comment on attachment 8777102 [details] Bug 1291346 - Fix updateLoginAnchor's search for form logins and delay it until the pref check. https://reviewboard.mozilla.org/r/68690/#review66128 ::: toolkit/components/passwordmgr/LoginManagerParent.jsm:520 (Diff revision 1) > + // Once this preference is removed, this version of the fill doorhanger > + // should be enabled for Desktop only, and not for Android or B2G. > + if (!Services.prefs.getBoolPref("signon.ui.experimental")) { > + return; Any reason not to just move the pref check to the very top of the function?

https://reviewboard.mozilla.org/r/68690/#review66128 > Any reason not to just move the pref check to the very top of the function? Nope, I guess not. Fixed.

Comment on attachment 8777102 [details] Bug 1291346 - Fix updateLoginAnchor's search for form logins and delay it until the pref check. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68690/diff/1-2/

Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/fx-team/rev/f54fea6078dd Fix updateLoginAnchor's search for form logins and delay it until the pref check. r=dolske

Matt, can you request uplift for this fix? Thanks!

Comment on attachment 8777102 [details] Bug 1291346 - Fix updateLoginAnchor's search for form logins and delay it until the pref check. Approval Request Comment [Feature/regressing bug #]: Bug 667233 [User impact if declined]: Useless work done when a non-default feature is disabled. Non-default feature would be missing most logins if it was enabled (not recommended) [Describe test coverage new/current, TreeHerder]: It's possible to write a test for this but that's already covered by bug 1164018 and this code should be removed in bug 1286718 so I didn't write a new test. It's also a trivial change. [Risks and why]: Low risk trivial changes and automated tests for other functionality didn't seem to regress. [String/UUID change made/needed]: N/A

Comment on attachment 8777102 [details] Bug 1291346 - Fix updateLoginAnchor's search for form logins and delay it until the pref check. Fixes a regression, Aurora50+, Beta49+