From 9ca04cf636c990712b0db1c8fcce3ce8c6038e93 Mon Sep 17 00:00:00 2001
From: Julian Descottes <jdescottes@mozilla.com>
Date: Mon, 5 Sep 2022 13:00:15 +0000
Subject: [PATCH] Bug 1789052 - [devtools] Fix storage inspector sorting logic.
r=nchevobbe, a=RyanVM
Two issues fixed:
- when sorting on one column, other column content should be re-arranged
- natural order sort was not symmetrical for dates and led to inconsistent sorting
Differential Revision: https://phabricator.services.mozilla.com/D156420
---
devtools/client/shared/widgets/TableWidget.js | 9 ++-
devtools/client/storage/test/browser.ini | 2 +
.../test/browser_storage_cookies_sort.js | 64 +++++++++++++++++++
.../storage/test/storage-cookies-sort.html | 26 ++++++++
devtools/shared/natural-sort.js | 8 +--
.../tests/xpcshell/test_natural-sort.js | 18 ++++++
6 files changed, 121 insertions(+), 6 deletions(-)
create mode 100644 devtools/client/storage/test/browser_storage_cookies_sort.js
create mode 100644 devtools/client/storage/test/storage-cookies-sort.html
diff --git a/devtools/client/shared/widgets/TableWidget.js b/devtools/client/shared/widgets/TableWidget.js
index f2f9480803d68..c0c0558763bdb 100644
--- a/devtools/client/shared/widgets/TableWidget.js
+++ b/devtools/client/shared/widgets/TableWidget.js
@@ -999,9 +999,16 @@ TableWidget.prototype = {
return;
}
+ // First sort the column to "sort by" explicitly.
const sortedItems = this.columns.get(column).sort([...this.items.values()]);
+
+ // Then, sort all the other columns (id !== column) only based on the
+ // sortedItems provided by the first sort.
+ // Each column keeps track of the fact that it is the "sort by" column or
+ // not, so this will not shuffle the items and will just make sure each
+ // column displays the correct value.
for (const [id, col] of this.columns) {
- if (id === col) {
+ if (id !== column) {
col.sort(sortedItems);
}
}
diff --git a/devtools/client/storage/test/browser.ini b/devtools/client/storage/test/browser.ini
index 186d39e7b740e..a92df77982734 100644
--- a/devtools/client/storage/test/browser.ini
+++ b/devtools/client/storage/test/browser.ini
@@ -11,6 +11,7 @@ support-files =
storage-complex-values.html
storage-cookies.html
storage-cookies-samesite.html
+ storage-cookies-sort.html
storage-dfpi.html
storage-empty-objectstores.html
storage-file-url.html
@@ -62,6 +63,7 @@ skip-if =
os == "linux" && debug && fission && socketprocess_networking # high frequency intermittent
[browser_storage_cookies_samesite.js]
skip-if = true # Bug 1448484 - sameSite1 is "Unset" - Got undefined, expected Unset
+[browser_storage_cookies_sort.js]
[browser_storage_cookies_tab_navigation.js]
[browser_storage_delete.js]
[browser_storage_delete_all.js]
diff --git a/devtools/client/storage/test/browser_storage_cookies_sort.js b/devtools/client/storage/test/browser_storage_cookies_sort.js
new file mode 100644
index 0000000000000..bb9bdbde25b3c
--- /dev/null
+++ b/devtools/client/storage/test/browser_storage_cookies_sort.js
@@ -0,0 +1,64 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Test column sorting works and sorts dates correctly (including "session"
+// cookies).
+
+"use strict";
+
+add_task(async function() {
+ const TEST_URL = MAIN_DOMAIN + "storage-cookies-sort.html";
+ await openTabAndSetupStorage(TEST_URL);
+ showAllColumns(true);
+
+ info("Sort on the expires column, ascending order");
+ clickColumnHeader("expires");
+
+ // Note that here we only specify `test_session` for `test_session1` and
+ // `test_session2`. Since we sort on the "expires" column, there is no point
+ // in asserting the order between those 2 items.
+ checkCells([
+ "test_session",
+ "test_session",
+ "test_hour",
+ "test_day",
+ "test_year",
+ ]);
+
+ info("Sort on the expires column, descending order");
+ clickColumnHeader("expires");
+
+ // Again, only assert `test_session` for `test_session1` and `test_session2`.
+ checkCells([
+ "test_year",
+ "test_day",
+ "test_hour",
+ "test_session",
+ "test_session",
+ ]);
+
+ info("Sort on the name column, ascending order");
+ clickColumnHeader("name");
+ checkCells([
+ "test_day",
+ "test_hour",
+ "test_session1",
+ "test_session2",
+ "test_year",
+ ]);
+});
+
+function checkCells(expected) {
+ const cells = [
+ ...gPanelWindow.document.querySelectorAll("#name .table-widget-cell"),
+ ];
+ cells.forEach(function(cell, i, arr) {
+ // We use startsWith in order to avoid asserting the relative order of
+ // "session" cookies when sorting on the "expires" column.
+ ok(
+ cell.value.startsWith(expected[i]),
+ `Cell value starts with "${expected[i]}".`
+ );
+ });
+}
diff --git a/devtools/client/storage/test/storage-cookies-sort.html b/devtools/client/storage/test/storage-cookies-sort.html
new file mode 100644
index 0000000000000..fb590d5cb2964
--- /dev/null
+++ b/devtools/client/storage/test/storage-cookies-sort.html
@@ -0,0 +1,26 @@
+<!DOCTYPE HTML>
+<html>
+ <!--
+ Bug 970517 - Storage inspector front end - tests
+ -->
+ <head>
+ <meta charset="utf-8">
+ <title>Storage inspector cookie test</title>
+ </head>
+ <body>
+ <script type="application/javascript">
+ "use strict";
+ const ONE_HOUR = 60 * 60 * 1000;
+ const ONE_DAY = 24 * ONE_HOUR;
+ const expiresOneHour = new Date(Date.now() + 1 * ONE_HOUR).toUTCString();
+ const expiresOneDay = new Date(Date.now() + 1 * ONE_DAY).toUTCString();
+ const expiresOneYear = new Date(Date.now() + 365 * ONE_DAY).toUTCString();
+
+ document.cookie = "test_hour=hour;expires=" + expiresOneHour;
+ document.cookie = "test_session1=session1";
+ document.cookie = "test_day=day;expires=" + expiresOneDay;
+ document.cookie = "test_session2=session2";
+ document.cookie = "test_year=year;expires=" + expiresOneYear;
+ </script>
+ </body>
+</html>
diff --git a/devtools/shared/natural-sort.js b/devtools/shared/natural-sort.js
index 5cb41b6dc7483..198980d9cd5a7 100644
--- a/devtools/shared/natural-sort.js
+++ b/devtools/shared/natural-sort.js
@@ -15,7 +15,6 @@
"use strict";
const tokenizeNumbersRx = /(^([+\-]?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?(?=\D|\s|$))|^0x[\da-fA-F]+$|\d+)/g;
-const dateRx = /(^([\w ]+,?[\w ]+)?[\w ]+,?[\w ]+\d+:\d+(:\d+)?[\w ]?|^\d{1,4}[\/\-]\d{1,4}[\/\-]\d{1,4}|^\w+, \w+ \d+, \d{4})/;
const hexRx = /^0x[0-9a-f]+$/i;
const startsWithNullRx = /^\0/;
const endsWithNullRx = /\0$/;
@@ -74,9 +73,7 @@ function naturalSort(a = "", b = "", insensitive = false) {
const aHexOrDate =
parseInt(a.match(hexRx), 16) || (aChunks.length !== 1 && Date.parse(a));
const bHexOrDate =
- parseInt(b.match(hexRx), 16) ||
- (aHexOrDate && b.match(dateRx) && Date.parse(b)) ||
- null;
+ parseInt(b.match(hexRx), 16) || (bChunks.length !== 1 && Date.parse(b));
if (
(aHexOrDate || bHexOrDate) &&
@@ -92,12 +89,13 @@ function naturalSort(a = "", b = "", insensitive = false) {
}
// Try and sort Hex codes or Dates.
- if (bHexOrDate) {
+ if (aHexOrDate && bHexOrDate) {
if (aHexOrDate < bHexOrDate) {
return -1;
} else if (aHexOrDate > bHexOrDate) {
return 1;
}
+ return 0;
}
// Natural sorting through split numeric strings and default strings
diff --git a/devtools/shared/tests/xpcshell/test_natural-sort.js b/devtools/shared/tests/xpcshell/test_natural-sort.js
index 95d52c61aadc0..3e04c947ac007 100644
--- a/devtools/shared/tests/xpcshell/test_natural-sort.js
+++ b/devtools/shared/tests/xpcshell/test_natural-sort.js
@@ -178,6 +178,24 @@ function run_test() {
],
`"${sessionString}" amongst date strings`
);
+ runTest(
+ [
+ "Wed, 04 Sep 2024 09:11:44 GMT",
+ sessionString,
+ "Tue, 06 Sep 2022 09:11:44 GMT",
+ sessionString,
+ "Mon, 05 Sep 2022 09:12:41 GMT",
+ ],
+ [
+ sessionString,
+ sessionString,
+ "Mon, 05 Sep 2022 09:12:41 GMT",
+ "Tue, 06 Sep 2022 09:11:44 GMT",
+ "Wed, 04 Sep 2024 09:11:44 GMT",
+ ],
+ `"${sessionString}" amongst date strings (complex)`
+ );
+
runTest(
[
"Madras",
--
GitLab