Bug 1285328: Part 1. Make sure the first button/escape is a no-op in these dialogs, and in the potential data loss case, make the default also a no-op. Refactor some common code into preferences.js. r?jaws draft
authorMilan Sreckovic <milan@mozilla.com>
Thu, 14 Jul 2016 13:55:07 -0400
changeset 387707 65429d7359fd207e090b219d2244e4d155263b20
parent 386693 94c926911767cbaf285badaccc65b0365ae5bae0
child 387708 92d9807cefb9dfef48b99b6eebffefcc8185ea1c
child 387709 4a62db0b531121fdfe2bd6a6e6eb494c21814258
push id23048
push usermsreckovic@mozilla.com
push dateThu, 14 Jul 2016 17:57:11 +0000
reviewersjaws
bugs1285328
milestone50.0a1
Bug 1285328: Part 1. Make sure the first button/escape is a no-op in these dialogs, and in the potential data loss case, make the default also a no-op. Refactor some common code into preferences.js. r?jaws MozReview-Commit-ID: 576kGMX08vz
browser/components/preferences/in-content/main.js
browser/components/preferences/in-content/preferences.js
browser/components/preferences/in-content/privacy.js
--- a/browser/components/preferences/in-content/main.js
+++ b/browser/components/preferences/in-content/main.js
@@ -136,44 +136,24 @@ var gMainPane = {
     } else {
       // Disabling e10s autostart
       prefsToChange = [e10sPref];
       if (e10sTempPref.value) {
        prefsToChange.push(e10sTempPref);
       }
     }
 
-    const Cc = Components.classes, Ci = Components.interfaces;
-    let brandName = document.getElementById("bundleBrand").getString("brandShortName");
-    let bundle = document.getElementById("bundlePreferences");
-    let msg = bundle.getFormattedString(e10sCheckbox.checked ?
-                                        "featureEnableRequiresRestart" : "featureDisableRequiresRestart",
-                                        [brandName]);
-    let restartText = bundle.getFormattedString("okToRestartButton", [brandName]);
-    let revertText = bundle.getString("revertNoRestartButton");
-
-    let title = bundle.getFormattedString("shouldRestartTitle", [brandName]);
-    let prompts = Cc["@mozilla.org/embedcomp/prompt-service;1"].getService(Ci.nsIPromptService);
-    let buttonFlags = (Services.prompt.BUTTON_POS_0 *
-                       Services.prompt.BUTTON_TITLE_IS_STRING) +
-                      (Services.prompt.BUTTON_POS_1 *
-                       Services.prompt.BUTTON_TITLE_IS_STRING) +
-                      Services.prompt.BUTTON_POS_0_DEFAULT;
-    let shouldProceed = prompts.confirmEx(window, title, msg,
-                                          buttonFlags, revertText, restartText,
-                                          null, null, {});
-
-    if (shouldProceed) {
+    let buttonIndex = confirmRestartPrompt(e10sCheckbox.checked, 0);
+    if (buttonIndex == CONFIRM_RESTART_PROMPT_RESTART_NOW) {
+      const Cc = Components.classes, Ci = Components.interfaces;
       let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"]
                          .createInstance(Ci.nsISupportsPRBool);
       Services.obs.notifyObservers(cancelQuit, "quit-application-requested",
                                    "restart");
-      shouldProceed = !cancelQuit.data;
-
-      if (shouldProceed) {
+      if (!cancelQuit.data) {
         for (let prefToChange of prefsToChange) {
           prefToChange.value = e10sCheckbox.checked;
         }
 
         Services.startup.quit(Ci.nsIAppStartup.eAttemptQuit |  Ci.nsIAppStartup.eRestart);
       }
     }
 
--- a/browser/components/preferences/in-content/preferences.js
+++ b/browser/components/preferences/in-content/preferences.js
@@ -169,8 +169,52 @@ function friendlyPrefCategoryNameToInter
     return aName;
   return "pane" + aName.substring(0,1).toUpperCase() + aName.substr(1);
 }
 
 // This function is duplicated inside of utilityOverlay.js's openPreferences.
 function internalPrefCategoryNameToFriendlyName(aName) {
   return (aName || "").replace(/^pane./, function(toReplace) { return toReplace[4].toLowerCase(); });
 }
+
+// Put up a confirm dialog with "ok to restart" and "revert without restarting"
+// buttons and returns the index of the button chosen.
+//
+// The constants are useful to interpret the return value of the function.
+const CONFIRM_RESTART_PROMPT_RESTART_NOW = 0;
+const CONFIRM_RESTART_PROMPT_CANCEL = 1;
+function confirmRestartPrompt(aRestartToEnable, aDefaultButtonIndex) {
+  let brandName = document.getElementById("bundleBrand").getString("brandShortName");
+  let bundle = document.getElementById("bundlePreferences");
+  let msg = bundle.getFormattedString(aRestartToEnable ?
+                                      "featureEnableRequiresRestart" :
+                                      "featureDisableRequiresRestart",
+                                      [brandName]);
+  let title = bundle.getFormattedString("shouldRestartTitle", [brandName]);
+  let prompts = Cc["@mozilla.org/embedcomp/prompt-service;1"].getService(Ci.nsIPromptService);
+
+  // Set up the first (index 0) button:
+  let button0Text = bundle.getFormattedString("okToRestartButton", [brandName]);
+  let buttonFlags = (Services.prompt.BUTTON_POS_0 *
+                     Services.prompt.BUTTON_TITLE_IS_STRING);
+
+
+  // Set up the second (index 1) button:
+  let button1Text = bundle.getString("revertNoRestartButton");
+  buttonFlags += (Services.prompt.BUTTON_POS_1 *
+                  Services.prompt.BUTTON_TITLE_IS_STRING);
+
+  switch(aDefaultButtonIndex) {
+    case 0:
+      buttonFlags += Services.prompt.BUTTON_POS_0_DEFAULT;
+      break;
+    case 1:
+      buttonFlags += Services.prompt.BUTTON_POS_1_DEFAULT;
+      break;
+    default:
+      break;
+  }
+
+  let buttonIndex = prompts.confirmEx(window, title, msg, buttonFlags,
+                                      button0Text, button1Text, null,
+                                      null, {});
+  return buttonIndex;
+}
--- a/browser/components/preferences/in-content/privacy.js
+++ b/browser/components/preferences/in-content/privacy.js
@@ -365,44 +365,24 @@ var gPrivacyPane = {
           return;
       }
 
       if (!this._shouldPromptForRestart) {
         // We're performing a revert. Just let it happen.
         return;
       }
 
-      const Cc = Components.classes, Ci = Components.interfaces;
-      let brandName = document.getElementById("bundleBrand").getString("brandShortName");
-      let bundle = document.getElementById("bundlePreferences");
-      let msg = bundle.getFormattedString(autoStart.checked ?
-                                          "featureEnableRequiresRestart" : "featureDisableRequiresRestart",
-                                          [brandName]);
-      let restartText = bundle.getFormattedString("okToRestartButton", [brandName]);
-      let revertText = bundle.getString("revertNoRestartButton");
-
-      let title = bundle.getFormattedString("shouldRestartTitle", [brandName]);
-      let prompts = Cc["@mozilla.org/embedcomp/prompt-service;1"].getService(Ci.nsIPromptService);
-      let buttonFlags = (Services.prompt.BUTTON_POS_0 *
-			 Services.prompt.BUTTON_TITLE_IS_STRING) +
-                        (Services.prompt.BUTTON_POS_1 *
-			 Services.prompt.BUTTON_TITLE_IS_STRING) +
-                        Services.prompt.BUTTON_POS_0_DEFAULT;
-
-      let shouldProceed = prompts.confirmEx(window, title, msg,
-					    buttonFlags, revertText, restartText,
-					    null, null, {});
-      if (shouldProceed) {
+      let buttonIndex = confirmRestartPrompt(autoStart.checked, 1);
+      if (buttonIndex == CONFIRM_RESTART_PROMPT_RESTART_NOW) {
+        const Cc = Components.classes, Ci = Components.interfaces;
         let cancelQuit = Cc["@mozilla.org/supports-PRBool;1"]
                            .createInstance(Ci.nsISupportsPRBool);
         Services.obs.notifyObservers(cancelQuit, "quit-application-requested",
                                      "restart");
-        shouldProceed = !cancelQuit.data;
-
-        if (shouldProceed) {
+        if (!cancelQuit.data) {
           pref.value = autoStart.hasAttribute('checked');
           let appStartup = Cc["@mozilla.org/toolkit/app-startup;1"]
                              .getService(Ci.nsIAppStartup);
           appStartup.quit(Ci.nsIAppStartup.eAttemptQuit |  Ci.nsIAppStartup.eRestart);
           return;
         }
       }