Bug 403420 - notification bar should show the host for the login it's asking to save. r=gavin, ui-r=beltzner
authorJustin Dolske <dolske@mozilla.com>
Thu, 30 Oct 2008 23:28:39 -0700
changeset 21136 e8b8677097d0b5409a815bb33778f75d07cf4dc9
parent 21135 495a10de898b45dd21b17f64025f54a3a80fdb4b
child 21137 a131999fa900eb305d4c71cbe90f2bbf8f53efc4
push id3345
push userjdolske@mozilla.com
push dateFri, 31 Oct 2008 06:28:51 +0000
treeherdermozilla-central@e8b8677097d0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin, beltzner
bugs403420
milestone1.9.1b2pre
Bug 403420 - notification bar should show the host for the login it's asking to save. r=gavin, ui-r=beltzner
toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js
toolkit/components/passwordmgr/test/Makefile.in
toolkit/components/passwordmgr/test/subtst_notifications_7.html
toolkit/components/passwordmgr/test/test_notifications.html
toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
--- a/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js
+++ b/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js
@@ -161,16 +161,31 @@ LoginManagerPrompter.prototype = {
     get _ioService() {
         if (!this.__ioService)
             this.__ioService = Cc["@mozilla.org/network/io-service;1"].
                                getService(Ci.nsIIOService);
         return this.__ioService;
     },
 
 
+    __ellipsis : null,
+    get _ellipsis() {
+        if (!this.__ellipsis) {
+            this.__ellipsis = "\u2026";
+            try {
+                var prefSvc = Cc["@mozilla.org/preferences-service;1"].
+                              getService(Ci.nsIPrefBranch);
+                this.__ellipsis = prefSvc.getComplexValue("intl.ellipsis",
+                                      Ci.nsIPrefLocalizedString).data;
+            } catch (e) { }
+        }
+        return this.__ellipsis;
+    },
+
+
     // Whether we are in private browsing mode
     get _inPrivateBrowsing() {
       // The Private Browsing service might not be available.
       try {
         var pbs = Cc["@mozilla.org/privatebrowsing;1"].
                   getService(Ci.nsIPrivateBrowsingService);
         return pbs.privateBrowsingEnabled;
       } catch (e) {
@@ -636,18 +651,37 @@ LoginManagerPrompter.prototype = {
               this._getLocalizedString("notifyBarRememberButtonAccessKey");
         var notNowButtonText =
               this._getLocalizedString("notifyBarNotNowButtonText");
         var notNowButtonAccessKey =
               this._getLocalizedString("notifyBarNotNowButtonAccessKey");
 
         var brandShortName =
               this._brandBundle.GetStringFromName("brandShortName");
-        var notificationText  = this._getLocalizedString(
-                                        "savePasswordText", [brandShortName]);
+        var displayHost = this._getShortDisplayHost(aLogin.hostname);
+        var notificationText;
+        if (aLogin.username) {
+            // A naughty site can cause this notification bar to be displayed
+            // (just self-submit a prefilled login form), so we want to limit
+            // the tricks they might play with arbitrary username strings.
+            // Limit the length, strip quotes
+            var displayUser = aLogin.username;
+            if (displayUser.length > 30) {
+                displayUser = displayUser.substring(0, 30);
+                displayUser += this._ellipsis;
+            }
+            displayUser = displayUser.replace(/['"]/g, "");
+            notificationText  = this._getLocalizedString(
+                                        "saveLoginText",
+                                        [brandShortName, displayUser, displayHost]);
+        } else {
+            notificationText  = this._getLocalizedString(
+                                        "saveLoginTextNoUsername",
+                                        [brandShortName, displayHost]);
+        }
 
         // The callbacks in |buttons| have a closure to access the variables
         // in scope here; set one to |this._pwmgr| so we can get back to pwmgr
         // without a getService() call.
         var pwmgr = this._pwmgr;
 
 
         var buttons = [
@@ -714,19 +748,38 @@ LoginManagerPrompter.prototype = {
     _showSaveLoginDialog : function (aLogin) {
         const buttonFlags = Ci.nsIPrompt.BUTTON_POS_1_DEFAULT +
             (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_0) +
             (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_1) +
             (Ci.nsIPrompt.BUTTON_TITLE_IS_STRING * Ci.nsIPrompt.BUTTON_POS_2);
 
         var brandShortName =
                 this._brandBundle.GetStringFromName("brandShortName");
+        var displayHost = this._getShortDisplayHost(aLogin.hostname);
 
-        var dialogText         = this._getLocalizedString(
-                                        "savePasswordText", [brandShortName]);
+        var dialogText;
+        if (aLogin.username) {
+            // A naughty site can cause this dialog to be displayed
+            // (just self-submit a prefilled login form), so we want to limit
+            // the tricks they might play with arbitrary username strings.
+            // Limit the length, strip quotes
+            var displayUser = aLogin.username;
+            if (displayUser.length > 30) {
+                displayUser = displayUser.substring(0, 30);
+                displayUser += this._ellipsis;
+            }
+            displayUser = displayUser.replace(/['"]/g, "");
+            dialogText = this._getLocalizedString(
+                                 "saveLoginText",
+                                 [brandShortName, displayUser, displayHost]);
+        } else {
+            dialogText = this._getLocalizedString(
+                                 "saveLoginTextNoUsername",
+                                 [brandShortName, displayHost]);
+        }
         var dialogTitle        = this._getLocalizedString(
                                         "savePasswordTitle");
         var neverButtonText    = this._getLocalizedString(
                                         "neverForSiteButtonText");
         var rememberButtonText = this._getLocalizedString(
                                         "rememberButtonText");
         var notNowButtonText   = this._getLocalizedString(
                                         "notNowButtonText");
@@ -1041,16 +1094,46 @@ LoginManagerPrompter.prototype = {
             var handler = this._ioService.getProtocolHandler(scheme);
             if (port != handler.defaultPort)
                 hostname += ":" + port;
         }
 
         return hostname;
     },
 
+
+    /*
+     * _getShortDisplayHost
+     *
+     * Converts a login's hostname field (a URL) to a short string for
+     * prompting purposes. Eg, "http://foo.com" --> "foo.com", or
+     * "ftp://www.site.co.uk" --> "site.co.uk".
+     */
+    _getShortDisplayHost: function (aURIString) {
+        var displayHost;
+
+        var eTLDService = Cc["@mozilla.org/network/effective-tld-service;1"].
+                          getService(Ci.nsIEffectiveTLDService);
+        var idnService = Cc["@mozilla.org/network/idn-service;1"].
+                         getService(Ci.nsIIDNService);
+        try {
+            var uri = this._ioService.newURI(aURIString, null, null);
+            var baseDomain = eTLDService.getBaseDomain(uri);
+            displayHost = idnService.convertToDisplayIDN(baseDomain, {});
+        } catch (e) {
+            this.log("_getShortDisplayHost couldn't process " + aURIString);
+        }
+
+        if (!displayHost)
+            displayHost = aURIString;
+
+        return displayHost;
+    },
+
+
     /*
      * _getAuthTarget
      *
      * Returns the hostname and realm for which authentication is being
      * requested, in the format expected to be used with nsILoginInfo.
      */
     _getAuthTarget : function (aChannel, aAuthInfo) {
         var hostname, realm;
--- a/toolkit/components/passwordmgr/test/Makefile.in
+++ b/toolkit/components/passwordmgr/test/Makefile.in
@@ -86,16 +86,17 @@ MOCHI_CONTENT = \
 		subtst_privbrowsing_3.html \
 		subtst_privbrowsing_4.html \
 		subtst_notifications_1.html \
 		subtst_notifications_2.html \
 		subtst_notifications_3.html \
 		subtst_notifications_4.html \
 		subtst_notifications_5.html \
 		subtst_notifications_6.html \
+		subtst_notifications_7.html \
 		subtst_notifications_8.html \
 		subtst_notifications_9.html \
 		subtst_notifications_10.html \
 		$(NULL)
 
 XPCSHELL_TESTS  = unit
 
 # This test doesn't pass because we can't ensure a cross-platform
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/subtst_notifications_7.html
@@ -0,0 +1,27 @@
+<html>
+<head>
+  <title>Subtest for Login Manager notifications</title>
+</head>
+<body>
+<h2>Subtest 7</h2>
+<form id="form" action="formsubmit.sjs">
+  <input id="user" name="user">
+  <input id="pass" name="pass" type="password">
+  <button type='submit'>Submit</button>
+</form>
+
+<script>
+function submitForm() {
+  userField.value = "nowisthetimeforallgoodmentocometotheaidoftheircountry";
+  passField.value = "notifyp1";
+  form.submit();
+}
+
+window.onload = submitForm;
+var form      = document.getElementById("form");
+var userField = document.getElementById("user");
+var passField = document.getElementById("pass");
+
+</script>
+</body>
+</html>
--- a/toolkit/components/passwordmgr/test/test_notifications.html
+++ b/toolkit/components/passwordmgr/test/test_notifications.html
@@ -16,16 +16,20 @@ Login Manager test: notifications
   <iframe id="iframe"></iframe>
 </div>
 
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
 /** Test for Login Manager: notifications. **/
 
+// Set testpath to the directory where we live. Used to load tests from
+// alternate Mochitest servers (different hostnames, same content).
+var testpath = document.location.pathname + "/../";
+
 var subtests = [
                    "subtst_notifications_1.html", // 1
                    "subtst_notifications_1.html", // 2
                    "subtst_notifications_1.html", // 3
                    "subtst_notifications_1.html", // 4
                    "subtst_notifications_1.html", // 5
                    "subtst_notifications_1.html", // 6
                    "subtst_notifications_1.html", // 7
@@ -36,17 +40,20 @@ var subtests = [
                    "subtst_notifications_5.html", // 12
                    "subtst_notifications_1.html", // 13
                    "subtst_notifications_6.html", // 14
                    "subtst_notifications_1.html", // 15
                    "subtst_notifications_6.html", // 16
                    "subtst_notifications_8.html", // 17
                    "subtst_notifications_8.html", // 18
                    "subtst_notifications_9.html", // 19
-                   "subtst_notifications_10.html"  // 20
+                   "subtst_notifications_10.html",  // 20
+                   "http://test1.example.org:80" + testpath + "subtst_notifications_1.html", // 21
+                   "http://test1.example.org:80" + testpath + "subtst_notifications_7.html", // 22
+                   "http://test1.example.org:80" + testpath + "subtst_notifications_6.html", // 23
                ];
 
 
 var ignoreLoad = false;
 function handleLoad(aEvent) {
     netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
 
     // ignore every other load event ... We get one for loading the subtest (which
@@ -74,17 +81,17 @@ function handleLoad(aEvent) {
         ok(true, "notification tests finished.");
         SimpleTest.finish();
     }
 }
 
 
 // Remember, Never for This Site, Not Now
 function checkTest() {
-    var bar;
+    var bar, notificationText, expectedText;
 
     // The document generated from formsubmit.sjs contains the user/pass it
     // received inside <span id="blah">value</span>
     var gotUser = iframe.contentDocument.getElementById("user").textContent;
     var gotPass = iframe.contentDocument.getElementById("pass").textContent;
 
 
     switch(testNum) {
@@ -307,16 +314,55 @@ function checkTest() {
         is(gotPass, "notifyp1", "Checking submitted password");
         bar = getNotificationBar(notifyBox, "password-change");
         ok(bar, "got notification bar");
         clickNotificationButton(bar, "Change");
 
         pwmgr.removeLogin(login2);
         break;
 
+      case 21:
+        // Check text on a user+pass notification bar
+        is(gotUser, "notifyu1", "Checking submitted username");
+        is(gotPass, "notifyp1", "Checking submitted password");
+        bar = getNotificationBar(notifyBox, "password-save");
+        ok(bar, "got notification bar");
+        // Check the text, which comes from the localized saveLoginText string.
+        notificationText = bar.getAttribute("label");
+        expectedText = /^Do you want .+ to remember the password for \"notifyu1\" on example.org\?$/;
+        ok(expectedText.test(notificationText), "Checking text: " + notificationText);
+        clickNotificationButton(bar, "Not Now");
+        break;
+
+      case 22:
+        // Check text on a user+pass notification bar, username is really long
+        is(gotUser, "nowisthetimeforallgoodmentocometotheaidoftheircountry", "Checking submitted username");
+        is(gotPass, "notifyp1", "Checking submitted password");
+        bar = getNotificationBar(notifyBox, "password-save");
+        ok(bar, "got notification bar");
+        // Check the text, which comes from the localized saveLoginText string.
+        notificationText = bar.getAttribute("label");
+        expectedText = /^Do you want .+ to remember the password for \"nowisthetimeforallgoodmentocom[^e]\" on example.org\?$/;
+        ok(expectedText.test(notificationText), "Checking text: " + notificationText);
+        clickNotificationButton(bar, "Not Now");
+        break;
+
+      case 23:
+        // Check text on a pass-only notification bar
+        is(gotUser, "null",     "Checking submitted username");
+        is(gotPass, "notifyp1", "Checking submitted password");
+        bar = getNotificationBar(notifyBox, "password-save");
+        ok(bar, "got notification bar");
+        // Check the text, which comes from the localized saveLoginTextNoUser string.
+        notificationText = bar.getAttribute("label");
+        expectedText = /^Do you want .+ to remember this password on example.org\?$/;
+        ok(expectedText.test(notificationText), "Checking text: " + notificationText);
+        clickNotificationButton(bar, "Not Now");
+        break;
+
       default:
         ok(false, "Unexpected call to checkTest for test #" + testNum);
 
     }
 
     // TODO:
     // * existing login test, form has different password --> change password, no save prompt
 }
--- a/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
+++ b/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
@@ -34,17 +34,21 @@
 # the provisions above, a recipient may use your version of this file under
 # the terms of any one of the MPL, the GPL or the LGPL.
 #
 # ***** END LICENSE BLOCK *****
 
 rememberValue = Use Password Manager to remember this value.
 rememberPassword = Use Password Manager to remember this password.
 savePasswordTitle = Confirm
-savePasswordText = Do you want %S to remember this password?
+# 1st string is product name, 2nd is the username for the login, 3rd is the
+#   login's hostname. Note that long usernames may be truncated.
+saveLoginText = Do you want %1$S to remember the password for "%2$S" on %3$S?
+# 1st string is product name, 2nd is the login's hostname
+saveLoginTextNoUsername = Do you want %1$S to remember this password on %2$S?
 notNowButtonText = &Not Now
 notifyBarNotNowButtonText = Not Now
 notifyBarNotNowButtonAccessKey = N
 neverForSiteButtonText = Ne&ver for This Site
 notifyBarNeverForSiteButtonText = Never for This Site
 notifyBarNeverForSiteButtonAccessKey = e
 rememberButtonText = &Remember
 notifyBarRememberButtonText = Remember