Bug 1455392 - Remove sortable UI in about:addons recent updates;r=aswan draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Fri, 20 Apr 2018 09:35:36 -0700
changeset 785753 765bc7f830886ed2a5a52ca433a6e1d86511cc46
parent 783746 5ded36cb383d3ccafd9b6c231c5120dcdae196a2
child 785754 b77fc5fecccf2c336a18c84364f70ccc33ea1fb6
push id107305
push userbgrinstead@mozilla.com
push dateFri, 20 Apr 2018 16:37:20 +0000
reviewersaswan
bugs1455392
milestone61.0a1
Bug 1455392 - Remove sortable UI in about:addons recent updates;r=aswan This UI is the only consumer of `checkState`, and it's using it in a confusing way represent a ascending and descending sorts on the checkbox. And also since the default sort for Recent Updates (most recent first) is reasonable, go ahead and remove the binding altogether. MozReview-Commit-ID: 4OnAN1t2fGq
toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
toolkit/mozapps/extensions/content/extensions.css
toolkit/mozapps/extensions/content/extensions.js
toolkit/mozapps/extensions/content/extensions.xml
toolkit/mozapps/extensions/content/extensions.xul
toolkit/mozapps/extensions/test/browser/browser_recentupdates.js
toolkit/themes/shared/extensions/extensions.inc.css
--- a/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
+++ b/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.dtd
@@ -156,23 +156,16 @@
 
 <!-- ratings -->
 <!ENTITY rating2.label                        "Rating">
 
 <!-- download/install progress -->
 <!ENTITY progress.pause.tooltip               "Pause">
 <!ENTITY progress.cancel.tooltip              "Cancel">
 
-
-<!-- list sorting -->
-<!ENTITY sort.name.label                      "Name">
-<!ENTITY sort.name.tooltip                    "Sort by name">
-<!ENTITY sort.dateUpdated.label               "Last Updated">
-<!ENTITY sort.dateUpdated.tooltip             "Sort by date updated">
-
 <!ENTITY addon.homepage                       "Homepage">
 <!ENTITY addon.details.label                  "More">
 <!ENTITY addon.details.tooltip                "Show more details about this add-on">
 <!ENTITY addon.unknownDate                    "Unknown">
 <!-- LOCALIZATION NOTE (addon.legacy.label): This appears in a badge next
      to the add-on name for extensions that are not webextensions, which
      will stop working in Firefox 57. -->
 <!ENTITY addon.legacy.label                   "LEGACY">
--- a/toolkit/mozapps/extensions/content/extensions.css
+++ b/toolkit/mozapps/extensions/content/extensions.css
@@ -12,20 +12,16 @@ xhtml|link {
 #categories {
   -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#categories-list");
 }
 
 .category {
   -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#category");
 }
 
-.sort-controls {
-  -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#sorters");
-}
-
 .addon[status="installed"] {
   -moz-box-orient: vertical;
   -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#addon-generic");
 }
 
 .addon[status="installing"] {
   -moz-box-orient: vertical;
   -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#addon-installing");
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -3228,26 +3228,23 @@ var gDetailView = {
   }
 };
 
 
 var gUpdatesView = {
   node: null,
   _listBox: null,
   _emptyNotice: null,
-  _sorters: null,
   _updateSelected: null,
   _categoryItem: null,
 
   initialize() {
     this.node = document.getElementById("updates-view");
     this._listBox = document.getElementById("updates-list");
     this._emptyNotice = document.getElementById("updates-list-empty");
-    this._sorters = document.getElementById("updates-sorters");
-    this._sorters.handler = this;
 
     this._categoryItem = gCategories.get("addons://updates/available");
 
     this._updateSelected = document.getElementById("update-selected-btn");
     this._updateSelected.addEventListener("command", function() {
       gUpdatesView.installSelected();
     });
 
@@ -3294,17 +3291,17 @@ var gUpdatesView = {
         if (addon.hidden || !addon.updateDate || addon.updateDate.getTime() < threshold)
           continue;
 
         elements.push(createItem(addon));
       }
 
       this.showEmptyNotice(elements.length == 0);
       if (elements.length > 0) {
-        sortElements(elements, [this._sorters.sortBy], this._sorters.ascending);
+        sortElements(elements, ["updateDate"], false);
         for (let element of elements)
           this._listBox.appendChild(element);
       }
 
       gViewController.notifyViewChanged();
     });
   },
 
@@ -3339,17 +3336,17 @@ var gUpdatesView = {
           this.maybeDisableUpdateSelected();
         });
         elements.push(item);
       }
 
       this.showEmptyNotice(elements.length == 0);
       if (elements.length > 0) {
         this._updateSelected.hidden = false;
-        sortElements(elements, [this._sorters.sortBy], this._sorters.ascending);
+        sortElements(elements, ["updateDate"], false);
         for (let element of elements)
           this._listBox.appendChild(element);
       }
 
       // ensure badge count is in sync
       this._categoryItem.badgeCount = this._listBox.itemCount;
 
       gViewController.notifyViewChanged();
--- a/toolkit/mozapps/extensions/content/extensions.xml
+++ b/toolkit/mozapps/extensions/content/extensions.xml
@@ -202,134 +202,16 @@
       <method name="cancel">
         <body><![CDATA[
           this.mInstall.cancel();
         ]]></body>
       </method>
     </implementation>
   </binding>
 
-
-  <!-- Sorters - displays and controls the sort state of a list. -->
-  <binding id="sorters">
-    <content orient="horizontal">
-      <xul:button anonid="name-btn" class="sorter"
-                  label="&sort.name.label;" tooltiptext="&sort.name.tooltip;"
-                  oncommand="this.parentNode._handleChange('name');"/>
-      <xul:button anonid="date-btn" class="sorter"
-                  label="&sort.dateUpdated.label;"
-                  tooltiptext="&sort.dateUpdated.tooltip;"
-                  oncommand="this.parentNode._handleChange('updateDate');"/>
-    </content>
-
-    <implementation>
-      <constructor><![CDATA[
-        if (!this.hasAttribute("sortby"))
-          this.setAttribute("sortby", "name");
-
-        this._refreshState();
-      ]]></constructor>
-
-      <field name="handler">null</field>
-      <field name="_btnName">
-        document.getAnonymousElementByAttribute(this, "anonid", "name-btn");
-      </field>
-      <field name="_btnDate">
-        document.getAnonymousElementByAttribute(this, "anonid", "date-btn");
-      </field>
-
-      <property name="sortBy">
-        <getter><![CDATA[
-          return this.getAttribute("sortby");
-        ]]></getter>
-        <setter><![CDATA[
-          if (val != this.sortBy) {
-            this.setAttribute("sortBy", val);
-            this._refreshState();
-          }
-        ]]></setter>
-      </property>
-
-      <property name="ascending">
-        <getter><![CDATA[
-          return (this.getAttribute("ascending") == "true");
-        ]]></getter>
-        <setter><![CDATA[
-          val = !!val;
-          if (val != this.ascending) {
-            this.setAttribute("ascending", val);
-            this._refreshState();
-          }
-        ]]></setter>
-      </property>
-
-      <method name="setSort">
-        <parameter name="aSort"/>
-        <parameter name="aAscending"/>
-        <body><![CDATA[
-          var sortChanged = false;
-          if (aSort != this.sortBy) {
-            this.setAttribute("sortby", aSort);
-            sortChanged = true;
-          }
-
-          aAscending = !!aAscending;
-          if (this.ascending != aAscending) {
-            this.setAttribute("ascending", aAscending);
-            sortChanged = true;
-          }
-
-          if (sortChanged)
-            this._refreshState();
-        ]]></body>
-      </method>
-
-      <method name="_handleChange">
-        <parameter name="aSort"/>
-        <body><![CDATA[
-          const ASCENDING_SORT_FIELDS = ["name"];
-
-          // Toggle ascending if sort by is not changing, otherwise
-          // name sorting defaults to ascending, others to descending
-          if (aSort == this.sortBy)
-            this.ascending = !this.ascending;
-          else
-            this.setSort(aSort, ASCENDING_SORT_FIELDS.includes(aSort));
-        ]]></body>
-      </method>
-
-      <method name="_refreshState">
-        <body><![CDATA[
-          var sortBy = this.sortBy;
-          var checkState = this.ascending ? 2 : 1;
-
-          if (sortBy == "name") {
-            this._btnName.checkState = checkState;
-            this._btnName.checked = true;
-          } else {
-            this._btnName.checkState = 0;
-            this._btnName.checked = false;
-          }
-
-          if (sortBy == "updateDate") {
-            this._btnDate.checkState = checkState;
-            this._btnDate.checked = true;
-          } else {
-            this._btnDate.checkState = 0;
-            this._btnDate.checked = false;
-          }
-
-          if (this.handler && "onSortChanged" in this.handler)
-            this.handler.onSortChanged(sortBy, this.ascending);
-        ]]></body>
-      </method>
-    </implementation>
-  </binding>
-
-
   <!-- Categories list - displays the list of categories on the left pane. -->
   <binding id="categories-list"
            extends="chrome://global/content/bindings/richlistbox.xml#richlistbox">
     <implementation>
       <!-- This needs to be overridden to allow the fancy animation while not
            allowing that item to be selected when hiding.  -->
       <method name="_canUserSelect">
         <parameter name="aItem"/>
--- a/toolkit/mozapps/extensions/content/extensions.xul
+++ b/toolkit/mozapps/extensions/content/extensions.xul
@@ -377,19 +377,16 @@
                            value="&warning.updatesecurity.label;"/>
                   </hbox>
                   <button class="button-link global-warning-updatesecurity"
                           label="&warning.updatesecurity.enable.label;"
                           tooltiptext="&warning.updatesecurity.enable.tooltip;"
                           command="cmd_enableUpdateSecurity"/>
                   <spacer flex="5000"/> <!-- Necessary to allow the message to wrap -->
                 </hbox>
-                <spacer flex="1"/>
-                <hbox id="updates-sorters" class="sort-controls" sortby="updateDate"
-                      ascending="false"/>
               </hbox>
               <vbox id="updates-list-empty" class="alert-container"
                     flex="1" hidden="true">
                 <spacer class="alert-spacer-before"/>
                 <vbox class="alert">
                   <label id="empty-availableUpdates-msg" value="&listEmpty.availableUpdates.label;"/>
                   <label id="empty-recentUpdates-msg" value="&listEmpty.recentUpdates.label;"/>
                   <button label="&listEmpty.findUpdates.label;"
--- a/toolkit/mozapps/extensions/test/browser/browser_recentupdates.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_recentupdates.js
@@ -59,61 +59,16 @@ add_test(function() {
       run_next_test();
     }, true);
     var menuitem = gManagerWindow.document.getElementById("utils-viewUpdates");
     EventUtils.synthesizeMouse(menuitem, 2, 2, { }, gManagerWindow);
   }, {once: true});
   EventUtils.synthesizeMouse(utilsBtn, 2, 2, { }, gManagerWindow);
 });
 
-
-add_test(function() {
-  var updatesList = gManagerWindow.document.getElementById("updates-list");
-  var sorters = gManagerWindow.document.getElementById("updates-sorters");
-  var dateSorter = gManagerWindow.document.getAnonymousElementByAttribute(sorters, "anonid", "date-btn");
-  var nameSorter = gManagerWindow.document.getAnonymousElementByAttribute(sorters, "anonid", "name-btn");
-
-  function check_order(expected) {
-    var items = updatesList.getElementsByTagName("richlistitem");
-    var possible = ["addon1@tests.mozilla.org", "addon2@tests.mozilla.org", "addon3@tests.mozilla.org"];
-    for (let item of items) {
-      let itemId = item.mAddon.id;
-      if (!possible.includes(itemId))
-        continue; // skip over any other addons, such as shipped addons that would update on every build
-      isnot(expected.length, 0, "Should be expecting more items");
-      is(itemId, expected.shift(), "Should get expected item based on sort order");
-      if (itemId == "addon1@tests.mozilla.org")
-        is_element_visible(item._relNotesToggle, "Release notes toggle should be visible for addon with release notes");
-      else
-        is_element_hidden(item._relNotesToggle, "Release notes toggle should be hidden for addon with no release notes");
-    }
-  }
-
-  is_element_visible(dateSorter);
-  is_element_visible(nameSorter);
-
-  // sorted by date, descending
-  check_order(["addon2@tests.mozilla.org", "addon1@tests.mozilla.org"]);
-
-  // sorted by date, ascending
-  EventUtils.synthesizeMouseAtCenter(dateSorter, { }, gManagerWindow);
-  check_order(["addon1@tests.mozilla.org", "addon2@tests.mozilla.org"]);
-
-  // sorted by name, ascending
-  EventUtils.synthesizeMouseAtCenter(nameSorter, { }, gManagerWindow);
-  check_order(["addon2@tests.mozilla.org", "addon1@tests.mozilla.org"]);
-
-  // sorted by name, descending
-  EventUtils.synthesizeMouseAtCenter(nameSorter, { }, gManagerWindow);
-  check_order(["addon1@tests.mozilla.org", "addon2@tests.mozilla.org"]);
-
-  run_next_test();
-});
-
-
 add_test(function() {
   close_manager(gManagerWindow, function() {
     open_manager(null, function(aWindow) {
       gManagerWindow = aWindow;
       gCategoryUtilities = new CategoryUtilities(gManagerWindow);
 
       var recentCat = gManagerWindow.gCategories.get("addons://updates/recent");
       is(gCategoryUtilities.isVisible(recentCat), true, "Recent Updates category should still be visible");
--- a/toolkit/themes/shared/extensions/extensions.inc.css
+++ b/toolkit/themes/shared/extensions/extensions.inc.css
@@ -281,49 +281,16 @@ button.warning {
   background-color: #ebebeb;
   cursor: pointer;
 }
 
 .header-button > .toolbarbutton-text {
   display: none;
 }
 
-/*** sorters ***/
-
-.sort-controls {
-  -moz-appearance: none;
-}
-
-.sorter {
-  height: 35px;
-  border: none;
-  border-radius: 0;
-  background-color: transparent;
-  color: #536680;
-  margin: 0;
-  min-width: 12px !important;
-  -moz-box-direction: reverse;
-}
-
-.sorter .button-box {
-  padding-top: 0;
-  padding-bottom: 0;
-}
-
-.sorter[checkState="1"],
-.sorter[checkState="2"] {
-  background-color: var(--in-content-box-background-hover);
-  box-shadow: 0 -4px 0 0 var(--in-content-border-highlight) inset;
-}
-
-.sorter .button-icon {
-  margin-inline-start: 6px;
-}
-
-
 /*** discover view ***/
 
 .discover-spacer-before,
 .discover-spacer-after {
   -moz-box-flex: 1;
 }
 
 #discover-error .alert {