Bug 1438499 - show 'close multiple tabs' warning dialog when quitting, r?jaws,whimboo draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 13 Jun 2018 09:34:52 -0700
changeset 810471 4062ceb3aacea6b42e30890f990fcb56b8b08a00
parent 810453 0fcf079101de5648a6b05b4938a0e30c5ba8d5d9
push id114008
push userbmo:gijskruitbosch+bugs@gmail.com
push dateMon, 25 Jun 2018 22:58:23 +0000
reviewersjaws, whimboo
bugs1438499
milestone63.0a1
Bug 1438499 - show 'close multiple tabs' warning dialog when quitting, r?jaws,whimboo MozReview-Commit-ID: J2gi9do8HK9
browser/app/profile/firefox.js
browser/base/content/tabbrowser.js
browser/components/nsBrowserGlue.js
browser/locales/en-US/chrome/browser/quitDialog.properties
browser/locales/en-US/chrome/browser/tabbrowser.properties
browser/locales/jar.mn
testing/geckodriver/src/prefs.rs
testing/marionette/components/marionette.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -266,19 +266,16 @@ pref("browser.slowStartup.maxSamples", 5
 // repackager of this code using an alternate snippet url, please keep your users safe
 pref("browser.aboutHomeSnippets.updateUrl", "https://snippets.cdn.mozilla.net/%STARTPAGE_VERSION%/%NAME%/%VERSION%/%APPBUILDID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/");
 
 pref("browser.enable_automatic_image_resizing", true);
 pref("browser.chrome.site_icons", true);
 pref("browser.chrome.favicons", true);
 // browser.warnOnQuit == false will override all other possible prompts when quitting or restarting
 pref("browser.warnOnQuit", true);
-// browser.showQuitWarning specifically controls the quit warning dialog. We
-// might still show the window closing dialog with showQuitWarning == false.
-pref("browser.showQuitWarning", false);
 pref("browser.fullscreen.autohide", true);
 pref("browser.overlink-delay", 80);
 
 #ifdef UNIX_BUT_NOT_MAC
 pref("browser.urlbar.clickSelectsAll", false);
 #else
 pref("browser.urlbar.clickSelectsAll", true);
 #endif
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -2520,17 +2520,17 @@ window._gBrowser = {
     // Additionally send pinned tab events
     if (aPinned) {
       this._notifyPinnedStatus(t);
     }
 
     return t;
   },
 
-  warnAboutClosingTabs(aCloseTabs, aTab) {
+  warnAboutClosingTabs(aCloseTabs, aTab, aOptionalMessage) {
     var tabsToClose;
     switch (aCloseTabs) {
       case this.closingTabsEnum.ALL:
         tabsToClose = this.tabs.length - this._removingTabs.length -
           gBrowser._numPinnedTabs;
         break;
       case this.closingTabsEnum.OTHER:
         tabsToClose = this.visibleTabs.length - 1 - gBrowser._numPinnedTabs;
@@ -2563,19 +2563,24 @@ window._gBrowser = {
     var warnOnClose = { value: true };
 
     // focus the window before prompting.
     // this will raise any minimized window, which will
     // make it obvious which window the prompt is for and will
     // solve the problem of windows "obscuring" the prompt.
     // see bug #350299 for more details
     window.focus();
-    var warningMessage =
-      PluralForm.get(tabsToClose, gTabBrowserBundle.GetStringFromName("tabs.closeWarningMultiple"))
-      .replace("#1", tabsToClose);
+    var warningMessage;
+    if (aOptionalMessage) {
+      warningMessage = aOptionalMessage;
+    } else {
+      warningMessage =
+        PluralForm.get(tabsToClose, gTabBrowserBundle.GetStringFromName("tabs.closeWarningMultiple"))
+          .replace("#1", tabsToClose);
+    }
     var buttonPressed =
       ps.confirmEx(window,
         gTabBrowserBundle.GetStringFromName("tabs.closeWarningTitle"),
         warningMessage,
         (ps.BUTTON_TITLE_IS_STRING * ps.BUTTON_POS_0) +
         (ps.BUTTON_TITLE_CANCEL * ps.BUTTON_POS_1),
         gTabBrowserBundle.GetStringFromName("tabs.closeButtonMultiple"),
         null, null,
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -175,16 +175,20 @@ if (AppConstants.MOZ_CRASHREPORTER) {
 XPCOMUtils.defineLazyGetter(this, "gBrandBundle", function() {
   return Services.strings.createBundle("chrome://branding/locale/brand.properties");
 });
 
 XPCOMUtils.defineLazyGetter(this, "gBrowserBundle", function() {
   return Services.strings.createBundle("chrome://browser/locale/browser.properties");
 });
 
+XPCOMUtils.defineLazyGetter(this, "gTabbrowserBundle", function() {
+  return Services.strings.createBundle("chrome://browser/locale/tabbrowser.properties");
+});
+
 const global = this;
 
 const listeners = {
   observers: {
     "update-staged": ["UpdateListener"],
     "update-downloaded": ["UpdateListener"],
     "update-available": ["UpdateListener"],
     "update-error": ["UpdateListener"],
@@ -1360,132 +1364,69 @@ BrowserGlue.prototype = {
     // There are several cases where we won't show a dialog here:
     // 1. There is only 1 tab open in 1 window
     // 2. The session will be restored at startup, indicated by
     //    browser.startup.page == 3 or browser.sessionstore.resume_session_once == true
     // 3. browser.warnOnQuit == false
     // 4. The browser is currently in Private Browsing mode
     // 5. The browser will be restarted.
     //
-    // Otherwise these are the conditions and the associated dialogs that will be shown:
-    // 1. aQuitType == "lastwindow" or "quit" and browser.showQuitWarning == true
-    //    - The quit dialog will be shown
-    // 2. aQuitType == "lastwindow" && browser.tabs.warnOnClose == true
-    //    - The "closing multiple tabs" dialog will be shown
+    // Otherwise, we will show the "closing multiple tabs" dialog.
     //
     // aQuitType == "lastwindow" is overloaded. "lastwindow" is used to indicate
     // "the last window is closing but we're not quitting (a non-browser window is open)"
     // and also "we're quitting by closing the last window".
 
     if (aQuitType == "restart" || aQuitType == "os-restart")
       return;
 
     var windowcount = 0;
     var pagecount = 0;
-    var browserEnum = Services.wm.getEnumerator("navigator:browser");
-    let allWindowsPrivate = true;
-    while (browserEnum.hasMoreElements()) {
-      // XXXbz should we skip closed windows here?
+    for (let win of BrowserWindowTracker.orderedWindows) {
+      if (win.closed) {
+        continue;
+      }
       windowcount++;
-
-      var browser = browserEnum.getNext();
-      if (!PrivateBrowsingUtils.isWindowPrivate(browser))
-        allWindowsPrivate = false;
-      var tabbrowser = browser.ownerGlobal.gBrowser;
+      let tabbrowser = win.gBrowser;
       if (tabbrowser)
         pagecount += tabbrowser.browsers.length - tabbrowser._numPinnedTabs;
     }
 
-    this._saveSession = false;
     if (pagecount < 2)
       return;
 
     if (!aQuitType)
       aQuitType = "quit";
 
     // browser.warnOnQuit is a hidden global boolean to override all quit prompts
     // browser.showQuitWarning specifically covers quitting
     // browser.tabs.warnOnClose is the global "warn when closing multiple tabs" pref
 
     var sessionWillBeRestored = Services.prefs.getIntPref("browser.startup.page") == 3 ||
                                 Services.prefs.getBoolPref("browser.sessionstore.resume_session_once");
-    if (sessionWillBeRestored || !Services.prefs.getBoolPref("browser.warnOnQuit"))
-      return;
-
-    let win = Services.wm.getMostRecentWindow("navigator:browser");
-
-    // On last window close or quit && showQuitWarning, we want to show the
-    // quit warning.
-    if (!Services.prefs.getBoolPref("browser.showQuitWarning")) {
-      if (aQuitType == "lastwindow") {
-        // If aQuitType is "lastwindow" and we aren't showing the quit warning,
-        // we should show the window closing warning instead. warnAboutClosing
-        // tabs checks browser.tabs.warnOnClose and returns if it's ok to close
-        // the window. It doesn't actually close the window.
-        aCancelQuit.data =
-          !win.gBrowser.warnAboutClosingTabs(win.gBrowser.closingTabsEnum.ALL);
-      }
+    if (sessionWillBeRestored || !Services.prefs.getBoolPref("browser.warnOnQuit") ||
+        !Services.prefs.getBoolPref("browser.tabs.warnOnClose"))
       return;
-    }
-
-    let prompt = Services.prompt;
-    let quitBundle = Services.strings.createBundle("chrome://browser/locale/quitDialog.properties");
-    let appName = gBrandBundle.GetStringFromName("brandShortName");
-    let quitDialogTitle = quitBundle.formatStringFromName("quitDialogTitle",
-                                                          [appName], 1);
-    let neverAskText = quitBundle.GetStringFromName("neverAsk2");
-    let neverAsk = {value: false};
-
-    let choice;
-    if (allWindowsPrivate) {
-      let text = quitBundle.formatStringFromName("messagePrivate", [appName], 1);
-      let flags = prompt.BUTTON_TITLE_IS_STRING * prompt.BUTTON_POS_0 +
-                  prompt.BUTTON_TITLE_IS_STRING * prompt.BUTTON_POS_1 +
-                  prompt.BUTTON_POS_0_DEFAULT;
-      choice = prompt.confirmEx(win, quitDialogTitle, text, flags,
-                                quitBundle.GetStringFromName("quitTitle"),
-                                quitBundle.GetStringFromName("cancelTitle"),
-                                null,
-                                neverAskText, neverAsk);
-
-      // The order of the buttons differs between the prompt.confirmEx calls
-      // here so we need to fix this for proper handling below.
-      if (choice == 0) {
-        choice = 2;
-      }
+
+    let win = BrowserWindowTracker.getTopWindow();
+
+    // warnAboutClosingTabs checks browser.tabs.warnOnClose and returns if it's
+    // ok to close the window. It doesn't actually close the window.
+    if (windowcount == 1) {
+      aCancelQuit.data =
+        !win.gBrowser.warnAboutClosingTabs(win.gBrowser.closingTabsEnum.ALL);
     } else {
-      let text = quitBundle.formatStringFromName(
-        windowcount == 1 ? "messageNoWindows" : "message", [appName], 1);
-      let flags = prompt.BUTTON_TITLE_IS_STRING * prompt.BUTTON_POS_0 +
-                  prompt.BUTTON_TITLE_IS_STRING * prompt.BUTTON_POS_1 +
-                  prompt.BUTTON_TITLE_IS_STRING * prompt.BUTTON_POS_2 +
-                  prompt.BUTTON_POS_0_DEFAULT;
-      choice = prompt.confirmEx(win, quitDialogTitle, text, flags,
-                                quitBundle.GetStringFromName("saveTitle"),
-                                quitBundle.GetStringFromName("cancelTitle"),
-                                quitBundle.GetStringFromName("quitTitle"),
-                                neverAskText, neverAsk);
-    }
-
-    switch (choice) {
-    case 2: // Quit
-      if (neverAsk.value)
-        Services.prefs.setBoolPref("browser.showQuitWarning", false);
-      break;
-    case 1: // Cancel
-      aCancelQuit.QueryInterface(Ci.nsISupportsPRBool);
-      aCancelQuit.data = true;
-      break;
-    case 0: // Save & Quit
-      this._saveSession = true;
-      if (neverAsk.value) {
-        // always save state when shutting down
-        Services.prefs.setIntPref("browser.startup.page", 3);
-      }
-      break;
+      // More than 1 window. Compose our own message.
+      let tabSubstring = gTabbrowserBundle.GetStringFromName("tabs.closeWarningMultipleWindowsTabSnippet");
+      tabSubstring = PluralForm.get(pagecount, tabSubstring).replace(/#1/, pagecount);
+      let windowString = gTabbrowserBundle.GetStringFromName("tabs.closeWarningMultipleWindows");
+      windowString = PluralForm.get(windowcount, windowString).replace(/#1/, windowcount);
+      windowString = windowString.replace(/%(?:1$)?S/i, tabSubstring);
+      aCancelQuit.data =
+        !win.gBrowser.warnAboutClosingTabs(win.gBrowser.closingTabsEnum.ALL, null, windowString);
     }
   },
 
   _showUpdateNotification: function BG__showUpdateNotification() {
     Services.prefs.clearUserPref("app.update.postupdate");
 
     var um = Cc["@mozilla.org/updates/update-manager;1"].
              getService(Ci.nsIUpdateManager);
deleted file mode 100644
--- a/browser/locales/en-US/chrome/browser/quitDialog.properties
+++ /dev/null
@@ -1,13 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-quitDialogTitle=Quit %S
-
-quitTitle=&Quit
-cancelTitle=&Cancel
-saveTitle=&Save and Quit
-neverAsk2=&Do not ask next time
-message=Do you want %S to save your tabs and windows for the next time it starts?
-messageNoWindows=Do you want %S to save your tabs for the next time it starts?
-messagePrivate=You’re in private browsing mode. Quitting %S now will discard all your open tabs and windows.
--- a/browser/locales/en-US/chrome/browser/tabbrowser.properties
+++ b/browser/locales/en-US/chrome/browser/tabbrowser.properties
@@ -11,16 +11,34 @@ tabs.closeWarningTitle=Confirm close
 # Semicolon-separated list of plural forms. See:
 # http://developer.mozilla.org/en/docs/Localization_and_Plurals
 # The singular form is not considered since this string is used only for
 # multiple tabs.
 tabs.closeWarningMultiple=;You are about to close #1 tabs. Are you sure you want to continue?
 tabs.closeButtonMultiple=Close tabs
 tabs.closeWarningPromptMe=Warn me when I attempt to close multiple tabs
 
+# LOCALIZATION NOTE (tabs.closeWarningMultipleWindows):
+# Semicolon-separated list of plural forms. See:
+# http://developer.mozilla.org/en/docs/Localization_and_Plurals
+# The singular form is not considered since this string is used only for
+# multiple windows. The %S replacement form will be replaced with the contents
+# of tabs.closeWarningMultipleWindowsTabSnippet, which will contain the number
+# of tabs in these windows.
+# Note that every one of these plural forms must contain one instance of '%S'.
+tabs.closeWarningMultipleWindows=;You are about to close #1 windows %S. Are you sure you want to continue?
+
+# LOCALIZATION NOTE (tabs.closeWarningMultipleWindowsTabSnippet):
+# Semicolon-separated list of plural forms. See:
+# http://developer.mozilla.org/en/docs/Localization_and_Plurals
+# The singular form is not considered since this string is used only for
+# multiple windows which must contain multiple tabs (in total).
+# This string will be inserted in tabs.closeWarningMultipleWindows
+tabs.closeWarningMultipleWindowsTabSnippet=;with #1 tabs
+
 tabs.closeTab.tooltip=Close tab
 # LOCALIZATION NOTE (tabs.closeSelectedTab.tooltip):
 # %S is the keyboard shortcut for closing the current tab
 tabs.closeSelectedTab.tooltip=Close tab (%S)
 # LOCALIZATION NOTE (tabs.muteAudio.tooltip):
 # %S is the keyboard shortcut for "Mute tab"
 tabs.muteAudio.tooltip=Mute tab (%S)
 # LOCALIZATION NOTE (tabs.unmuteAudio.tooltip):
--- a/browser/locales/jar.mn
+++ b/browser/locales/jar.mn
@@ -25,17 +25,16 @@
     locale/browser/browser.dtd                     (%chrome/browser/browser.dtd)
     locale/browser/baseMenuOverlay.dtd             (%chrome/browser/baseMenuOverlay.dtd)
     locale/browser/browser.properties              (%chrome/browser/browser.properties)
     locale/browser/customizableui/customizableWidgets.properties (%chrome/browser/customizableui/customizableWidgets.properties)
     locale/browser/lightweightThemes.properties    (%chrome/browser/lightweightThemes.properties)
     locale/browser/uiDensity.properties            (%chrome/browser/uiDensity.properties)
     locale/browser/pageInfo.dtd                    (%chrome/browser/pageInfo.dtd)
     locale/browser/pageInfo.properties             (%chrome/browser/pageInfo.properties)
-    locale/browser/quitDialog.properties           (%chrome/browser/quitDialog.properties)
     locale/browser/safeMode.dtd                    (%chrome/browser/safeMode.dtd)
     locale/browser/sanitize.dtd                    (%chrome/browser/sanitize.dtd)
     locale/browser/search.properties               (%chrome/browser/search.properties)
     locale/browser/siteData.properties             (%chrome/browser/siteData.properties)
     locale/browser/sitePermissions.properties      (%chrome/browser/sitePermissions.properties)
     locale/browser/setDesktopBackground.dtd        (%chrome/browser/setDesktopBackground.dtd)
     locale/browser/shellservice.properties         (%chrome/browser/shellservice.properties)
     locale/browser/tabbrowser.properties           (%chrome/browser/tabbrowser.properties)
--- a/testing/geckodriver/src/prefs.rs
+++ b/testing/geckodriver/src/prefs.rs
@@ -30,20 +30,16 @@ lazy_static! {
         ("browser.safebrowsing.phishing.enabled", Pref::new(false)),
 
         // Do not restore the last open set of tabs if the browser crashed
         ("browser.sessionstore.resume_from_crash", Pref::new(false)),
 
         // Skip check for default browser on startup
         ("browser.shell.checkDefaultBrowser", Pref::new(false)),
 
-        // Do not warn when quitting with multiple tabs
-        // TODO: Remove once minimum supported Firefox release is 61.
-        ("browser.showQuitWarning", Pref::new(false)),
-
         // Disable Android snippets
         ("browser.snippets.enabled", Pref::new(false)),
         ("browser.snippets.syncPromo.enabled", Pref::new(false)),
         ("browser.snippets.firstrunHomepage.enabled", Pref::new(false)),
 
         // Do not redirect user when a milestone upgrade of Firefox
         // is detected
         ("browser.startup.homepage_override.mstone", Pref::new("ignore")),
--- a/testing/marionette/components/marionette.js
+++ b/testing/marionette/components/marionette.js
@@ -103,19 +103,16 @@ const RECOMMENDED_PREFS = new Map([
   ["browser.sessionstore.resume_from_crash", false],
 
   // Don't check for the default web browser during startup.
   //
   // These should also be set in the profile prior to starting Firefox,
   // as it is picked up at runtime.
   ["browser.shell.checkDefaultBrowser", false],
 
-  // Do not warn when quitting with multiple tabs
-  ["browser.showQuitWarning", false],
-
   // Do not redirect user when a milstone upgrade of Firefox is detected
   ["browser.startup.homepage_override.mstone", "ignore"],
 
   // Disable browser animations (tabs, fullscreen, sliding alerts)
   ["toolkit.cosmeticAnimations.enabled", false],
 
   // Do not close the window when the last tab gets closed
   ["browser.tabs.closeWindowWithLastTab", false],