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

--HG--
extra : moz-landing-system : lando

devtools/client/netmonitor/src/assets/styles/RequestList.css

@@ -51,25 +51,17 @@
   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 {
@@ -89,19 +81,20 @@
 
 /* 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 {
@@ -574,13 +567,7 @@
 /* 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;
 }

devtools/client/netmonitor/src/components/RequestListContent.js

@@ -86,24 +86,22 @@ class RequestListContent extends Component {
 
   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.
@@ -118,7 +116,7 @@ class RequestListContent extends Component {
   }
 
   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();
@@ -126,23 +124,22 @@ class RequestListContent extends Component {
   }
 
   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;
   }
 
   /**
@@ -275,16 +272,21 @@ class RequestListContent extends Component {
     } = 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,
@@ -302,7 +304,7 @@ class RequestListContent extends Component {
               onWaterfallMouseDown: () => onWaterfallMouseDown(),
               requestFilterTypes,
             }))
-          )
+          ) // end of requests-list-row-group">
         )
       )
     );

devtools/client/netmonitor/src/components/RequestListHeader.js

@@ -167,7 +167,7 @@ class RequestListHeader extends Component {
     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,

devtools/client/netmonitor/test/browser_net_accessibility-02.js

@@ -33,7 +33,7 @@ add_task(async function() {
   // Execute requests.
   await performRequests(monitor, tab, 2);
 
-  document.querySelector(".requests-list-contents").focus();
+  document.querySelector(".requests-list-row-group").focus();
 
   check(-1, false);
 

devtools/client/netmonitor/test/browser_net_autoscroll.js

@@ -19,7 +19,7 @@ add_task(async function() {
   // 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
@@ -53,7 +53,7 @@ add_task(async function() {
   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.");
 
@@ -67,7 +67,7 @@ add_task(async function() {
 
   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() {
@@ -75,7 +75,7 @@ add_task(async function() {
     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;
       }

devtools/client/netmonitor/test/browser_net_columns_showhide.js

@@ -27,7 +27,7 @@ add_task(async function() {
     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");
 
@@ -61,7 +61,7 @@ async function testWhiteSpaceContextMenuItem(column, document, parent) {
 
   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();

devtools/client/netmonitor/test/browser_net_headers-alignment.js

@@ -19,12 +19,13 @@ add_task(async function() {
   // 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);
 
@@ -38,7 +39,7 @@ add_task(async function() {
 
   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"));
   }
 });
 
@@ -47,7 +48,7 @@ async function waitForRequestsToOverflowContainer(monitor, requestList) {
   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;
     }

devtools/client/netmonitor/test/browser_net_image-tooltip.js

@@ -46,7 +46,7 @@ add_task(async function test() {
   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"));

devtools/client/netmonitor/test/head.js

@@ -688,8 +688,7 @@ function waitForContentMessage(name) {
 }
 
 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;
@@ -708,7 +707,7 @@ async function hideColumn(monitor, column) {
 
   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();
@@ -723,7 +722,7 @@ async function showColumn(monitor, column) {
 
   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();