Bug 1454962 - Add ways to reset sorting. r=Honza
authorMrigank Krishan <mrigankkrishan@gmail.com>
Tue, 12 Mar 2019 12:35:47 +0000
changeset 524513 a28e95a5730fb3db52d36f92df2c4876160bf36f
parent 524512 18fa4e4ce0351ff8897eb13f4378a4fe15e7b2b3
child 524514 ee300588de96f75bb1d205fa9c6e8da3a41a4627
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1454962
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1454962 - Add ways to reset sorting. r=Honza As suggested by @Harald, I've added two ways to "Reset state": - Middle click on request list header. - "Reset Columns" option in the context menu will also sort by timeline. Differential Revision: https://phabricator.services.mozilla.com/D22064
devtools/client/locales/en-US/netmonitor.properties
devtools/client/netmonitor/src/components/RequestListHeader.js
devtools/client/netmonitor/src/reducers/sort.js
devtools/client/netmonitor/src/widgets/RequestListHeaderContextMenu.js
devtools/client/netmonitor/test/browser.ini
devtools/client/netmonitor/test/browser_net_sort-reset.js
--- a/devtools/client/locales/en-US/netmonitor.properties
+++ b/devtools/client/locales/en-US/netmonitor.properties
@@ -715,16 +715,20 @@ netmonitor.toolbar.toggleRecording=Pause
 # LOCALIZATION NOTE (netmonitor.toolbar.perf): This is the label displayed
 # in the network toolbar for the performance analysis button.
 netmonitor.toolbar.perf=Toggle performance analysis…
 
 # LOCALIZATION NOTE (netmonitor.toolbar.resetColumns): This is the label
 # displayed in the network table header context menu.
 netmonitor.toolbar.resetColumns=Reset Columns
 
+# LOCALIZATION NOTE (netmonitor.toolbar.resetSorting): This is the label
+# displayed in the network table header context menu to reset sorting
+netmonitor.toolbar.resetSorting=Reset Sorting
+
 # LOCALIZATION NOTE (netmonitor.toolbar.timings): This is the label
 # displayed in the network table header context menu for the timing submenu
 netmonitor.toolbar.timings=Timings
 
 # LOCALIZATION NOTE (netmonitor.toolbar.responseHeaders): This is the
 # label displayed in the network table header context menu for the
 # response headers submenu.
 netmonitor.toolbar.responseHeaders=Response Headers
--- a/devtools/client/netmonitor/src/components/RequestListHeader.js
+++ b/devtools/client/netmonitor/src/components/RequestListHeader.js
@@ -39,16 +39,17 @@ const RESIZE_COLUMNS =
  * Displays tick marks in the waterfall column header.
  * Also draws the waterfall background canvas and updates it when needed.
  */
 class RequestListHeader extends Component {
   static get propTypes() {
     return {
       columns: PropTypes.object.isRequired,
       resetColumns: PropTypes.func.isRequired,
+      resetSorting: PropTypes.func.isRequired,
       resizeWaterfall: PropTypes.func.isRequired,
       scale: PropTypes.number,
       sort: PropTypes.object,
       sortBy: PropTypes.func.isRequired,
       toggleColumn: PropTypes.func.isRequired,
       waterfallWidth: PropTypes.number,
       columnsData: PropTypes.object.isRequired,
       setColumnsWidth: PropTypes.func.isRequired,
@@ -59,22 +60,24 @@ class RequestListHeader extends Componen
     super(props);
     this.requestListHeader = createRef();
 
     this.onContextMenu = this.onContextMenu.bind(this);
     this.drawBackground = this.drawBackground.bind(this);
     this.resizeWaterfall = this.resizeWaterfall.bind(this);
     this.waterfallDivisionLabels = this.waterfallDivisionLabels.bind(this);
     this.waterfallLabel = this.waterfallLabel.bind(this);
+    this.onHeaderClick = this.onHeaderClick.bind(this);
   }
 
   componentWillMount() {
-    const { resetColumns, toggleColumn } = this.props;
+    const { resetColumns, resetSorting, toggleColumn } = this.props;
     this.contextMenu = new RequestListHeaderContextMenu({
       resetColumns,
+      resetSorting,
       toggleColumn,
     });
   }
 
   componentDidMount() {
     // Create the object that takes care of drawing the waterfall canvas background
     this.background = new WaterfallBackground(document);
     this.drawBackground();
@@ -104,16 +107,26 @@ class RequestListHeader extends Componen
     removeThemeObserver(this.drawBackground);
   }
 
   onContextMenu(evt) {
     evt.preventDefault();
     this.contextMenu.open(evt, this.props.columns);
   }
 
+  onHeaderClick(evt, headerName) {
+    const { sortBy, resetSorting } = this.props;
+    if (evt.button == 1) {
+      // reset sort state on middle click
+      resetSorting();
+    } else {
+      sortBy(headerName);
+    }
+  }
+
   drawBackground() {
     // The background component is theme dependent, so add the current theme to the props.
     const props = Object.assign({}, this.props, {
       theme: getTheme(),
     });
     this.background.draw(props);
   }
 
@@ -480,17 +493,17 @@ class RequestListHeader extends Componen
     const columnsData = this.props.columnsData;
     const visibleColumns = this.getVisibleColumns();
     const lastVisibleColumn = visibleColumns[visibleColumns.length - 1].name;
     const name = header.name;
     const boxName = header.boxName || name;
     const label = header.noLocalization
       ? name : L10N.getStr(`netmonitor.toolbar.${header.label || name}`);
 
-    const { scale, sort, sortBy, waterfallWidth } = this.props;
+    const { scale, sort, waterfallWidth } = this.props;
     let sorted, sortedTitle;
     const active = sort.type == name ? true : undefined;
 
     if (active) {
       sorted = sort.ascending ? "ascending" : "descending";
       sortedTitle = L10N.getStr(sort.ascending
         ? "networkMenu.sortedAsc"
         : "networkMenu.sortedDesc");
@@ -525,17 +538,17 @@ class RequestListHeader extends Componen
         // Used to style the next column.
         "data-active": active,
       },
         button({
           id: `requests-list-${name}-button`,
           className: `requests-list-header-button`,
           "data-sorted": sorted,
           title: sortedTitle ? `${label} (${sortedTitle})` : label,
-          onClick: () => sortBy(name),
+          onClick: (evt) => this.onHeaderClick(evt, name),
         },
           name === "waterfall"
             ? this.waterfallLabel(waterfallWidth, scale, label)
             : div({ className: "button-text" }, label),
           div({ className: "button-icon" })
         ),
         (name !== lastVisibleColumn) && draggable
       )
@@ -574,14 +587,15 @@ module.exports = connect(
     firstRequestStartedMillis: state.requests.firstStartedMillis,
     scale: getWaterfallScale(state),
     sort: state.sort,
     timingMarkers: state.timingMarkers,
     waterfallWidth: state.ui.waterfallWidth,
   }),
   (dispatch) => ({
     resetColumns: () => dispatch(Actions.resetColumns()),
+    resetSorting: () => dispatch(Actions.sortBy(null)),
     resizeWaterfall: (width) => dispatch(Actions.resizeWaterfall(width)),
     sortBy: (type) => dispatch(Actions.sortBy(type)),
     toggleColumn: (column) => dispatch(Actions.toggleColumn(column)),
     setColumnsWidth: (widths) => dispatch(Actions.setColumnsWidth(widths)),
   })
 )(RequestListHeader);
--- a/devtools/client/netmonitor/src/reducers/sort.js
+++ b/devtools/client/netmonitor/src/reducers/sort.js
@@ -1,36 +1,44 @@
 /* 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/. */
 
 "use strict";
 
-const { SORT_BY } = require("../constants");
+const { SORT_BY, RESET_COLUMNS } = require("../constants");
 
 function Sort() {
   return {
     // null means: sort by "waterfall", but don't highlight the table header
     type: null,
     ascending: true,
   };
 }
 
 function sortReducer(state = new Sort(), action) {
   switch (action.type) {
     case SORT_BY: {
       state = { ...state };
-      if (action.sortType == state.type) {
+      if (action.sortType != null && action.sortType == state.type) {
         state.ascending = !state.ascending;
       } else {
         state.type = action.sortType;
         state.ascending = true;
       }
       return state;
     }
+
+    case RESET_COLUMNS: {
+      state = { ...state };
+      state.type = null;
+      state.ascending = true;
+      return state;
+    }
+
     default:
       return state;
   }
 }
 
 module.exports = {
   Sort,
   sortReducer,
--- a/devtools/client/netmonitor/src/widgets/RequestListHeaderContextMenu.js
+++ b/devtools/client/netmonitor/src/widgets/RequestListHeaderContextMenu.js
@@ -65,16 +65,22 @@ class RequestListHeaderContextMenu {
 
     menu.push({ type: "separator" });
     menu.push({
       id: "request-list-header-reset-columns",
       label: L10N.getStr("netmonitor.toolbar.resetColumns"),
       click: () => this.props.resetColumns(),
     });
 
+    menu.push({
+      id: "request-list-header-reset-sorting",
+      label: L10N.getStr("netmonitor.toolbar.resetSorting"),
+      click: () => this.props.resetSorting(),
+    });
+
     showMenu(menu, {
       screenX: event.screenX,
       screenY: event.screenY,
     });
   }
 }
 
 module.exports = RequestListHeaderContextMenu;
--- a/devtools/client/netmonitor/test/browser.ini
+++ b/devtools/client/netmonitor/test/browser.ini
@@ -185,16 +185,17 @@ skip-if = os == 'win' # bug 1391264
 [browser_net_send-beacon-other-tab.js]
 [browser_net_set-cookie-same-site.js]
 [browser_net_simple-request-data.js]
 [browser_net_simple-request-details.js]
 skip-if = true # Bug 1258809
 [browser_net_simple-request.js]
 [browser_net_sort-01.js]
 [browser_net_sort-02.js]
+[browser_net_sort-reset.js]
 [browser_net_statistics-01.js]
 skip-if = true # Bug 1373558
 [browser_net_statistics-02.js]
 [browser_net_status-bar-transferred-size.js]
 [browser_net_status-bar.js]
 [browser_net_status-codes.js]
 [browser_net_streaming-response.js]
 [browser_net_telemetry_edit_resend.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/netmonitor/test/browser_net_sort-reset.js
@@ -0,0 +1,220 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * Test if sorting columns in the network table works correctly.
+ */
+
+add_task(async function() {
+  const { L10N } = require("devtools/client/netmonitor/src/utils/l10n");
+
+  const { monitor } = await initNetMonitor(SORTING_URL);
+  info("Starting test... ");
+
+  // It seems that this test may be slow on debug builds. This could be because
+  // of the heavy dom manipulation associated with sorting.
+  requestLongerTimeout(2);
+
+  const { parent, document, store, windowRequire } = monitor.panelWin;
+  const Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
+  const {
+    getDisplayedRequests,
+    getSelectedRequest,
+    getSortedRequests,
+  } = windowRequire("devtools/client/netmonitor/src/selectors/index");
+
+  store.dispatch(Actions.batchEnable(false));
+
+  // Loading the frame script and preparing the xhr request URLs so we can
+  // generate some requests later.
+  loadFrameScriptUtils();
+  const requests = [{
+    url: "sjs_sorting-test-server.sjs?index=1&" + Math.random(),
+    method: "GET1",
+  }, {
+    url: "sjs_sorting-test-server.sjs?index=5&" + Math.random(),
+    method: "GET5",
+  }, {
+    url: "sjs_sorting-test-server.sjs?index=2&" + Math.random(),
+    method: "GET2",
+  }, {
+    url: "sjs_sorting-test-server.sjs?index=4&" + Math.random(),
+    method: "GET4",
+  }, {
+    url: "sjs_sorting-test-server.sjs?index=3&" + Math.random(),
+    method: "GET3",
+  }];
+
+  const wait = waitForNetworkEvents(monitor, 5);
+  await performRequestsInContent(requests);
+  await wait;
+
+  store.dispatch(Actions.toggleNetworkDetails());
+
+  testHeaders();
+  await testContents([0, 2, 4, 3, 1]);
+
+  EventUtils.sendMouseEvent({ type: "click" },
+    document.querySelector("#requests-list-status-button"));
+  info("Testing sort reset using middle click.");
+  EventUtils.sendMouseEvent({ type: "click", button: 1 },
+    document.querySelector("#requests-list-status-button"));
+  testHeaders();
+  await testContents([0, 2, 4, 3, 1]);
+
+  EventUtils.sendMouseEvent({ type: "click" },
+    document.querySelector("#requests-list-status-button"));
+  info("Testing sort reset using context menu 'Reset Sorting'");
+  EventUtils.sendMouseEvent({ type: "contextmenu" },
+    document.querySelector("#requests-list-contentSize-button"));
+  parent.document.querySelector("#request-list-header-reset-sorting").click();
+  testHeaders();
+  await testContents([0, 2, 4, 3, 1]);
+
+  EventUtils.sendMouseEvent({ type: "click" },
+    document.querySelector("#requests-list-status-button"));
+  info("Testing sort reset using context menu 'Reset Columns'");
+  EventUtils.sendMouseEvent({ type: "contextmenu" },
+    document.querySelector("#requests-list-contentSize-button"));
+  parent.document.querySelector("#request-list-header-reset-columns").click();
+  testHeaders();
+  // add columns because verifyRequestItemTarget expects some extra columns
+  showColumn(monitor, "protocol");
+  showColumn(monitor, "remoteip");
+  showColumn(monitor, "scheme");
+  showColumn(monitor, "duration");
+  showColumn(monitor, "latency");
+  await testContents([0, 2, 4, 3, 1]);
+
+  return teardown(monitor);
+
+  function getSelectedIndex(state) {
+    if (!state.requests.selectedId) {
+      return -1;
+    }
+    return getSortedRequests(state).findIndex(r => r.id === state.requests.selectedId);
+  }
+
+  function testHeaders(sortType, direction) {
+    const doc = monitor.panelWin.document;
+    const target = doc.querySelector("#requests-list-" + sortType + "-button");
+    const headers = doc.querySelectorAll(".requests-list-header-button");
+
+    for (const header of headers) {
+      if (header != target) {
+        ok(!header.hasAttribute("data-sorted"),
+          "The " + header.id + " header does not have a 'data-sorted' attribute.");
+        ok(!header.getAttribute("title").includes(L10N.getStr("networkMenu.sortedAsc")) &&
+          !header.getAttribute("title").includes(L10N.getStr("networkMenu.sortedDesc")),
+          "The " + header.id +
+          " header does not include any sorting in the 'title' attribute.");
+      } else {
+        is(header.getAttribute("data-sorted"), direction,
+          "The " + header.id + " header has a correct 'data-sorted' attribute.");
+        const sorted = direction == "ascending"
+          ? L10N.getStr("networkMenu.sortedAsc")
+          : L10N.getStr("networkMenu.sortedDesc");
+        ok(header.getAttribute("title").includes(sorted),
+          "The " + header.id +
+          " header includes the used sorting in the 'title' attribute.");
+      }
+    }
+  }
+
+  async function testContents([a, b, c, d, e]) {
+    isnot(getSelectedRequest(store.getState()), undefined,
+      "There should still be a selected item after sorting.");
+    is(getSelectedIndex(store.getState()), a,
+      "The first item should be still selected after sorting.");
+    is(!!document.querySelector(".network-details-panel"), true,
+      "The network details panel should still be visible after sorting.");
+
+    is(getSortedRequests(store.getState()).length, 5,
+      "There should be a total of 5 items in the requests menu.");
+    is(getDisplayedRequests(store.getState()).length, 5,
+      "There should be a total of 5 visible items in the requests menu.");
+    is(document.querySelectorAll(".request-list-item").length, 5,
+      "The visible items in the requests menu are, in fact, visible!");
+
+    const requestItems = document.querySelectorAll(".request-list-item");
+    for (const requestItem of requestItems) {
+      requestItem.scrollIntoView();
+      const requestsListStatus = requestItem.querySelector(".status-code");
+      EventUtils.sendMouseEvent({ type: "mouseover" }, requestsListStatus);
+      await waitUntil(() => requestsListStatus.title);
+    }
+
+    verifyRequestItemTarget(
+      document,
+      getDisplayedRequests(store.getState()),
+      getSortedRequests(store.getState()).get(a),
+      "GET1", SORTING_SJS + "?index=1", {
+        fuzzyUrl: true,
+        status: 101,
+        statusText: "Meh",
+        type: "1",
+        fullMimeType: "text/1",
+        transferred: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 198),
+        size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 0),
+        time: true,
+      });
+    verifyRequestItemTarget(
+      document,
+      getDisplayedRequests(store.getState()),
+      getSortedRequests(store.getState()).get(b),
+      "GET2", SORTING_SJS + "?index=2", {
+        fuzzyUrl: true,
+        status: 200,
+        statusText: "Meh",
+        type: "2",
+        fullMimeType: "text/2",
+        transferred: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 217),
+        size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 19),
+        time: true,
+      });
+    verifyRequestItemTarget(
+      document,
+      getDisplayedRequests(store.getState()),
+      getSortedRequests(store.getState()).get(c),
+      "GET3", SORTING_SJS + "?index=3", {
+        fuzzyUrl: true,
+        status: 300,
+        statusText: "Meh",
+        type: "3",
+        fullMimeType: "text/3",
+        transferred: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 227),
+        size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 29),
+        time: true,
+      });
+    verifyRequestItemTarget(
+      document,
+      getDisplayedRequests(store.getState()),
+      getSortedRequests(store.getState()).get(d),
+      "GET4", SORTING_SJS + "?index=4", {
+        fuzzyUrl: true,
+        status: 400,
+        statusText: "Meh",
+        type: "4",
+        fullMimeType: "text/4",
+        transferred: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 237),
+        size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 39),
+        time: true,
+      });
+    verifyRequestItemTarget(
+      document,
+      getDisplayedRequests(store.getState()),
+      getSortedRequests(store.getState()).get(e),
+      "GET5", SORTING_SJS + "?index=5", {
+        fuzzyUrl: true,
+        status: 500,
+        statusText: "Meh",
+        type: "5",
+        fullMimeType: "text/5",
+        transferred: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 247),
+        size: L10N.getFormatStrWithNumbers("networkMenu.sizeB", 49),
+        time: true,
+      });
+  }
+});