moebius#342: Columns are not sorted correctly (Natural Sort algorithm)

Issue #31
https://github.com/MoonchildProductions/moebius/pull/342

devtools/client/shared/moz.build

@@ -35,6 +35,7 @@ DevToolsModules(
     'Jsbeautify.jsm',
     'key-shortcuts.js',
     'keycodes.js',
+    'natural-sort.js',
     'network-throttling-profiles.js',
     'node-attribute-parser.js',
     'options-view.js',

devtools/client/shared/natural-sort.js

@@ -0,0 +1,106 @@
+/*
+ * Natural Sort algorithm for Javascript - Version 0.8.1 - Released under MIT license
+ * Author: Jim Palmer (based on chunking idea from Dave Koelle)
+ *
+ * Includes pull request to move regexes out of main function for performance
+ * increases.
+ *
+ * Repository:
+ *   https://github.com/overset/javascript-natural-sort/
+ */
+
+"use strict";
+
+var re = /(^([+\-]?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?(?=\D|\s|$))|^0x[\da-fA-F]+$|\d+)/g;
+var sre = /^\s+|\s+$/g;   // trim pre-post whitespace
+var snre = /\s+/g;        // normalize all whitespace to single ' ' character
+
+// eslint-disable-next-line
+var dre = /(^([\w ]+,?[\w ]+)?[\w ]+,?[\w ]+\d+:\d+(:\d+)?[\w ]?|^\d{1,4}[\/\-]\d{1,4}[\/\-]\d{1,4}|^\w+, \w+ \d+, \d{4})/;
+var hre = /^0x[0-9a-f]+$/i;
+var ore = /^0/;
+var b0re = /^\0/;
+var e0re = /\0$/;
+
+exports.naturalSortCaseSensitive =
+function naturalSortCaseSensitive(a, b) {
+  return naturalSort(a, b, false);
+};
+
+exports.naturalSortCaseInsensitive =
+function naturalSortCaseInsensitive(a, b) {
+  return naturalSort(a, b, true);
+};
+
+/**
+ * Sort numbers, strings, IP Addresses, Dates, Filenames, version numbers etc.
+ * "the way humans do."
+ *
+ * This function should only be called via naturalSortCaseSensitive and
+ * naturalSortCaseInsensitive.
+ *
+ * e.g. [3, 2, 1, 10].sort(naturalSort)
+ *
+ * @param  {Object} a
+ *         Passed in by Array.sort(a, b)
+ * @param  {Object} b
+ *         Passed in by Array.sort(a, b)
+ * @param  {Boolean} insensitive
+ *         Should the search be case insensitive?
+ */
+function naturalSort(a, b, insensitive) {
+  // convert all to strings strip whitespace
+  let i = function (s) {
+    return (insensitive && ("" + s).toLowerCase() || "" + s)
+                                   .replace(sre, "");
+  };
+  let x = i(a) || "";
+  let y = i(b) || "";
+  // chunk/tokenize
+  let xN = x.replace(re, "\0$1\0").replace(e0re, "").replace(b0re, "").split("\0");
+  let yN = y.replace(re, "\0$1\0").replace(e0re, "").replace(b0re, "").split("\0");
+  // numeric, hex or date detection
+  let xD = parseInt(x.match(hre), 16) || (xN.length !== 1 && Date.parse(x));
+  let yD = parseInt(y.match(hre), 16) || xD && y.match(dre) && Date.parse(y) || null;
+  let normChunk = function (s, l) {
+    // normalize spaces; find floats not starting with '0', string or 0 if
+    // not defined (Clint Priest)
+    return (!s.match(ore) || l == 1) &&
+           parseFloat(s) || s.replace(snre, " ").replace(sre, "") || 0;
+  };
+  let oFxNcL;
+  let oFyNcL;
+
+  // first try and sort Hex codes or Dates
+  if (yD) {
+    if (xD < yD) {
+      return -1;
+    } else if (xD > yD) {
+      return 1;
+    }
+  }
+
+  // natural sorting through split numeric strings and default strings
+  // eslint-disable-next-line
+  for (let cLoc = 0, xNl = xN.length, yNl = yN.length, numS = Math.max(xNl, yNl); cLoc < numS; cLoc++) {
+    oFxNcL = normChunk(xN[cLoc] || "", xNl);
+    oFyNcL = normChunk(yN[cLoc] || "", yNl);
+
+    // handle numeric vs string comparison - number < string - (Kyle Adams)
+    if (isNaN(oFxNcL) !== isNaN(oFyNcL)) {
+      return isNaN(oFxNcL) ? 1 : -1;
+    }
+    // if unicode use locale comparison
+    // eslint-disable-next-line
+    if (/[^\x00-\x80]/.test(oFxNcL + oFyNcL) && oFxNcL.localeCompare) {
+      let comp = oFxNcL.localeCompare(oFyNcL);
+      return comp / Math.abs(comp);
+    }
+    if (oFxNcL < oFyNcL) {
+      return -1;
+    } else if (oFxNcL > oFyNcL) {
+      return 1;
+    }
+  }
+  return null;
+}

devtools/client/shared/widgets/TableWidget.js

@@ -8,6 +8,8 @@ loader.lazyRequireGetter(this, "setNamedTimeout",
   "devtools/client/shared/widgets/view-helpers", true);
 loader.lazyRequireGetter(this, "clearNamedTimeout",
   "devtools/client/shared/widgets/view-helpers", true);
+loader.lazyRequireGetter(this, "naturalSortCaseInsensitive",
+  "devtools/client/shared/natural-sort", true);
 const {KeyCodes} = require("devtools/client/shared/keycodes");
 
 const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
@@ -1283,11 +1285,11 @@ Column.prototype = {
       let index;
       if (this.sorted == 1) {
         index = this.cells.findIndex(element => {
-          return value < element.value;
+          return naturalSortCaseInsensitive(value, element.value) === -1;
         });
       } else {
         index = this.cells.findIndex(element => {
-          return value > element.value;
+          return naturalSortCaseInsensitive(value, element.value) === 1;
         });
       }
       index = index >= 0 ? index : this.cells.length;
@@ -1415,7 +1417,7 @@ Column.prototype = {
             a[this.id].textContent : a[this.id];
         let val2 = (b[this.id] instanceof Node) ?
             b[this.id].textContent : b[this.id];
-        return val1 > val2;
+        return naturalSortCaseInsensitive(val1, val2);
       });
     } else if (this.sorted > 1) {
       items.sort((a, b) => {
@@ -1423,7 +1425,7 @@ Column.prototype = {
             a[this.id].textContent : a[this.id];
         let val2 = (b[this.id] instanceof Node) ?
             b[this.id].textContent : b[this.id];
-        return val2 > val1;
+        return naturalSortCaseInsensitive(val2, val1);
       });
     }
 

devtools/client/storage/test/browser_storage_overflow.js

@@ -2,40 +2,58 @@
 // inspector table.
 "use strict";
 
+const ITEMS_PER_PAGE = 50;
+
 add_task(function* () {
   yield openTabAndSetupStorage(MAIN_DOMAIN + "storage-overflow.html");
 
-  let $ = id => gPanelWindow.document.querySelector(id);
-  let $$ = sel => gPanelWindow.document.querySelectorAll(sel);
-
   gUI.tree.expandAll();
   yield selectTreeItem(["localStorage", "http://test1.example.org"]);
+  checkCellLength(ITEMS_PER_PAGE);
+ 
+  yield scroll();
+  checkCellLength(ITEMS_PER_PAGE * 2);
+
+  yield scroll();
+  checkCellLength(ITEMS_PER_PAGE * 3);
+
+  // Check that the columns are sorted in a human readable way (ascending).
+  checkCellValues("ASC");
+
+  // Sort descending.
+  clickColumnHeader("name");
+
+  // Check that the columns are sorted in a human readable way (descending).
+  checkCellValues("DEC");
+
+  yield finishTests();
+});
+
+function checkCellLength(len) {
+  let cells = gPanelWindow.document
+                          .querySelectorAll("#name .table-widget-cell");
+  let msg = `Table should initially display ${len} items`;
+
+  is(cells.length, len, msg);
+}
+
+function checkCellValues(order) {
+  let cells = [...gPanelWindow.document
+                              .querySelectorAll("#name .table-widget-cell")];
+  cells.forEach(function (cell, index, arr) {
+    let i = order === "ASC" ? index + 1 : arr.length - index;
+    is(cell.value, `item-${i}`, `Cell value is correct (${order}).`);
+  });
+}
+
+function* scroll() {
+  let $ = id => gPanelWindow.document.querySelector(id);
 
   let table = $("#storage-table .table-widget-body");
-  let cellHeight = $(".table-widget-cell").getBoundingClientRect().height;
-
-  is($$("#value .table-widget-cell").length, 50,
-     "Table should initially display 50 items");
+  let cell = $("#name .table-widget-cell");
+  let cellHeight = cell.getBoundingClientRect().height;
 
   let onStoresUpdate = gUI.once("store-objects-updated");
   table.scrollTop += cellHeight * 50;
   yield onStoresUpdate;
-
-  is($$("#value .table-widget-cell").length, 100,
-     "Table should display 100 items after scrolling");
-
-  onStoresUpdate = gUI.once("store-objects-updated");
-  table.scrollTop += cellHeight * 50;
-  yield onStoresUpdate;
-
-  is($$("#value .table-widget-cell").length, 150,
-     "Table should display 150 items after scrolling");
-
-  onStoresUpdate = gUI.once("store-objects-updated");
-  table.scrollTop += cellHeight * 50;
-  yield onStoresUpdate;
-
-  is($$("#value .table-widget-cell").length, 160,
-     "Table should display all 160 items after scrolling");
-  yield finishTests();
-});
+}

devtools/client/storage/test/head.js

@@ -733,6 +733,20 @@ function showColumn(id, state) {
   }
 }
 
+/**
+ * Toggle sort direction on a column by clicking on the column header.
+ *
+ * @param  {String} id
+ *         The uniqueId of the given column.
+ */
+function clickColumnHeader(id) {
+  let columns = gUI.table.columns;
+  let column = columns.get(id);
+  let header = column.header;
+
+  header.click();
+}
+
 /**
  * Show or hide all columns.
  *

devtools/client/storage/test/storage-overflow.html

@@ -11,7 +11,7 @@ Bug 1171903 - Storage Inspector endless scrolling
 <script type="text/javascript;version=1.8">
 "use strict";
 
-for (let i = 0; i < 160; i++) {
+for (let i = 1; i < 151; i++) {
   localStorage.setItem(`item-${i}`, `value-${i}`);
 }
 </script>

devtools/server/actors/storage.js

@@ -17,6 +17,9 @@ const { Task } = require("devtools/shared/task");
 
 const DEFAULT_VALUE = "value";
 
+loader.lazyRequireGetter(this, "naturalSortCaseInsensitive",
+  "devtools/client/shared/natural-sort", true);
+
 // GUID to be used as a separator in compound keys. This must match the same
 // constant in devtools/client/storage/ui.js,
 // devtools/client/storage/test/head.js and
@@ -326,15 +329,20 @@ StorageActors.defaults = function (typeName, observationTopic) {
             toReturn.data.push(...values);
           }
         }
+
         toReturn.total = this.getObjectsSize(host, names, options);
+
         if (offset > toReturn.total) {
           // In this case, toReturn.data is an empty array.
           toReturn.offset = toReturn.total;
           toReturn.data = [];
         } else {
-          toReturn.data = toReturn.data.sort((a, b) => {
-            return a[sortOn] - b[sortOn];
-          }).slice(offset, offset + size).map(a => this.toStoreObject(a));
+          // We need to use natural sort before slicing.
+          let sorted = toReturn.data.sort((a, b) => {
+            return naturalSortCaseInsensitive(a[sortOn], b[sortOn]);
+          });
+          let sliced = sorted.slice(offset, offset + size);
+          toReturn.data = sliced.map(a => this.toStoreObject(a));
         }
       } else {
         let obj = yield this.getValuesForHost(host, undefined, undefined,
@@ -344,15 +352,18 @@ StorageActors.defaults = function (typeName, observationTopic) {
         }
 
         toReturn.total = obj.length;
+
         if (offset > toReturn.total) {
           // In this case, toReturn.data is an empty array.
           toReturn.offset = offset = toReturn.total;
           toReturn.data = [];
         } else {
-          toReturn.data = obj.sort((a, b) => {
-            return a[sortOn] - b[sortOn];
-          }).slice(offset, offset + size)
-            .map(object => this.toStoreObject(object));
+          // We need to use natural sort before slicing.
+          let sorted = obj.sort((a, b) => {
+            return naturalSortCaseInsensitive(a[sortOn], b[sortOn]);
+          });
+          let sliced = sorted.slice(offset, offset + size);
+          toReturn.data = sliced.map(object => this.toStoreObject(object));
         }
       }