Bug 355063 - Password manager does not work on script-generated forms, should use new DOMFormHasPassword event instead. r=mattn

browser/base/content/content.js

@@ -48,6 +48,7 @@ if (Services.prefs.getBoolPref("browser.tabs.remote")) {
   });
   addEventListener("DOMFormHasPassword", function(event) {
     InsecurePasswordUtils.checkForInsecurePasswords(event.target);
+    LoginManagerContent.onFormPassword(event);
   });
   addEventListener("DOMAutoComplete", function(event) {
     LoginManagerContent.onUsernameInput(event);

modules/libpref/src/init/all.js

@@ -3909,6 +3909,7 @@ pref("signon.SignonFileName3",              "signons3.txt"); // obsolete
 pref("signon.autofillForms",                true);
 pref("signon.autologin.proxy",              false);
 pref("signon.debug",                        false);
+pref("signon.useDOMFormHasPassword",        true);
 
 // Satchel (Form Manager) prefs
 pref("browser.formfill.debug",            false);

toolkit/components/passwordmgr/LoginManagerContent.jsm

@@ -13,7 +13,8 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
 
-var gEnabled = false, gDebug = false; // these mirror signon.* prefs
+var gEnabled = false, gDebug = false, gAutofillForms = true; // these mirror signon.* prefs
+var gUseDOMFormHasPassword = false; // use DOMFormHasPassword event for autofill
 
 function log(...pieces) {
     function generateLogMessage(args) {
@@ -70,6 +71,8 @@ var observer = {
     onPrefChange : function() {
         gDebug = Services.prefs.getBoolPref("signon.debug");
         gEnabled = Services.prefs.getBoolPref("signon.rememberSignons");
+        gAutofillForms = Services.prefs.getBoolPref("signon.autofillForms");
+        gUseDOMFormHasPassword = Services.prefs.getBoolPref("signon.useDOMFormHasPassword");
     },
 };
 
@@ -92,6 +95,10 @@ var LoginManagerContent = {
     },
 
     onContentLoaded : function (event) {
+      // If we're using the new DOMFormHasPassword event, don't fill at pageload.
+      if (gUseDOMFormHasPassword)
+          return;
+
       if (!event.isTrusted)
           return;
 
@@ -108,6 +115,64 @@ var LoginManagerContent = {
     },
 
 
+    onFormPassword: function (event) {
+      // If we're not using the new DOMFormHasPassword event, only fill at pageload.
+      if (!gUseDOMFormHasPassword)
+          return;
+
+      if (!event.isTrusted)
+          return;
+
+      if (!gEnabled)
+          return;
+
+      let form = event.target;
+      let doc = form.ownerDocument;
+
+      log("onFormPassword for", doc.documentURI);
+
+      // If there are no logins for this site, bail out now.
+      let formOrigin = LoginUtils._getPasswordOrigin(doc.documentURI);
+      if (!Services.logins.countLogins(formOrigin, "", null))
+          return;
+
+      // If we're currently displaying a master password prompt, defer
+      // processing this form until the user handles the prompt.
+      if (Services.logins.uiBusy) {
+        log("deferring onFormPassword for", doc.documentURI);
+        let self = this;
+        let observer = {
+            QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
+
+            observe: function (subject, topic, data) {
+                log("Got deferred onFormPassword notification:", topic);
+                // Only run observer once.
+                Services.obs.removeObserver(this, "passwordmgr-crypto-login");
+                Services.obs.removeObserver(this, "passwordmgr-crypto-loginCanceled");
+                if (topic == "passwordmgr-crypto-loginCanceled")
+                    return;
+                self.onFormPassword(event);
+            },
+            handleEvent : function (event) {
+                // Not expected to be called
+            }
+        };
+        // Trickyness follows: We want an observer, but don't want it to
+        // cause leaks. So add the observer with a weak reference, and use
+        // a dummy event listener (a strong reference) to keep it alive
+        // until the form is destroyed.
+        Services.obs.addObserver(observer, "passwordmgr-crypto-login", true);
+        Services.obs.addObserver(observer, "passwordmgr-crypto-loginCanceled", true);
+        form.addEventListener("mozCleverClosureHack", observer);
+        return;
+      }
+
+      let autofillForm = gAutofillForms && !PrivateBrowsingUtils.isWindowPrivate(doc.defaultView);
+
+      this._fillForm(form, autofillForm, false, false, null);
+    },
+
+
     /*
      * onUsernameInput
      *
@@ -537,8 +602,7 @@ var LoginManagerContent = {
 
         log("fillDocument processing", forms.length, "forms on", doc.documentURI);
 
-        var autofillForm = !PrivateBrowsingUtils.isWindowPrivate(doc.defaultView) &&
-                           Services.prefs.getBoolPref("signon.autofillForms");
+        var autofillForm = gAutofillForms && !PrivateBrowsingUtils.isWindowPrivate(doc.defaultView);
         var previousActionOrigin = null;
         var foundLogins = null;
 

toolkit/components/passwordmgr/test/Makefile.in

@@ -27,6 +27,7 @@ MOCHITEST_FILES = \
     test_basic_form_observer_autocomplete.html \
     test_basic_form_observer_foundLogins.html \
     test_basic_form_pwonly.html \
+    test_basic_form_pwevent.html \
     test_bug_227640.html \
     test_bug_242956.html \
     test_bug_360493_1.html \

toolkit/components/passwordmgr/test/test_basic_form_observer_autofillForms.html

@@ -21,6 +21,36 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 // Assume that the pref starts out true, so set to false
 SpecialPowers.setBoolPref("signon.autofillForms", false);
+
+var TestObserver = {
+  receivedNotificationFoundForm : false,
+  receivedNotificationFoundLogins : false,
+  dataFoundForm : "",
+  dataFoundLogins : null,
+  QueryInterface : XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
+  observe : function (subject, topic, data) {
+    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+    var pwmgr = Cc["@mozilla.org/login-manager;1"].
+                getService(Ci.nsILoginManager);
+    if (topic == "passwordmgr-found-form") {
+      info("got passwordmgr-found-form");
+      this.receivedNotificationFoundForm = true;
+      this.dataFoundForm = data;
+      // Now fill the form
+      pwmgr.fillForm(subject);
+    } else if (topic == "passwordmgr-found-logins") {
+      info("got passwordmgr-found-logins");
+      this.receivedNotificationFoundLogins = true;
+      this.dataFoundLogins = subject.QueryInterface(Ci.nsIPropertyBag2);
+    }
+  }
+};
+
+// Add the observer
+var os = Cc["@mozilla.org/observer-service;1"].
+         getService(Ci.nsIObserverService);
+os.addObserver(TestObserver, "passwordmgr-found-form", false);
+os.addObserver(TestObserver, "passwordmgr-found-logins", false);
 </script>
 
 <p id="display"></p>
@@ -42,35 +72,6 @@ SpecialPowers.setBoolPref("signon.autofillForms", false);
 <script class="testbody" type="text/javascript">
 
 /** Test for Login Manager: simple form with autofillForms disabled and notifying observers **/
-
-var TestObserver = {
-  receivedNotificationFoundForm : false,
-  receivedNotificationFoundLogins : false,
-  dataFoundForm : "",
-  dataFoundLogins : null,
-  QueryInterface : XPCOMUtils.generateQI([Ci.nsIObserver, Ci.nsISupportsWeakReference]),
-  observe : function (subject, topic, data) {
-    netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
-    var pwmgr = Cc["@mozilla.org/login-manager;1"].
-                getService(Ci.nsILoginManager);
-    if (topic == "passwordmgr-found-form") {
-      this.receivedNotificationFoundForm = true;
-      this.dataFoundForm = data;
-      // Now fill the form
-      pwmgr.fillForm(subject);
-    } else if (topic == "passwordmgr-found-logins") {
-      this.receivedNotificationFoundLogins = true;
-      this.dataFoundLogins = subject.QueryInterface(Ci.nsIPropertyBag2);
-    }
-  }
-};
-
-// Add the observer
-var os = Cc["@mozilla.org/observer-service;1"].
-         getService(Ci.nsIObserverService);
-os.addObserver(TestObserver, "passwordmgr-found-form", false);
-os.addObserver(TestObserver, "passwordmgr-found-logins", false);
-
 function startTest(){
   netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
   // Test that found-form observer is notified & got correct data

toolkit/components/passwordmgr/test/test_basic_form_pwevent.html

@@ -0,0 +1,66 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=355063
+-->
+<head>
+  <meta charset="utf-8"/>
+  <title>Test for Bug 355063</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="text/javascript" src="pwmgr_common.js"></script>
+  <script type="application/javascript">
+  /** Test for Bug 355063 **/
+
+  function startTest() {
+    info("startTest");
+    addForm();
+  }
+
+  function addForm() {
+    info("addForm");
+    var c = document.getElementById("content");
+    c.innerHTML = "<form id=form1>form1: <input id=u1><input type=password id=p1></form><br>";
+  }
+
+  function checkForm() {
+    info("checkForm");
+    var userField = document.getElementById("u1");
+    var passField = document.getElementById("p1");
+    is(userField.value, "testuser", "checking filled username");
+    is(passField.value, "testpass", "checking filled password");
+
+    chromeWin.gBrowser.removeEventListener("DOMFormHasPassword", checkForm);
+    SimpleTest.finish();
+  }
+
+  if (SpecialPowers.getBoolPref("signon.useDOMFormHasPassword")) {
+    commonInit();
+
+    // Password Manager's own listener should always have been added first, so
+    // the test's listener should be called after the pwmgr's listener fills in
+    // a login.
+    //
+    var chromeWin = SpecialPowers.wrap(window)
+                                 .QueryInterface(SpecialPowers.Ci.nsIInterfaceRequestor)
+                                 .getInterface(SpecialPowers.Ci.nsIWebNavigation)
+                                 .QueryInterface(SpecialPowers.Ci.nsIDocShell)
+                                 .chromeEventHandler.ownerDocument.defaultView;
+    chromeWin.gBrowser.addEventListener("DOMFormHasPassword", checkForm);
+    window.addEventListener("load", startTest);
+    SimpleTest.waitForExplicitFinish();
+  } else {
+    ok(true, "Skipping test when useDOMFormHasPassword is disabled");
+  }
+</script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=355063">Mozilla Bug 355063</a>
+<p id="display"></p>
+<div id="content">
+forms go here!
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>

toolkit/components/passwordmgr/test/test_maxforms_1.html

@@ -52,7 +52,12 @@ function startTest() {
   SimpleTest.finish();
 }
 
-window.onload = startTest;
+if (SpecialPowers.getBoolPref("signon.useDOMFormHasPassword")) {
+  info("skipping test when signon.useDOMFormHasPassword is enabled");
+  SimpleTest.finish();
+} else {
+  window.onload = startTest;
+}
 </script>
 </pre>
 </body>

toolkit/components/passwordmgr/test/test_maxforms_2.html

@@ -52,7 +52,12 @@ function startTest() {
   SimpleTest.finish();
 }
 
-window.onload = startTest;
+if (SpecialPowers.getBoolPref("signon.useDOMFormHasPassword")) {
+  info("skipping test when signon.useDOMFormHasPassword is enabled");
+  SimpleTest.finish();
+} else {
+  window.onload = startTest;
+}
 </script>
 </pre>
 </body>

toolkit/components/passwordmgr/test/test_maxforms_3.html

@@ -52,7 +52,12 @@ function startTest() {
   SimpleTest.finish();
 }
 
-window.onload = startTest;
+if (SpecialPowers.getBoolPref("signon.useDOMFormHasPassword")) {
+  info("skipping test when signon.useDOMFormHasPassword is enabled");
+  SimpleTest.finish();
+} else {
+  window.onload = startTest;
+}
 </script>
 </pre>
 </body>