Bug 1844092 - [devtools] Make Stacktrace frame location keyboard accessible. r=devtools-reviewers,bomsy.

The frame elements already had the correct roles assigned, but this was made completely useless because we had a `display: contents` on them, which strips any accessibility role. This was done for layout purpose that we now can do with subgrids, so we update the component's CSS. We also update the markup so copying stacktrace still works as expected. A webconsole test is added to ensure those stactkrace frames can be navigated with the keyboard. Differential Revision: https://phabricator.services.mozilla.com/D190594
2023-10-23 09:19:22 +00:00 · 2023-10-23 09:19:22 +00:00 · e98d45f7b3
--- a/devtools/client/debugger/src/components/SecondaryPanes/Frames/Frame.js
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Frames/Frame.js
@ -98,6 +98,7 @@ export default class FrameComponent extends Component {
      frame: PropTypes.object.isRequired,
      getFrameTitle: PropTypes.func,
      hideLocation: PropTypes.bool.isRequired,
+      isInGroup: PropTypes.bool,
      panel: PropTypes.oneOf(["debugger", "webconsole"]).isRequired,
      selectFrame: PropTypes.func.isRequired,
      selectedFrame: PropTypes.object,
@ -149,6 +150,7 @@ export default class FrameComponent extends Component {
      getFrameTitle,
      disableContextMenu,
      shouldDisplayOriginalLocation,
+      isInGroup,
    } = this.props;
    const { l10n } = this.context;

@ -193,7 +195,10 @@ export default class FrameComponent extends Component {
              className: "clipboard-only",
            })
        ),
-      this.isSelectable && React.createElement(FrameIndent, null),
+      this.isSelectable &&
+        React.createElement(FrameIndent, {
+          indentLevel: isInGroup ? 2 : 1,
+        }),
      React.createElement(FrameTitle, {
        frame,
        options: {
--- a/devtools/client/debugger/src/components/SecondaryPanes/Frames/FrameIndent.js
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Frames/FrameIndent.js
@ -4,9 +4,10 @@

 import React from "react";

-export default function FrameIndent() {
+export default function FrameIndent({ indentLevel = 1 } = {}) {
  // \xA0 represents the non breakable space &nbsp;
-  const nonBreakableSpaces = "\xA0\xA0\xA0\xA0";
+  const indentWidth = 4 * indentLevel;
+  const nonBreakableSpaces = "\xA0".repeat(indentWidth);
  return React.createElement(
    "span",
    {
--- a/devtools/client/debugger/src/components/SecondaryPanes/Frames/Group.js
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Frames/Group.js
@ -111,31 +111,23 @@ export default class Group extends Component {
      {
        className: "frames-list",
      },
-      group.reduce((acc, frame, i) => {
-        if (this.isSelectable) {
-          acc.push(
-            React.createElement(FrameIndent, {
-              key: `frame-indent-${i}`,
-            })
-          );
-        }
-        return acc.concat(
-          React.createElement(FrameComponent, {
-            frame: frame,
-            showFrameContextMenu: showFrameContextMenu,
-            hideLocation: true,
-            key: frame.id,
-            selectedFrame: selectedFrame,
-            selectFrame: selectFrame,
-            selectLocation: selectLocation,
-            shouldMapDisplayName: false,
-            displayFullUrl: displayFullUrl,
-            getFrameTitle: getFrameTitle,
-            disableContextMenu: disableContextMenu,
-            panel: panel,
-          })
-        );
-      }, [])
+      group.map(frame =>
+        React.createElement(FrameComponent, {
+          frame: frame,
+          showFrameContextMenu: showFrameContextMenu,
+          hideLocation: true,
+          key: frame.id,
+          selectedFrame: selectedFrame,
+          selectFrame: selectFrame,
+          selectLocation: selectLocation,
+          shouldMapDisplayName: false,
+          displayFullUrl: displayFullUrl,
+          getFrameTitle: getFrameTitle,
+          disableContextMenu: disableContextMenu,
+          panel: panel,
+          isInGroup: true,
+        })
+      )
    );
  }

--- a/devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/snapshots/Frame.spec.js.snap
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/snapshots/Frame.spec.js.snap
@ -11,7 +11,9 @@ exports[`Frame getFrameTitle 1`] = `
  tabIndex={0}
  title="Jump to https://aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.com/assets/src/js/foo-view.js:10"
 >
-  <FrameIndent />
+  <FrameIndent
+    indentLevel={1}
+  />
  <FrameTitle
    frame={
      Object {
@ -301,7 +303,9 @@ exports[`Frame library frame 1`] = `
  role="listitem"
  tabIndex={0}
 >
-  <FrameIndent />
+  <FrameIndent
+    indentLevel={1}
+  />
  <FrameTitle
    frame={
      Object {
@ -593,7 +597,9 @@ exports[`Frame user frame (not selected) 1`] = `
  role="listitem"
  tabIndex={0}
 >
-  <FrameIndent />
+  <FrameIndent
+    indentLevel={1}
+  />
  <FrameTitle
    frame={
      Object {
@ -883,7 +889,9 @@ exports[`Frame user frame 1`] = `
  role="listitem"
  tabIndex={0}
 >
-  <FrameIndent />
+  <FrameIndent
+    indentLevel={1}
+  />
  <FrameTitle
    frame={
      Object {
--- a/devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/snapshots/Group.spec.js.snap
+++ b/devtools/client/debugger/src/components/SecondaryPanes/Frames/tests/snapshots/Group.spec.js.snap
@ -324,9 +324,6 @@ exports[`Group passes the getFrameTitle prop to the Frame components 1`] = `
  <div
    className="frames-list"
  >
-    <FrameIndent
-      key="frame-indent-0"
-    />
    <Frame
      disableContextMenu={false}
      displayFullUrl={false}
@ -462,6 +459,7 @@ exports[`Group passes the getFrameTitle prop to the Frame components 1`] = `
      }
      getFrameTitle={[Function]}
      hideLocation={true}
+      isInGroup={true}
      key="1"
      panel="webconsole"
      selectFrame={[MockFunction]}
@ -598,9 +596,6 @@ exports[`Group passes the getFrameTitle prop to the Frame components 1`] = `
      }
      shouldMapDisplayName={false}
    />
-    <FrameIndent
-      key="frame-indent-1"
-    />
    <Frame
      disableContextMenu={false}
      displayFullUrl={false}
@ -736,6 +731,7 @@ exports[`Group passes the getFrameTitle prop to the Frame components 1`] = `
      }
      getFrameTitle={[Function]}
      hideLocation={true}
+      isInGroup={true}
      key="2"
      panel="webconsole"
      selectFrame={[MockFunction]}
@ -872,9 +868,6 @@ exports[`Group passes the getFrameTitle prop to the Frame components 1`] = `
      }
      shouldMapDisplayName={false}
    />
-    <FrameIndent
-      key="frame-indent-2"
-    />
    <Frame
      disableContextMenu={false}
      displayFullUrl={false}
@ -1010,6 +1003,7 @@ exports[`Group passes the getFrameTitle prop to the Frame components 1`] = `
      }
      getFrameTitle={[Function]}
      hideLocation={true}
+      isInGroup={true}
      key="3"
      panel="webconsole"
      selectFrame={[MockFunction]}
@ -1474,9 +1468,6 @@ exports[`Group renders group with anonymous functions 2`] = `
  <div
    className="frames-list"
  >
-    <FrameIndent
-      key="frame-indent-0"
-    />
    <Frame
      disableContextMenu={false}
      displayFullUrl={false}
@ -1611,6 +1602,7 @@ exports[`Group renders group with anonymous functions 2`] = `
        }
      }
      hideLocation={true}
+      isInGroup={true}
      key="1"
      panel="webconsole"
      selectFrame={[MockFunction]}
@ -1747,9 +1739,6 @@ exports[`Group renders group with anonymous functions 2`] = `
      }
      shouldMapDisplayName={false}
    />
-    <FrameIndent
-      key="frame-indent-1"
-    />
    <Frame
      disableContextMenu={false}
      displayFullUrl={false}
@ -1884,6 +1873,7 @@ exports[`Group renders group with anonymous functions 2`] = `
        }
      }
      hideLocation={true}
+      isInGroup={true}
      key="2"
      panel="webconsole"
      selectFrame={[MockFunction]}
@ -2020,9 +2010,6 @@ exports[`Group renders group with anonymous functions 2`] = `
      }
      shouldMapDisplayName={false}
    />
-    <FrameIndent
-      key="frame-indent-2"
-    />
    <Frame
      disableContextMenu={false}
      displayFullUrl={false}
@ -2157,6 +2144,7 @@ exports[`Group renders group with anonymous functions 2`] = `
        }
      }
      hideLocation={true}
+      isInGroup={true}
      key="3"
      panel="webconsole"
      selectFrame={[MockFunction]}
--- a/devtools/client/shared/components/SmartTrace.css
+++ b/devtools/client/shared/components/SmartTrace.css
@ -25,7 +25,11 @@
 }

 .frames .frame {
-  display: contents;
+  /* Parent is a grid container whose grid template we want to use, so span the whole line */
+  grid-column: 1 / -1;
+  display: grid;
+  /* Grid is defined in `.frames [role="list"]` rule  */
+  grid-template-columns: subgrid;
  cursor: pointer;
  white-space: normal;
 }
@ -76,7 +80,6 @@
 }

 .frames .frames-group .frames-list {
-  grid-column: 1 / -1;
  margin-block-start: 2px;
  /*
   * We want to display each frame name on its own row, without having new lines in the
@ -87,7 +90,7 @@
 }

 .frames .frames-group .frames-list .frame {
-  padding-inline-start: 16px;
+  padding-inline-start: 0;
  text-overflow: ellipsis;
 }

--- a/devtools/client/webconsole/test/browser/browser_webconsole_keyboard_accessibility.js
+++ b/devtools/client/webconsole/test/browser/browser_webconsole_keyboard_accessibility.js
@ -5,36 +5,68 @@

 "use strict";

-const TEST_URI = `data:text/html;charset=utf-8,<!DOCTYPE html><p>Test keyboard accessibility</p>
+const HTML_FILENAME = `test.html`;
+const HTML_CONTENT = `<!DOCTYPE html><p>Test keyboard accessibility</p>
  <script>
    for (let i = 1; i <= 100; i++) {
      console.log("console message " + i);
    }
+    function logTrace() {
+      const sub = () => console.trace("console trace message");
+      sub();
+    }
+    logTrace();
  </script>
  `;

+const TRACE_FRAME_LINE_REGEX = /test\.html:\d+:?\d*/;
+
+const httpServer = createTestHTTPServer();
+httpServer.registerContentType("html", "text/html");
+httpServer.registerPathHandler(
+  "/" + HTML_FILENAME,
+  function (request, response) {
+    response.setStatusLine(request.httpVersion, 200, "OK");
+    response.write(HTML_CONTENT);
+  }
+);
+
+const port = httpServer.identity.primaryPort;
+const TEST_URI = `http://localhost:${port}/${HTML_FILENAME}`;
+
 add_task(async function () {
+  // Force tabfocus for all elements on OSX.
+  SpecialPowers.pushPrefEnv({ set: [["accessibility.tabfocus", 7]] });
+
  const hud = await openNewTabAndConsole(TEST_URI);
+
  info("Web Console opened");
  const outputScroller = hud.ui.outputScroller;
-  await waitFor(
-    () => findConsoleAPIMessage(hud, "console message 100"),
+  const traceMsgNode = await waitFor(
+    () => findConsoleAPIMessage(hud, "console trace message"),
    "waiting for all the messages to be displayed",
    100,
    1000
  );

+  // wait for all the stacktrace frames to be rendered.
+  await waitFor(() =>
+    traceMsgNode.querySelector(".message-body-wrapper > .stacktrace .frames")
+  );
+
  let currentPosition = outputScroller.scrollTop;
  const bottom = currentPosition;
  hud.jsterm.focus();
-  // Page up.
+
+  info("Check Page up keyboard shortcut");
  EventUtils.synthesizeKey("KEY_PageUp");
  isnot(
    outputScroller.scrollTop,
    currentPosition,
    "scroll position changed after page up"
  );
-  // Page down.
+
+  info("Check Page down keyboard shortcut");
  currentPosition = outputScroller.scrollTop;
  EventUtils.synthesizeKey("KEY_PageDown");
  ok(
@ -42,11 +74,11 @@ add_task(async function () {
    "scroll position now at bottom"
  );

-  // Home
+  info("Check Home keyboard shortcut");
  EventUtils.synthesizeKey("KEY_Home");
  is(outputScroller.scrollTop, 0, "scroll position now at top");

-  // End
+  info("Check End keyboard shortcut");
  EventUtils.synthesizeKey("KEY_End");
  const scrollTop = outputScroller.scrollTop;
  ok(
@ -54,7 +86,113 @@ add_task(async function () {
    "scroll position now at bottom"
  );

-  // Clear output
+  info("Hit Shift-Tab to focus switch to editor mode button");
+  EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true });
+  ok(
+    outputScroller.ownerDocument.activeElement.classList.contains(
+      "webconsole-input-openEditorButton"
+    ),
+    "switch to editor mode button is focused"
+  );
+
+  info("Check stacktrace frames can be focused");
+  EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true });
+  ok(
+    outputScroller.ownerDocument.activeElement.closest(".message.trace"),
+    "The active element is in the trace message"
+  );
+  is(
+    TRACE_FRAME_LINE_REGEX.exec(
+      outputScroller.ownerDocument.activeElement.innerText
+    )?.[0],
+    "test.html:10",
+    `last frame of the stacktrace is focused ${outputScroller.ownerDocument.activeElement.innerText}`
+  );
+  is(
+    outputScroller.ownerDocument.activeElement.getAttribute("class"),
+    "frame",
+    "active element has expected class"
+  );
+
+  info("Hit Tab to navigate to second frame of the stacktrace");
+  EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true });
+  ok(
+    outputScroller.ownerDocument.activeElement.closest(".message.trace"),
+    "The active element is in the trace message"
+  );
+  is(
+    TRACE_FRAME_LINE_REGEX.exec(
+      outputScroller.ownerDocument.activeElement.innerText
+    )?.[0],
+    "test.html:8",
+    "second frame of the stacktrace is focused"
+  );
+  is(
+    outputScroller.ownerDocument.activeElement.getAttribute("class"),
+    "frame",
+    "active element has expected class"
+  );
+
+  info("Hit Tab to navigate to first frame of the stacktrace");
+  EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true });
+  ok(
+    outputScroller.ownerDocument.activeElement.closest(".message.trace"),
+    "The active element is in the trace message"
+  );
+  is(
+    TRACE_FRAME_LINE_REGEX.exec(
+      outputScroller.ownerDocument.activeElement.innerText
+    )?.[0],
+    "test.html:7",
+    "third frame of the stacktrace is focused"
+  );
+  is(
+    outputScroller.ownerDocument.activeElement.getAttribute("class"),
+    "frame",
+    "active element has expected class"
+  );
+
+  info("Hit Tab to navigate to the message location");
+  EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true });
+  ok(
+    outputScroller.ownerDocument.activeElement.closest(".message.trace"),
+    "The active element is in the trace message"
+  );
+  is(
+    outputScroller.ownerDocument.activeElement.getAttribute("class"),
+    "frame-link-source",
+    "active element is the console trace message location"
+  );
+  is(
+    TRACE_FRAME_LINE_REGEX.exec(
+      outputScroller.ownerDocument.activeElement.innerText
+    )?.[0],
+    "test.html:7:33",
+    "active element is expected message location"
+  );
+
+  info("Hit Tab to navigate to the previous message location");
+  EventUtils.synthesizeKey("KEY_Tab", { shiftKey: true });
+  ok(
+    !outputScroller.ownerDocument.activeElement.closest(".message.trace"),
+    "The active element is not in the trace message"
+  );
+  is(
+    outputScroller.ownerDocument.activeElement.getAttribute("class"),
+    "frame-link-source",
+    "active element is the console trace message location"
+  );
+  is(
+    TRACE_FRAME_LINE_REGEX.exec(
+      outputScroller.ownerDocument.activeElement.innerText
+    )[0],
+    "test.html:4:15",
+    "active element is expected message location"
+  );
+
+  info("Clear output");
+  hud.jsterm.focus();
+
  info("try ctrl-l to clear output");
  let clearShortcut;
  if (Services.appinfo.OS === "Darwin") {