Bug 600505: Reorganize app update preference ui to not favor disabling app update. ui-review=faaborg, r=rstrong
authorSteffen Wilberg <steffen.wilberg@web.de>
Thu, 13 Oct 2011 13:01:58 -0700 (2011-10-13)
changeset 78713 2d9076dbcd1510fd732978ad61d666f8899db238
parent 78712 2f53f26d9e1b90687890a3b5010854536a444f84
child 78714 756d9bfae05d98c7ca27f003bea2b7fec0303b17
push id21326
push userbmo@edmorley.co.uk
push dateFri, 14 Oct 2011 10:00:06 +0000 (2011-10-14)
treeherdermozilla-central@ca73f057dab7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs600505
milestone10.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 600505: Reorganize app update preference ui to not favor disabling app update. ui-review=faaborg, r=rstrong
browser/components/preferences/advanced.js
browser/components/preferences/advanced.xul
browser/locales/en-US/chrome/browser/preferences/advanced.dtd
--- a/browser/components/preferences/advanced.js
+++ b/browser/components/preferences/advanced.js
@@ -17,16 +17,17 @@
 # The Initial Developer of the Original Code is
 # Ben Goodger.
 # Portions created by the Initial Developer are Copyright (C) 2005
 # the Initial Developer. All Rights Reserved.
 #
 # Contributor(s):
 #   Ben Goodger <ben@mozilla.org>
 #   Jeff Walden <jwalden+code@mit.edu>
+#   Steffen Wilberg <steffen.wilberg@web.de>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
 # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
 # in which case the provisions of the GPL or the LGPL are applicable instead
 # of those above. If you wish to allow use of your version of this file only
 # under the terms of either the GPL or the LGPL, and not to allow others to
 # use your version of this file under the terms of the MPL, indicate your
@@ -56,19 +57,17 @@ var gAdvancedPane = {
       advancedPrefs.selectedTab = document.getElementById(extraArgs["advancedTab"]);
     } else {
       var preference = document.getElementById("browser.preferences.advanced.selectedTabIndex");
       if (preference.value !== null)
         advancedPrefs.selectedIndex = preference.value;
     }
 
 #ifdef MOZ_UPDATER
-    this.updateAppUpdateItems();
-    this.updateAutoItems();
-    this.updateModeItems();
+    this.updateReadPrefs();
 #endif
     this.updateOfflineApps();
 #ifdef MOZ_CRASHREPORTER
     this.initSubmitCrashes();
 #endif
     this.updateActualCacheSize();
   },
 
@@ -454,92 +453,100 @@ var gAdvancedPane = {
    * app.update.mode
    * - an integer:
    *     0    do not warn if an update will disable extensions or themes
    *     1    warn if an update will disable extensions or themes
    *     2    warn if an update will disable extensions or themes *or* if the
    *          update is a major update
    */
 
+#ifdef MOZ_UPDATER
   /**
-   * Enables and disables various UI preferences as necessary to reflect locked,
-   * disabled, and checked/unchecked states.
+   * Selects the item of the radiogroup, and sets the warnIncompatible checkbox
+   * based on the pref values and locked states.
    *
    * UI state matrix for update preference conditions
    * 
-   * UI Components:                                     Preferences
-   * 1 = Firefox checkbox                               i   = app.update.enabled
-   * 2 = When updates for Firefox are found label       ii  = app.update.auto
-   * 3 = Automatic Radiogroup (Ask vs. Automatically)   iii = app.update.mode
-   * 4 = Warn before disabling extensions checkbox
-   * 
-   * States:
-   * Element     p   val     locked    Disabled 
-   * 1           i   t/f     f         false
-   *             i   t/f     t         true
-   *             ii  t/f     t/f       false
-   *             iii 0/1/2   t/f       false
-   * 2,3         i   t       t/f       false
-   *             i   f       t/f       true
-   *             ii  t/f     f         false
-   *             ii  t/f     t         true
-   *             iii 0/1/2   t/f       false
-   * 4           i   t       t/f       false
-   *             i   f       t/f       true
-   *             ii  t       t/f       false
-   *             ii  f       t/f       true
-   *             iii 0/1/2   f         false
-   *             iii 0/1/2   t         true   
-   * 
+   * UI Components:                              Preferences
+   * Radiogroup                                  i   = app.update.enabled
+   * Warn before disabling extensions checkbox   ii  = app.update.auto
+   *                                             iii = app.update.mode
+   *
+   * Disabled states:
+   * Element           pref  value  locked  disabled
+   * radiogroup        i     t/f    f       false
+   *                   i     t/f    *t*     *true*
+   *                   ii    t/f    f       false
+   *                   ii    t/f    *t*     *true*
+   *                   iii   0/1/2  t/f     false
+   * warnIncompatible  i     t      f       false
+   *                   i     t      *t*     *true*
+   *                   i     *f*    t/f     *true*
+   *                   ii    t      f       false
+   *                   ii    t      *t*     *true*
+   *                   ii    *f*    t/f     *true*
+   *                   iii   0/1/2  f       false
+   *                   iii   0/1/2  *t*     *true*
    */
-#ifdef MOZ_UPDATER
-  updateAppUpdateItems: function () 
+  updateReadPrefs: function ()
   {
-    var aus = 
-        Components.classes["@mozilla.org/updates/update-service;1"].
-        getService(Components.interfaces.nsIApplicationUpdateService);
+    var enabledPref = document.getElementById("app.update.enabled");
+    var autoPref = document.getElementById("app.update.auto");
+    var radiogroup = document.getElementById("updateRadioGroup");
+
+    if (!enabledPref.value)   // Don't care for autoPref.value in this case.
+      radiogroup.value="manual"     // 3. Never check for updates.
+    else if (autoPref.value)  // enabledPref.value && autoPref.value
+      radiogroup.value="auto";      // 1. Automatically install updates
+    else                      // enabledPref.value && !autoPref.value
+      radiogroup.value="checkOnly"; // 2. Check, but let me choose
 
-    var enabledPref = document.getElementById("app.update.enabled");
-    var enableAppUpdate = document.getElementById("enableAppUpdate");
+    var canCheck = Components.classes["@mozilla.org/updates/update-service;1"].
+                     getService(Components.interfaces.nsIApplicationUpdateService).
+                     canCheckForUpdates;
+    // canCheck is false if the enabledPref is false and locked,
+    // or the binary platform or OS version is not known.
+    // A locked pref is sufficient to disable the radiogroup.
+    radiogroup.disabled = !canCheck || enabledPref.locked || autoPref.locked;
 
-    enableAppUpdate.disabled = !aus.canCheckForUpdates || enabledPref.locked;
+    var modePref = document.getElementById("app.update.mode");
+    var warnIncompatible = document.getElementById("warnIncompatible");
+    // the warnIncompatible checkbox value is set by readAddonWarn
+    warnIncompatible.disabled = radiogroup.disabled || modePref.locked ||
+                                !enabledPref.value || !autoPref.value;
   },
 
   /**
-   * Enables/disables UI for "when updates are found" based on the values,
-   * and "locked" states of associated preferences.
+   * Sets the pref values based on the selected item of the radiogroup,
+   * and sets the disabled state of the warnIncompatible checkbox accordingly.
    */
-  updateAutoItems: function () 
+  updateWritePrefs: function ()
   {
     var enabledPref = document.getElementById("app.update.enabled");
     var autoPref = document.getElementById("app.update.auto");
-    
-    var updateModeLabel = document.getElementById("updateModeLabel");
-    var updateMode = document.getElementById("updateMode");
-    
-    var disable = enabledPref.locked || !enabledPref.value ||
-                  autoPref.locked;
-    updateModeLabel.disabled = updateMode.disabled = disable;
-  },
+    var radiogroup = document.getElementById("updateRadioGroup");
+    switch (radiogroup.value) {
+      case "auto":      // 1. Automatically install updates
+        enabledPref.value = true;
+        autoPref.value = true;
+        break;
+      case "checkOnly": // 2. Check, but let me choose
+        enabledPref.value = true;
+        autoPref.value = false;
+        break;
+      case "manual":    // 3. Never check for updates.
+        enabledPref.value = false;
+        autoPref.value = false;
+    }
 
-  /**
-   * Enables/disables the "warn if incompatible extensions/themes exist" UI
-   * based on the values and "locked" states of various preferences.
-   */
-  updateModeItems: function () 
-  {
-    var enabledPref = document.getElementById("app.update.enabled");
-    var autoPref = document.getElementById("app.update.auto");
+    var warnIncompatible = document.getElementById("warnIncompatible");
     var modePref = document.getElementById("app.update.mode");
-    
-    var warnIncompatible = document.getElementById("warnIncompatible");
-    
-    var disable = enabledPref.locked || !enabledPref.value || autoPref.locked ||
-                  !autoPref.value || modePref.locked;
-    warnIncompatible.disabled = disable;
+    warnIncompatible.disabled = enabledPref.locked || !enabledPref.value ||
+                                autoPref.locked || !autoPref.value ||
+                                modePref.locked;
   },
 
   /**
    * Stores the value of the app.update.mode preference, which is a tristate
    * integer preference.  We store the value here so that we can properly
    * restore the preference value if the UI reflecting the preference value
    * is in a state which can represent either of two integer values (as
    * opposed to only one possible value in the other UI state).
@@ -548,28 +555,28 @@ var gAdvancedPane = {
 
   /**
    * Reads the app.update.mode preference and converts its value into a
    * true/false value for use in determining whether the "Warn me if this will
    * disable extensions or themes" checkbox is checked.  We also save the value
    * of the preference so that the preference value can be properly restored if
    * the user's preferences cannot adequately be expressed by a single checkbox.
    *
-   * app.update.modee         Checkbox State    Meaning
+   * app.update.mode          Checkbox State    Meaning
    * 0                        Unchecked         Do not warn
    * 1                        Checked           Warn if there are incompatibilities
    * 2                        Checked           Warn if there are incompatibilities,
    *                                            or the update is major.
    */
   readAddonWarn: function ()
   {
     var preference = document.getElementById("app.update.mode");
-    var doNotWarn = preference.value != 0;
-    gAdvancedPane._modePreference = doNotWarn ? preference.value : 1;
-    return doNotWarn;
+    var warn = preference.value != 0;
+    gAdvancedPane._modePreference = warn ? preference.value : 1;
+    return warn;
   },
 
   /**
    * Converts the state of the "Warn me if this will disable extensions or
    * themes" checkbox into the integer preference which represents it,
    * returning that value.
    */
   writeAddonWarn: function ()
@@ -586,26 +593,26 @@ var gAdvancedPane = {
     var prompter = Components.classes["@mozilla.org/updates/update-prompt;1"]
                              .createInstance(Components.interfaces.nsIUpdatePrompt);
     prompter.showUpdateHistory(window);
   },
 #endif
 
   /**
    * The Extensions checkbox and button are disabled only if the enable Addon
-   * update preference is locked. 
+   * update preference is locked.
    */
   updateAddonUpdateUI: function ()
   {
     var enabledPref = document.getElementById("extensions.update.enabled");
     var enableAddonUpdate = document.getElementById("enableAddonUpdate");
 
     enableAddonUpdate.disabled = enabledPref.locked;
-  },  
-  
+  },
+
   // ENCRYPTION TAB
 
   /*
    * Preferences:
    *
    * security.enable_ssl3
    * - true if SSL 3 encryption is enabled, false otherwise
    * security.enable_tls
--- a/browser/components/preferences/advanced.xul
+++ b/browser/components/preferences/advanced.xul
@@ -20,16 +20,17 @@
 # Ben Goodger.
 # Portions created by the Initial Developer are Copyright (C) 2005
 # the Initial Developer. All Rights Reserved.
 #
 # Contributor(s):
 #   Ben Goodger <ben@mozilla.org>
 #   Jeff Walden <jwalden+code@mit.edu>
 #   Ehsan Akhgari <ehsan.akhgari@gmail.com>
+#   Steffen Wilberg <steffen.wilberg@web.de>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
 # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
 # in which case the provisions of the GPL or the LGPL are applicable instead
 # of those above. If you wish to allow use of your version of this file only
 # under the terms of either the GPL or the LGPL, and not to allow others to
 # use your version of this file under the terms of the MPL, indicate your
@@ -95,25 +96,19 @@
  
       <preference id="browser.cache.disk.smart_size.enabled"
                   name="browser.cache.disk.smart_size.enabled"
                   inverted="true"
                   type="bool"/>
  
      <!-- Update tab -->
 #ifdef MOZ_UPDATER
-      <preference id="app.update.enabled"              name="app.update.enabled"              type="bool"
-                  onchange="gAdvancedPane.updateAppUpdateItems();
-                            gAdvancedPane.updateAutoItems();
-                            gAdvancedPane.updateModeItems();"/>
-      <preference id="app.update.auto"                 name="app.update.auto"                 type="bool"
-                  onchange="gAdvancedPane.updateAutoItems();
-                            gAdvancedPane.updateModeItems();"/>
-      <preference id="app.update.mode"                 name="app.update.mode"                 type="int"
-                  onchange="gAdvancedPane.updateModeItems();"/>
+      <preference id="app.update.enabled"              name="app.update.enabled"              type="bool"/>
+      <preference id="app.update.auto"                 name="app.update.auto"                 type="bool"/>
+      <preference id="app.update.mode"                 name="app.update.mode"                 type="int"/>
 
       <preference id="app.update.disable_button.showUpdateHistory"
                   name="app.update.disable_button.showUpdateHistory"
                   type="bool"/>
 #endif
 
       <preference id="extensions.update.enabled"       name="extensions.update.enabled"       type="bool"
                   onchange="gAdvancedPane.updateAddonUpdateUI();"/>
@@ -294,65 +289,61 @@
                         accesskey="&offlineAppsListRemove.accesskey;" 
                         oncommand="gAdvancedPane.removeOfflineApp();"/>
               </vbox>
             </hbox>
           </groupbox>
         </tabpanel>
 
         <!-- Update -->
-        <tabpanel id="updatePanel" orient="vertical" align="start">
-          <label control="autoUpdateGroup">&autoCheck.label;</label>
-          <vbox class="indent" id="autoUpdateGroup" role="group">
+        <tabpanel id="updatePanel" orient="vertical">
 #ifdef MOZ_UPDATER
-            <checkbox id="enableAppUpdate"
-                      label="&enableAppUpdate.label;"
-                      accesskey="&enableAppUpdate.accesskey;"
-                      preference="app.update.enabled"/>
+          <groupbox id="updateApp">
+            <caption label="&updateApp.label;"/>
+            <radiogroup id="updateRadioGroup"
+                        oncommand="gAdvancedPane.updateWritePrefs();">
+              <radio value="auto"
+                     label="&updateAuto.label;"
+                     accesskey="&updateAuto.accesskey;"/>
+              <hbox class="indent">
+                <checkbox id="warnIncompatible"
+                          label="&updateAutoAddonWarn.label;"
+                          accesskey="&updateAutoAddonWarn.accesskey;"
+                          preference="app.update.mode"
+                          onsyncfrompreference="return gAdvancedPane.readAddonWarn();"
+                          onsynctopreference="return gAdvancedPane.writeAddonWarn();"/>
+              </hbox>
+              <radio value="checkOnly"
+                     label="&updateCheck.label;"
+                     accesskey="&updateCheck.accesskey;"/>
+              <radio value="manual"
+                     label="&updateManual.label;"
+                     accesskey="&updateManual.accesskey;"/>
+            </radiogroup>
+
+            <hbox>
+              <button id="showUpdateHistory"
+                      label="&updateHistory.label;"
+                      accesskey="&updateHistory.accesskey;"
+                      preference="app.update.disable_button.showUpdateHistory"
+                      oncommand="gAdvancedPane.showUpdates();"/>
+            </hbox>
+          </groupbox>
 #endif
+          <groupbox id="updateOthers">
+            <caption label="&updateOthers.label;"/>
             <checkbox id="enableAddonUpdate"
-                      label="&enableAddonsUpdate2.label;"
-                      accesskey="&enableAddonsUpdate2.accesskey;"
+                      label="&enableAddonsUpdate3.label;"
+                      accesskey="&enableAddonsUpdate3.accesskey;"
                       preference="extensions.update.enabled"/>
             <checkbox id="enableSearchUpdate"
                       label="&enableSearchUpdate.label;"
                       accesskey="&enableSearchUpdate.accesskey;"
                       preference="browser.search.update"/>
-          </vbox>
-
-#ifdef MOZ_UPDATER
-          <separator id="updateSeparator1"/>
-
-          <label id="updateModeLabel" control="updateMode">&whenUpdatesFound.label;</label>
-          <radiogroup id="updateMode" class="indent"
-                      preference="app.update.auto">
-            <radio id="ask" value="false"
-                   label="&askMe.label;"
-                   accesskey="&askMe.accesskey;"/>
-            <radio id="automatic" value="true"
-                   label="&modeAutomatic.label;"
-                   accesskey="&modeAutomatic.accesskey;"/>
-            <hbox class="indent">
-              <checkbox id="warnIncompatible" 
-                        label="&modeAutoAddonWarn.label;" accesskey="&modeAutoAddonWarn.accesskey;"
-                        preference="app.update.mode"
-                        onsyncfrompreference="return gAdvancedPane.readAddonWarn();"
-                        onsynctopreference="return gAdvancedPane.writeAddonWarn();"/>
-            </hbox>
-          </radiogroup>
-
-          <separator id="updateSeparator2"/>
-
-          <hbox>
-            <button id="showUpdateHistory" 
-                    label="&updateHistory.label;" accesskey="&updateHistory.accesskey;"
-                    preference="app.update.disable_button.showUpdateHistory"
-                    oncommand="gAdvancedPane.showUpdates();"/>
-          </hbox>
-#endif
+          </groupbox>
         </tabpanel>
 
         <!-- Encryption -->
         <tabpanel id="encryptionPanel" orient="vertical">
 
           <!-- Protocols -->
           <groupbox id="protocolsGroup">
             <caption label="&protocols.label;"/>
--- a/browser/locales/en-US/chrome/browser/preferences/advanced.dtd
+++ b/browser/locales/en-US/chrome/browser/preferences/advanced.dtd
@@ -53,32 +53,35 @@
 <!ENTITY limitCacheSizeAfter.label       "MB of space">
 <!ENTITY clearCacheNow.label             "Clear Now">
 <!ENTITY clearCacheNow.accesskey         "C">
 <!ENTITY overrideSmartCacheSize.label    "Override automatic cache management">
 <!ENTITY overrideSmartCacheSize.accesskey "O">
 
 <!ENTITY updateTab.label                 "Update">
 
-<!ENTITY autoCheck.label                 "Automatically check for updates to:">
-<!ENTITY enableAppUpdate.label           "&brandShortName;">
-<!ENTITY enableAppUpdate.accesskey       "F">
-<!ENTITY enableAddonsUpdate2.label       "Add-ons">
-<!ENTITY enableAddonsUpdate2.accesskey   "n">
+<!ENTITY updateApp.label                 "&brandShortName; updates:">
+<!ENTITY updateAuto.label                "Automatically install updates (recommended: improved security)">
+<!ENTITY updateAuto.accesskey            "A">
+<!ENTITY updateCheck.label               "Check for updates, but let me choose whether to install them">
+<!ENTITY updateCheck.accesskey           "C">
+<!ENTITY updateManual.label              "Never check for updates (not recommended: security risk)">
+<!ENTITY updateManual.accesskey          "N">
+
+<!ENTITY updateAutoAddonWarn.label       "Warn me if this will disable any of my add-ons">
+<!ENTITY updateAutoAddonWarn.accesskey   "W">
+
+<!ENTITY updateHistory.label             "Show Update History">
+<!ENTITY updateHistory.accesskey         "p">
+
+<!ENTITY updateOthers.label              "Automatically update:">
+<!ENTITY enableAddonsUpdate3.label       "Add-ons">
+<!ENTITY enableAddonsUpdate3.accesskey   "o">
 <!ENTITY enableSearchUpdate.label        "Search Engines">
 <!ENTITY enableSearchUpdate.accesskey    "E">
-<!ENTITY whenUpdatesFound.label          "When updates to &brandShortName; are found:">
-<!ENTITY askMe.label                     "Ask me what I want to do">
-<!ENTITY askMe.accesskey                 "k">
-<!ENTITY modeAutomatic.label             "Automatically download and install the update">
-<!ENTITY modeAutomatic.accesskey         "d">
-<!ENTITY modeAutoAddonWarn.label         "Warn me if this will disable any of my add-ons">
-<!ENTITY modeAutoAddonWarn.accesskey     "W">
-<!ENTITY updateHistory.label             "Show Update History">
-<!ENTITY updateHistory.accesskey         "p">
 
 <!ENTITY offlineNotify.label             "Tell me when a website asks to store data for offline use">
 <!ENTITY offlineNotify.accesskey         "T">
 <!ENTITY offlineNotifyExceptions.label   "Exceptions…">
 <!ENTITY offlineNotifyExceptions.accesskey "x">
 
 <!ENTITY offlineAppsList.label           "The following websites have stored data for offline use:">
 <!ENTITY offlineAppsList.height          "7em">