Firefox Nightly and Aurora 20140928 on Windows 8.1 The columns in the storage inspector (tested with Cookies view) currently sort alphabetically, but a) the date columns should sort by date. At the moment, the order is e.g. Session 4.6.2015 28.9.2015 28.9.2014 27.9.2016 27.3.2015 b) for the host list, a sorting by domain and then sorting each set by subdomain would make more sense.

Jim Palmer's natural sort works with numbers and strings in the following formats: - strings - datetime - version number strings - numerics - IP addresses - filenames - hex - unicode

Changlist: - Added Jim Palmer's well proven natural sort algorithm. - Added natural sort license (MIT). - Use natural sort everywhere inside TableWidget.js wherever we use .sort() - Changed browser_storage_overflow.js so that the test is faster and more maintainable. The test now also tests column sorting (ascending and descending). - Use natural sort everywhere inside storage.js wherever we need to slice the array. Without natural sort here we get e.g. row-1, row-10, row-100, row-2 etc. Review commit: https://reviewboard.mozilla.org/r/132400/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/132400/

Comment on attachment 8860385 [details] Bug 1073967 - Storage Inspector columns should use natural sort https://reviewboard.mozilla.org/r/132400/#review135288 Please fix the hardcoded `150` value in the test, or change the whole test if you think my comment of rewriting it makes sense. ::: devtools/client/storage/test/browser_storage_overflow.js:38 (Diff revision 1) > +function checkCellValues(prefix, start, end) { > + let cells = [...gPanelWindow.document > + .querySelectorAll("#name .table-widget-cell")]; > + > + if (start < end) { > + for (let i = start; i < end; i++) { > + let cell = cells[i - 1]; > + > + is(cell.value, `${prefix}${i}`, "Cell value is correct (ascending)."); > + } > + } else { > + for (let i = start; i >= end; i--) { > + let cell = cells[150 - i]; > + > + is(cell.value, `${prefix}${i}`, "Cell value is correct (descending)."); > + } > + } I have the feeling we could simplify this a bit more. Since we already retrieve all the nodes directly within the test, I think we could simplify its signature with `function checkCellValues(order)`. We would then call : ``` // Check that the columns are sorted in a human readable way (ascending). checkCellValues("ASC"); // Check that the columns are sorted in a human readable way (descending). checkCellValues("DESC"); ``` The total number of items is not important here, we already tested it with `checkCellLength`. We could then only have a simple forEach to test the cells values ``` cells.forEach(function(cell, index, arr){ let i = order === "ASC" ? index : arr.length; is(cell.value, `item-${i}`, `Cell value is correct (${order}).`) }) ``` What do you think of this ? ::: devtools/client/storage/test/browser_storage_overflow.js:50 (Diff revision 1) > + > + is(cell.value, `${prefix}${i}`, "Cell value is correct (ascending)."); > + } > + } else { > + for (let i = start; i >= end; i--) { > + let cell = cells[150 - i]; I think this should be `start` instead of 150

Comment on attachment 8860385 [details] Bug 1073967 - Storage Inspector columns should use natural sort https://reviewboard.mozilla.org/r/132400/#review135288 > I have the feeling we could simplify this a bit more. > > Since we already retrieve all the nodes directly within the test, I think we could simplify its signature with `function checkCellValues(order)`. > > We would then call : > ``` > // Check that the columns are sorted in a human readable way (ascending). > checkCellValues("ASC"); > // Check that the columns are sorted in a human readable way (descending). > checkCellValues("DESC"); > ``` > > The total number of items is not important here, we already tested it with `checkCellLength`. > > We could then only have a simple forEach to test the cells values > ``` > cells.forEach(function(cell, index, arr){ > let i = order === "ASC" ? index : arr.length; > is(cell.value, `item-${i}`, `Cell value is correct (${order}).`) > }) > ``` > > What do you think of this ? I got the `let i` part wrong, but I think you see what I mean

Comment on attachment 8860385 [details] Bug 1073967 - Storage Inspector columns should use natural sort https://reviewboard.mozilla.org/r/132400/#review135320 All good Mike, thanks !

Pushed by mratcliffe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ef0ad7316ebb Storage Inspector columns should use natural sort r=nchevobbe