Bug 1457027 - Part 1 - Don't persist the last selected handler. r=jaws draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Mon, 30 Apr 2018 11:11:48 +0100
changeset 791107 54b6b8bf8dd9151b25e0bd7bcd8b7c97bc31d179
parent 790721 f877359308b17e691209e1afb7193b8e86f175ce
child 791108 811bd50cdcd5a3e0075117a0f9d1ba8206783875
push id108697
push userpaolo.mozmail@amadzone.org
push dateThu, 03 May 2018 14:21:26 +0000
reviewersjaws
bugs1457027
milestone61.0a1
Bug 1457027 - Part 1 - Don't persist the last selected handler. r=jaws We plan to remove XUL persistence eventually, and this is a special case that uses the feature with an unusual attribute. This change also prevents pitfalls with getElementsByAttribute when the richlistitem descendants are made non-anonymous. Some unneeded attributes in both the handlers and the containers richlistbox items are also cleaned up. MozReview-Commit-ID: C05ArQGZb95
browser/components/preferences/in-content/containers.xul
browser/components/preferences/in-content/main.js
browser/components/preferences/in-content/main.xul
--- a/browser/components/preferences/in-content/containers.xul
+++ b/browser/components/preferences/in-content/containers.xul
@@ -19,19 +19,16 @@
       data-category="paneContainers">
   <label class="header-name" flex="1" data-l10n-id="containers-header"/>
 </hbox>
 
 <!-- Containers -->
 <groupbox id="browserContainersGroupPane" data-category="paneContainers" hidden="true"
           data-hidden-from-search="true" data-subpanel="true">
   <vbox id="browserContainersbox">
-
-    <richlistbox id="containersView" orient="vertical" persist="lastSelectedType"
-                 flex="1">
-    </richlistbox>
+    <richlistbox id="containersView"/>
   </vbox>
   <vbox>
     <hbox flex="1">
       <button id="containersAdd" oncommand="gContainersPane.onAddButtonCommand();" data-l10n-id="containers-add-button"/>
     </hbox>
   </vbox>
 </groupbox>
--- a/browser/components/preferences/in-content/main.js
+++ b/browser/components/preferences/in-content/main.js
@@ -538,18 +538,16 @@ var gMainPane = {
     Services.prefs.addObserver(PREF_VIDEO_FEED_SELECTED_READER, this);
 
     Services.prefs.addObserver(PREF_AUDIO_FEED_SELECTED_APP, this);
     Services.prefs.addObserver(PREF_AUDIO_FEED_SELECTED_WEB, this);
     Services.prefs.addObserver(PREF_AUDIO_FEED_SELECTED_ACTION, this);
     Services.prefs.addObserver(PREF_AUDIO_FEED_SELECTED_READER, this);
 
     setEventListener("filter", "command", gMainPane.filter);
-    setEventListener("handlersView", "select",
-      gMainPane.onSelectionChanged);
     setEventListener("typeColumn", "click", gMainPane.sort);
     setEventListener("actionColumn", "click", gMainPane.sort);
     setEventListener("chooseFolder", "command", gMainPane.chooseFolder);
     setEventListener("saveWhere", "command", gMainPane.handleSaveToCommand);
     Preferences.get("browser.download.folderList").on("change",
       gMainPane.displayDownloadDirPref.bind(gMainPane));
     Preferences.get("browser.download.dir").on("change",
       gMainPane.displayDownloadDirPref.bind(gMainPane));
@@ -1496,16 +1494,19 @@ var gMainPane = {
       if (handlerInfo.description in this._visibleTypeDescriptionCount)
         this._visibleTypeDescriptionCount[handlerInfo.description]++;
       else
         this._visibleTypeDescriptionCount[handlerInfo.description] = 1;
     }
   },
 
   _rebuildView() {
+    let lastSelectedType = this._list.selectedItem &&
+                           this._list.selectedItem.getAttribute("type");
+
     // Clear the list of entries.
     while (this._list.childNodes.length > 1)
       this._list.removeChild(this._list.lastChild);
 
     var visibleTypes = this._visibleTypes;
 
     // If the user is filtering the list, then only show matching types.
     if (this._filter.value)
@@ -1521,19 +1522,21 @@ var gMainPane = {
         this._describePreferredAction(visibleType));
 
       if (!this._setIconClassForPreferredAction(visibleType, item)) {
         item.setAttribute("actionIcon",
           this._getIconURLForPreferredAction(visibleType));
       }
 
       this._list.appendChild(item);
+
+      if (visibleType.type === lastSelectedType) {
+        this._list.selectedItem = item;
+      }
     }
-
-    this._selectLastSelectedType();
   },
 
   _matchesFilter(aType) {
     var filterValue = this._filter.value.toLowerCase();
     return this._describeType(aType).toLowerCase().includes(filterValue) ||
       this._describePreferredAction(aType).toLowerCase().includes(filterValue);
   },
 
@@ -1625,35 +1628,16 @@ var gMainPane = {
         return this._prefsBundle.getFormattedString("usePluginIn",
           [aHandlerInfo.pluginName,
           this._brandShortName]);
       default:
         throw new Error(`Unexpected preferredAction: ${aHandlerInfo.preferredAction}`);
     }
   },
 
-  _selectLastSelectedType() {
-    // If the list is disabled by the pref.downloads.disable_button.edit_actions
-    // preference being locked, then don't select the type, as that would cause
-    // it to appear selected, with a different background and an actions menu
-    // that makes it seem like you can choose an action for the type.
-    if (this._list.disabled)
-      return;
-
-    var lastSelectedType = this._list.getAttribute("lastSelectedType");
-    if (!lastSelectedType)
-      return;
-
-    var item = this._list.getElementsByAttribute("type", lastSelectedType)[0];
-    if (!item)
-      return;
-
-    this._list.selectedItem = item;
-  },
-
   /**
    * Whether or not the given handler app is valid.
    *
    * @param aHandlerApp {nsIHandlerApp} the handler app in question
    *
    * @returns {boolean} whether or not it's valid
    */
   isValidHandlerApp(aHandlerApp) {
@@ -2166,24 +2150,16 @@ var gMainPane = {
       // Prompt the user to pick an app.  If they pick one, and it's a valid
       // selection, then add it to the list of possible handlers.
       fp.init(window, winTitle, Ci.nsIFilePicker.modeOpen);
       fp.appendFilters(Ci.nsIFilePicker.filterApps);
       fp.open(fpCallback);
     }
   },
 
-  // Mark which item in the list was last selected so we can reselect it
-  // when we rebuild the list or when the user returns to the prefpane.
-  onSelectionChanged() {
-    if (this._list.selectedItem)
-      this._list.setAttribute("lastSelectedType",
-        this._list.selectedItem.getAttribute("type"));
-  },
-
   _setIconClassForPreferredAction(aHandlerInfo, aElement) {
     // If this returns true, the attribute that CSS sniffs for was set to something
     // so you shouldn't manually set an icon URI.
     // This removes the existing actionIcon attribute if any, even if returning false.
     aElement.removeAttribute("actionIcon");
 
     if (aHandlerInfo.alwaysAskBeforeHandling) {
       aElement.setAttribute(APP_ICON_ATTR_NAME, "ask");
--- a/browser/components/preferences/in-content/main.xul
+++ b/browser/components/preferences/in-content/main.xul
@@ -365,19 +365,18 @@
 <groupbox id="applicationsGroup" data-category="paneGeneral" hidden="true">
   <caption><label data-l10n-id="applications-header"/></caption>
   <description data-l10n-id="applications-description"/>
   <textbox id="filter" flex="1"
            type="search"
            data-l10n-id="applications-filter"
            aria-controls="handlersView"/>
 
-  <richlistbox id="handlersView" orient="vertical" persist="lastSelectedType"
-               preference="pref.downloads.disable_button.edit_actions"
-               flex="1">
+  <richlistbox id="handlersView"
+               preference="pref.downloads.disable_button.edit_actions">
     <listheader equalsize="always">
         <treecol id="typeColumn" data-l10n-id="applications-type-column" value="type"
                  persist="sortDirection"
                  flex="1" sortDirection="ascending"/>
         <treecol id="actionColumn" data-l10n-id="applications-action-column" value="action"
                  persist="sortDirection"
                  flex="1"/>
     </listheader>