Bug 1529018 - Simplify netmonitor DOM/CSS around table elements. r=Honza
authorAlexandre Poirot <poirot.alex@gmail.com>
Fri, 01 Mar 2019 10:08:52 +0000
changeset 519810 6b23faa481d9029dd0f9164b51dedaaca60424ce
parent 519809 f1ba7c69a2c903598ab81d8aaeca4cbbb919042e
child 519811 91d3c9e78ce7d6f4cc7f91617290933c060f7c21
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1529018
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 1529018 - Simplify netmonitor DOM/CSS around table elements. r=Honza * Use the right hiearchy of table elements: <table> <thead> (display: table-header-group) <td>...</td> (display: table-cell) </thead> <tbody> (display: table-row-group) <tr> (display: table-row) <td>...</td> </tr> ... </tbody> </table> * Prevent using position:absolute within the <table> Instead set the overflow: auto on the parent of <table> in order to create an scrollable table. * Remove any wrapper / intermediate elements that would be intermediate childs between <table> and <thead> or between <tbody> and <tr>, ... * Remove manual size definition in JS from RequestListContent. Differential Revision: https://phabricator.services.mozilla.com/D20383
devtools/client/netmonitor/src/assets/styles/RequestList.css
devtools/client/netmonitor/src/components/RequestListContent.js
devtools/client/netmonitor/src/components/RequestListHeader.js
devtools/client/netmonitor/test/browser_net_accessibility-02.js
devtools/client/netmonitor/test/browser_net_autoscroll.js
devtools/client/netmonitor/test/browser_net_columns_showhide.js
devtools/client/netmonitor/test/browser_net_headers-alignment.js
devtools/client/netmonitor/test/browser_net_image-tooltip.js
devtools/client/netmonitor/test/head.js
--- a/devtools/client/netmonitor/src/assets/styles/RequestList.css
+++ b/devtools/client/netmonitor/src/assets/styles/RequestList.css
@@ -46,35 +46,27 @@
   display: flex;
   flex-direction: column;
   width: 100%;
   height: 100%;
   overflow: hidden;
   color: var(--table-text-color);
 }
 
-.requests-list-wrapper {
-  width: 100%;
-  height: 100%;
+.requests-list-scroll {
+  overflow-x: hidden;
+  overflow-y: auto;
 }
 
 .requests-list-table {
   display: table;
-  position: relative;
-  width: 100%;
-  height: 100%;
 }
 
-.requests-list-contents {
+.requests-list-row-group {
   display: table-row-group;
-  position: absolute;
-  overflow-x: hidden;
-  overflow-y: auto;
-  --timings-scale: 1;
-  --timings-rev-scale: 1;
 }
 
 .requests-list-column {
   display: table-cell;
   cursor: default;
   white-space: nowrap;
   overflow: hidden;
   text-overflow: ellipsis;
@@ -84,29 +76,30 @@
 }
 
 .requests-list-column > * {
   display: inline-block;
 }
 
 /* Requests list headers */
 
-.requests-list-headers-wrapper {
+.requests-list-headers-group {
+  display: table-header-group;
+
   position: sticky;
   top: 0;
-  z-index: 1;
+  left: 0;
   width: 100%;
-  padding: 0;
+  z-index: 1;
 }
 
 .requests-list-headers {
-  display: table-header-group;
+  display: table-row;
   height: 24px;
   padding: 0;
-  width: 100%;
 }
 
 .requests-list-headers .requests-list-column:first-child .requests-list-header-button {
   border-width: 0;
 }
 
 .requests-list-header-button {
   background-color: transparent;
@@ -569,23 +562,17 @@
 
 .requests-list-timings-total:dir(rtl) {
   transform-origin: right center;
 }
 
 /* Request list item */
 
 .request-list-item {
-  position: relative;
-/*
   display: table-row;
-  Bug 1431132: Disabling this rule is surprising as we no longer have "rows".
-  But this helps preventing reflowing the whole requests list anytime we append
-  a new request/row.
-*/
   height: 24px;
   line-height: 24px;
 }
 
 .request-list-item.selected {
   background-color: var(--theme-selection-background);
   color: var(--theme-selection-color);
 }
--- a/devtools/client/netmonitor/src/components/RequestListContent.js
+++ b/devtools/client/netmonitor/src/components/RequestListContent.js
@@ -81,73 +81,70 @@ class RequestListContent extends Compone
 
   componentWillMount() {
     this.tooltip = new HTMLTooltip(window.parent.document, { type: "arrow" });
     window.addEventListener("resize", this.onResize);
   }
 
   componentDidMount() {
     // Install event handler for displaying a tooltip
-    this.tooltip.startTogglingOnHover(this.refs.contentEl, this.onHover, {
+    this.tooltip.startTogglingOnHover(this.refs.scrollEl, this.onHover, {
       toggleDelay: REQUESTS_TOOLTIP_TOGGLE_DELAY,
       interactive: true,
     });
     // Install event handler to hide the tooltip on scroll
-    this.refs.contentEl.addEventListener("scroll", this.onScroll, true);
+    this.refs.scrollEl.addEventListener("scroll", this.onScroll, true);
     this.onResize();
   }
 
   componentWillUpdate(nextProps) {
     // Check if the list is scrolled to bottom before the UI update.
-    // The scroll is ever needed only if new rows are added to the list.
-    const delta = nextProps.displayedRequests.size - this.props.displayedRequests.size;
-    this.shouldScrollBottom = delta > 0 && this.isScrolledToBottom();
+    this.shouldScrollBottom = this.isScrolledToBottom();
   }
 
   componentDidUpdate(prevProps) {
-    const node = this.refs.contentEl;
+    const node = this.refs.scrollEl;
     // Keep the list scrolled to bottom if a new row was added
     if (this.shouldScrollBottom && node.scrollTop !== MAX_SCROLL_HEIGHT) {
       // Using maximum scroll height rather than node.scrollHeight to avoid sync reflow.
       node.scrollTop = MAX_SCROLL_HEIGHT;
     }
     if (prevProps.networkDetailsOpen !== this.props.networkDetailsOpen ||
       prevProps.networkDetailsWidth !== this.props.networkDetailsWidth ||
       prevProps.networkDetailsHeight !== this.props.networkDetailsHeight
     ) {
       this.onResize();
     }
   }
 
   componentWillUnmount() {
-    this.refs.contentEl.removeEventListener("scroll", this.onScroll, true);
+    this.refs.scrollEl.removeEventListener("scroll", this.onScroll, true);
 
     // Uninstall the tooltip event handler
     this.tooltip.stopTogglingOnHover();
     window.removeEventListener("resize", this.onResize);
   }
 
   onResize() {
-    const parent = this.refs.contentEl.parentNode;
-    this.refs.contentEl.style.width = parent.offsetWidth + "px";
-    this.refs.contentEl.style.height = parent.offsetHeight + "px";
+    const parent = this.refs.scrollEl.parentNode;
+    this.refs.scrollEl.style.width = parent.offsetWidth + "px";
+    this.refs.scrollEl.style.height = parent.offsetHeight + "px";
   }
 
   isScrolledToBottom() {
-    const { contentEl } = this.refs;
-    const lastChildEl = contentEl.lastElementChild;
+    const { scrollEl, rowGroupEl } = this.refs;
+    const lastChildEl = rowGroupEl.lastElementChild;
 
     if (!lastChildEl) {
       return false;
     }
 
-    const lastChildRect = lastChildEl.getBoundingClientRect();
-    const contentRect = contentEl.getBoundingClientRect();
-
-    return (lastChildRect.height + lastChildRect.top) <= contentRect.bottom;
+    const lastNodeHeight = lastChildEl.clientHeight;
+    return scrollEl.scrollTop + scrollEl.clientHeight >=
+      scrollEl.scrollHeight - lastNodeHeight / 2;
   }
 
   /**
    * The predicate used when deciding whether a popup should be shown
    * over a request item or not.
    *
    * @param Node target
    *        The element node currently being hovered.
@@ -270,26 +267,31 @@ class RequestListContent extends Compone
       onSecurityIconMouseDown,
       onWaterfallMouseDown,
       requestFilterTypes,
       scale,
       selectedRequest,
     } = this.props;
 
     return (
-      div({ className: "requests-list-wrapper" },
-        div({ className: "requests-list-table" },
+      div({
+        ref: "scrollEl",
+        className: "requests-list-scroll",
+      },
+        div({
+          className: "requests-list-table",
+        },
+          RequestListHeader(),
           div({
-            ref: "contentEl",
-            className: "requests-list-contents",
+            ref: "rowGroupEl",
+            className: "requests-list-row-group",
             tabIndex: 0,
             onKeyDown: this.onKeyDown,
             style: { "--timings-scale": scale, "--timings-rev-scale": 1 / scale },
           },
-            RequestListHeader(),
             displayedRequests.map((item, index) => RequestListItem({
               firstRequestStartedMillis,
               fromCache: item.status === "304" || item.fromCache,
               connector,
               columns,
               item,
               index,
               isSelected: item.id === (selectedRequest && selectedRequest.id),
@@ -297,17 +299,17 @@ class RequestListContent extends Compone
               onContextMenu: this.onContextMenu,
               onFocusedNodeChange: this.onFocusedNodeChange,
               onMouseDown: () => onItemMouseDown(item.id),
               onCauseBadgeMouseDown: () => onCauseBadgeMouseDown(item.cause),
               onSecurityIconMouseDown: () => onSecurityIconMouseDown(item.securityState),
               onWaterfallMouseDown: () => onWaterfallMouseDown(),
               requestFilterTypes,
             }))
-          )
+          ) // end of requests-list-row-group">
         )
       )
     );
   }
 }
 
 module.exports = connect(
   (state) => ({
--- a/devtools/client/netmonitor/src/components/RequestListHeader.js
+++ b/devtools/client/netmonitor/src/components/RequestListHeader.js
@@ -162,17 +162,17 @@ class RequestListHeader extends Componen
 
     return div({ className }, label);
   }
 
   render() {
     const { columns, scale, sort, sortBy, waterfallWidth } = this.props;
 
     return (
-      div({ className: "devtools-toolbar requests-list-headers-wrapper" },
+      div({ className: "devtools-toolbar requests-list-headers-group" },
         div({
           className: "devtools-toolbar requests-list-headers",
           onContextMenu: this.onContextMenu,
         },
           HEADERS.filter((header) => columns[header.name]).map((header) => {
             const name = header.name;
             const boxName = header.boxName || name;
             const label = header.noLocalization
--- a/devtools/client/netmonitor/test/browser_net_accessibility-02.js
+++ b/devtools/client/netmonitor/test/browser_net_accessibility-02.js
@@ -28,17 +28,17 @@ add_task(async function() {
       "The selected item in the requests menu was incorrect.");
     is(!!document.querySelector(".network-details-panel"), panelVisibility,
       "The network details panel should render correctly.");
   }
 
   // Execute requests.
   await performRequests(monitor, tab, 2);
 
-  document.querySelector(".requests-list-contents").focus();
+  document.querySelector(".requests-list-row-group").focus();
 
   check(-1, false);
 
   EventUtils.sendKey("DOWN", window);
   check(0, true);
   EventUtils.sendKey("UP", window);
   check(0, true);
 
--- a/devtools/client/netmonitor/test/browser_net_autoscroll.js
+++ b/devtools/client/netmonitor/test/browser_net_autoscroll.js
@@ -14,17 +14,17 @@ add_task(async function() {
   const { document, windowRequire, store } = monitor.panelWin;
   const Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
 
   store.dispatch(Actions.batchEnable(false));
 
   // Wait until the first request makes the empty notice disappear
   await waitForRequestListToAppear();
 
-  const requestsContainer = document.querySelector(".requests-list-contents");
+  const requestsContainer = document.querySelector(".requests-list-scroll");
   ok(requestsContainer, "Container element exists as expected.");
 
   // (1) Check that the scroll position is maintained at the bottom
   // when the requests overflow the vertical size of the container.
   await waitForRequestsToOverflowContainer();
   await waitForScroll();
   ok(true, "Scrolled to bottom on overflow.");
 
@@ -48,39 +48,39 @@ add_task(async function() {
   ok(true, "Still scrolled to bottom.");
 
   // (4) Now select the first item in the list
   // and check that additional requests do not change the scroll position
   // from just below the headers.
   store.dispatch(Actions.selectRequestByIndex(0));
   await waitForNetworkEvents(monitor, 8);
   await waitSomeTime();
-  const requestsContainerHeaders = requestsContainer.firstChild;
+  const requestsContainerHeaders = document.querySelector(".requests-list-headers");
   const headersHeight = requestsContainerHeaders.offsetHeight;
   is(requestsContainer.scrollTop, headersHeight, "Did not scroll.");
 
   // Stop doing requests.
   await ContentTask.spawn(tab.linkedBrowser, {}, function() {
     content.wrappedJSObject.stopRequests();
   });
 
   // Done: clean up.
   return teardown(monitor);
 
   function waitForRequestListToAppear() {
     info("Waiting until the empty notice disappears and is replaced with the list");
-    return waitUntil(() => !!document.querySelector(".requests-list-contents"));
+    return waitUntil(() => !!document.querySelector(".requests-list-row-group"));
   }
 
   async function waitForRequestsToOverflowContainer() {
     info("Waiting for enough requests to overflow the container");
     while (true) {
       info("Waiting for one network request");
       await waitForNetworkEvents(monitor, 1);
-      if (requestsContainer.scrollHeight > requestsContainer.clientHeight) {
+      if (requestsContainer.scrollHeight > requestsContainer.clientHeight + 50) {
         info("The list is long enough, returning");
         return;
       }
     }
   }
 
   function scrolledToBottom(element) {
     return element.scrollTop + element.clientHeight >= element.scrollHeight;
--- a/devtools/client/netmonitor/test/browser_net_columns_showhide.js
+++ b/devtools/client/netmonitor/test/browser_net_columns_showhide.js
@@ -22,17 +22,17 @@ add_task(async function() {
 
   const item = getSortedRequests(store.getState()).get(0);
   ok(item.responseHeadersAvailable, "headers are available for lazily fetching");
 
   if (item.responseHeadersAvailable && !item.responseHeaders) {
     await requestData(item.id, "responseHeaders");
   }
 
-  const requestsContainer = document.querySelector(".requests-list-contents");
+  const requestsContainer = document.querySelector(".requests-list-row-group");
   ok(requestsContainer, "Container element exists as expected.");
   const headers = document.querySelector(".requests-list-headers");
 
   let columns = store.getState().ui.columns;
   for (const column in columns) {
     if (columns[column]) {
       await testVisibleColumnContextMenuItem(column, document, parent);
       testColumnsAlignment(headers, requestsContainer);
@@ -56,17 +56,17 @@ add_task(async function() {
 });
 
 async function testWhiteSpaceContextMenuItem(column, document, parent) {
   ok(!document.querySelector(`#requests-list-${column}-button`),
      `Column ${column} should be hidden`);
 
   info(`Right clicking on white-space in the header to get the context menu`);
   EventUtils.sendMouseEvent({ type: "contextmenu" },
-    document.querySelector(".devtools-toolbar.requests-list-headers"));
+    document.querySelector(".requests-list-headers"));
 
   // Wait for next tick to do stuff async and force repaint.
   await waitForTick();
   await toggleAndCheckColumnVisibility(column, document, parent);
 }
 
 async function testVisibleColumnContextMenuItem(column, document, parent) {
   ok(document.querySelector(`#requests-list-${column}-button`),
--- a/devtools/client/netmonitor/test/browser_net_headers-alignment.js
+++ b/devtools/client/netmonitor/test/browser_net_headers-alignment.js
@@ -14,42 +14,43 @@ add_task(async function() {
   const { document, windowRequire, store } = monitor.panelWin;
   const Actions = windowRequire("devtools/client/netmonitor/src/actions/index");
 
   store.dispatch(Actions.batchEnable(false));
 
   // Wait until the first request makes the empty notice disappear
   await waitForRequestListToAppear();
 
-  const requestsContainer = document.querySelector(".requests-list-contents");
-  ok(requestsContainer, "Container element exists as expected.");
+  const requestsContainerScroll = document.querySelector(".requests-list-scroll");
+  ok(requestsContainerScroll, "Container element exists as expected.");
+  const requestsContainer = document.querySelector(".requests-list-row-group");
   const headers = document.querySelector(".requests-list-headers");
   ok(headers, "Headers element exists as expected.");
 
-  await waitForRequestsToOverflowContainer(monitor, requestsContainer);
+  await waitForRequestsToOverflowContainer(monitor, requestsContainerScroll);
 
   testColumnsAlignment(headers, requestsContainer);
 
   // Stop doing requests.
   await ContentTask.spawn(tab.linkedBrowser, {}, async function() {
     content.wrappedJSObject.stopRequests();
   });
 
   // Done: clean up.
   return teardown(monitor);
 
   function waitForRequestListToAppear() {
     info("Waiting until the empty notice disappears and is replaced with the list");
-    return waitUntil(() => !!document.querySelector(".requests-list-contents"));
+    return waitUntil(() => !!document.querySelector(".requests-list-row-group"));
   }
 });
 
 async function waitForRequestsToOverflowContainer(monitor, requestList) {
   info("Waiting for enough requests to overflow the container");
   while (true) {
     info("Waiting for one network request");
     await waitForNetworkEvents(monitor, 1);
-    if (requestList.scrollHeight > requestList.clientHeight) {
+    if (requestList.scrollHeight > requestList.clientHeight + 50) {
       info("The list is long enough, returning");
       return;
     }
   }
 }
--- a/devtools/client/netmonitor/test/browser_net_image-tooltip.js
+++ b/devtools/client/netmonitor/test/browser_net_image-tooltip.js
@@ -41,17 +41,17 @@ add_task(async function test() {
     content.wrappedJSObject.performRequests();
   });
   await onEvents;
 
   info("Checking the image thumbnail after a reload.");
   await showTooltipAndVerify(document.querySelectorAll(".request-list-item")[1]);
 
   info("Checking if the image thumbnail is hidden when mouse leaves the menu widget");
-  const requestsListContents = document.querySelector(".requests-list-contents");
+  const requestsListContents = document.querySelector(".requests-list-row-group");
   EventUtils.synthesizeMouse(requestsListContents, 0, 0, { type: "mousemove" },
                              monitor.panelWin);
   await waitUntil(() => !toolboxDoc.querySelector(".tooltip-container.tooltip-visible"));
 
   await teardown(monitor);
 
   /**
    * Show a tooltip on the {target} and verify that it was displayed
--- a/devtools/client/netmonitor/test/head.js
+++ b/devtools/client/netmonitor/test/head.js
@@ -683,18 +683,17 @@ function waitForContentMessage(name) {
     mm.addMessageListener(name, function onMessage(msg) {
       mm.removeMessageListener(name, onMessage);
       resolve(msg);
     });
   });
 }
 
 function testColumnsAlignment(headers, requestList) {
-  // Get first request line, not child 0 as this is the headers
-  const firstRequestLine = requestList.childNodes[1];
+  const firstRequestLine = requestList.childNodes[0];
 
   // Find number of columns
   const numberOfColumns = headers.childElementCount;
   for (let i = 0; i < numberOfColumns; i++) {
     const headerColumn = headers.childNodes[i];
     const requestColumn = firstRequestLine.childNodes[i];
     is(headerColumn.getBoundingClientRect().left,
        requestColumn.getBoundingClientRect().left,
@@ -703,32 +702,32 @@ function testColumnsAlignment(headers, r
   }
 }
 
 async function hideColumn(monitor, column) {
   const { document, parent } = monitor.panelWin;
 
   info(`Clicking context-menu item for ${column}`);
   EventUtils.sendMouseEvent({ type: "contextmenu" },
-    document.querySelector(".devtools-toolbar.requests-list-headers"));
+    document.querySelector(".requests-list-headers"));
 
   const onHeaderRemoved = waitForDOM(document, `#requests-list-${column}-button`, 0);
   parent.document.querySelector(`#request-list-header-${column}-toggle`).click();
   await onHeaderRemoved;
 
   ok(!document.querySelector(`#requests-list-${column}-button`),
      `Column ${column} should be hidden`);
 }
 
 async function showColumn(monitor, column) {
   const { document, parent } = monitor.panelWin;
 
   info(`Clicking context-menu item for ${column}`);
   EventUtils.sendMouseEvent({ type: "contextmenu" },
-    document.querySelector(".devtools-toolbar.requests-list-headers"));
+    document.querySelector(".requests-list-headers"));
 
   const onHeaderAdded = waitForDOM(document, `#requests-list-${column}-button`, 1);
   parent.document.querySelector(`#request-list-header-${column}-toggle`).click();
   await onHeaderAdded;
 
   ok(document.querySelector(`#requests-list-${column}-button`),
      `Column ${column} should be visible`);
 }