Improve keyboard/screenreader experience in DOM breakpoints

Currently, there is no intuitive way to navigate the DOM breakpoints pane by keyboard. This change refactors the DOM breakpoints pane to use UI.ListControl to manage keyboard navigation, in response to feedback here [3] about code duplication. Using ListControl also makes it easier to manage an accessible description on breakpoint elements so that screen reader users are informed about the checked state of breakpoints and whether the page is currently paused on them. Focusing the list items, before/after: https://gyazo.com/5edc75de0c0e9d7f968f49e114cec324 https://gyazo.com/c3efc3322783f3505bc4dde3edabbf13 This CL breaks a web test, so [1] must be merged first to disable it. [2] Fixes and reenables it. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1893960 [2] https://chromium-review.googlesource.com/c/chromium/src/+/1644461 [3] https://chromium-review.googlesource.com/c/chromium/src/+/1644461/14/third_party/blink/renderer/devtools/front_end/browser_debugger/DOMBreakpointsSidebarPane.js#141 Bug: 963183 Change-Id: I41e2e8b73baa7ae3e6169163785e245493ab4ba7 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1889352 Commit-Queue: Jack Lynch <jalyn@microsoft.com> Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
2026-10-02 18:11:04 +08:00 · 2019-12-12 03:34:36 +00:00
parent 94b70d6b3d
commit 8a344769f6
8 changed files with 184 additions and 108 deletions
@@ -30,18 +30,25 @@

 /**
 * @implements {UI.ContextFlavorListener}
+ * @implements {UI.ListDelegate<!SDK.DOMDebuggerModel.DOMBreakpoint>}
 */
 export class DOMBreakpointsSidebarPane extends UI.VBox {
  constructor() {
    super(true);
    this.registerRequiredCSS('browser_debugger/domBreakpointsSidebarPane.css');

-    this._listElement = this.contentElement.createChild('div', 'breakpoint-list hidden');
    this._emptyElement = this.contentElement.createChild('div', 'gray-info-message');
    this._emptyElement.textContent = Common.UIString('No breakpoints');
+    /** @type {!UI.ListModel.<!SDK.DOMDebuggerModel.DOMBreakpoint>} */
+    this._breakpoints = new UI.ListModel();
+    /** @type {!UI.ListControl.<!SDK.DOMDebuggerModel.DOMBreakpoint>} */
+    this._list = new UI.ListControl(this._breakpoints, this, UI.ListMode.NonViewport);
+    this.contentElement.appendChild(this._list.element);
+    this._list.element.classList.add('breakpoint-list', 'hidden');
+    UI.ARIAUtils.markAsList(this._list.element);
+    UI.ARIAUtils.setAccessibleName(this._list.element, ls`DOM Breakpoints list`);
+    this._emptyElement.tabIndex = -1;

-    /** @type {!Map<!SDK.DOMDebuggerModel.DOMBreakpoint, !BrowserDebugger.DOMBreakpointsSidebarPane.Item>} */
-    this._items = new Map();
    SDK.targetManager.addModelListener(
        SDK.DOMDebuggerModel, SDK.DOMDebuggerModel.Events.DOMBreakpointAdded, this._breakpointAdded, this);
    SDK.targetManager.addModelListener(
@@ -56,10 +63,114 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {
      }
    }

-    this._highlightedElement = null;
+    this._highlightedBreakpoint = null;
    this._update();
  }

+  /**
+   * @override
+   * @param {!SDK.DOMDebuggerModel.DOMBreakpoint} item
+   * @return {!Element}
+   */
+  createElementForItem(item) {
+    const element = createElementWithClass('div', 'breakpoint-entry');
+    element.addEventListener('contextmenu', this._contextMenu.bind(this, item), true);
+    UI.ARIAUtils.markAsListitem(element);
+    element.tabIndex = this._list.selectedItem() === item ? 0 : -1;
+
+    const checkboxLabel = UI.CheckboxLabel.create(/* title */ '', item.enabled);
+    const checkboxElement = checkboxLabel.checkboxElement;
+    checkboxElement.addEventListener('click', this._checkboxClicked.bind(this, item), false);
+    checkboxElement.tabIndex = -1;
+    UI.ARIAUtils.markAsHidden(checkboxLabel);
+    element.appendChild(checkboxLabel);
+
+    const labelElement = createElementWithClass('div', 'dom-breakpoint');
+    element.appendChild(labelElement);
+    element.addEventListener('keydown', event => {
+      if (event.key === ' ') {
+        checkboxElement.click();
+        event.consume(true);
+      }
+    });
+
+    const description = createElement('div');
+    const breakpointTypeLabel = BrowserDebugger.DOMBreakpointsSidebarPane.BreakpointTypeLabels.get(item.type);
+    description.textContent = breakpointTypeLabel;
+    const linkifiedNode = createElementWithClass('monospace');
+    linkifiedNode.style.display = 'block';
+    labelElement.appendChild(linkifiedNode);
+    Common.Linkifier.linkify(item.node, {preventKeyboardFocus: true}).then(linkified => {
+      linkifiedNode.appendChild(linkified);
+      UI.ARIAUtils.setAccessibleName(checkboxElement, ls`${breakpointTypeLabel}: ${linkified.deepTextContent()}`);
+    });
+
+    labelElement.appendChild(description);
+
+    const checkedStateText = item.enabled ? ls`checked` : ls`unchecked`;
+    if (item === this._highlightedBreakpoint) {
+      element.classList.add('breakpoint-hit');
+      UI.ARIAUtils.setDescription(element, ls`${checkedStateText} breakpoint hit`);
+    } else {
+      UI.ARIAUtils.setDescription(element, checkedStateText);
+    }
+
+
+    this._emptyElement.classList.add('hidden');
+    this._list.element.classList.remove('hidden');
+
+    return element;
+  }
+
+  /**
+   * @override
+   * @param {!SDK.DOMDebuggerModel.DOMBreakpoint} item
+   * @return {number}
+   */
+  heightForItem(item) {
+    return 0;
+  }
+
+  /**
+   * @override
+   * @param {!SDK.DOMDebuggerModel.DOMBreakpoint} item
+   * @return {boolean}
+   */
+  isItemSelectable(item) {
+    return true;
+  }
+
+  /**
+   * @override
+   * @param {?Element} fromElement
+   * @param {?Element} toElement
+   * @return {boolean}
+   */
+  updateSelectedItemARIA(fromElement, toElement) {
+    return true;
+  }
+
+  /**
+   * @override
+   * @param {?SDK.DOMDebuggerModel.DOMBreakpoint} from
+   * @param {?SDK.DOMDebuggerModel.DOMBreakpoint} to
+   * @param {?Element} fromElement
+   * @param {?Element} toElement
+   */
+  selectedItemChanged(from, to, fromElement, toElement) {
+    if (fromElement) {
+      fromElement.tabIndex = -1;
+    }
+
+    if (toElement) {
+      this.setDefaultFocusedElement(toElement);
+      toElement.tabIndex = 0;
+      if (this.hasFocus()) {
+        toElement.focus();
+      }
+    }
+  }
+
  /**
   * @param {!Common.Event} event
   */
@@ -71,10 +182,11 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {
   * @param {!Common.Event} event
   */
  _breakpointToggled(event) {
+    const hadFocus = this.hasFocus();
    const breakpoint = /** @type {!SDK.DOMDebuggerModel.DOMBreakpoint} */ (event.data);
-    const item = this._items.get(breakpoint);
-    if (item) {
-      item.checkbox.checked = breakpoint.enabled;
+    this._list.refreshItem(breakpoint);
+    if (hadFocus) {
+      this.focus();
    }
  }

@@ -82,17 +194,28 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {
   * @param {!Common.Event} event
   */
  _breakpointsRemoved(event) {
+    const hadFocus = this.hasFocus();
    const breakpoints = /** @type {!Array<!SDK.DOMDebuggerModel.DOMBreakpoint>} */ (event.data);
+    let lastIndex = -1;
    for (const breakpoint of breakpoints) {
-      const item = this._items.get(breakpoint);
-      if (item) {
-        this._items.delete(breakpoint);
-        this._listElement.removeChild(item.element);
+      const index = this._breakpoints.indexOf(breakpoint);
+      if (index >= 0) {
+        this._breakpoints.remove(index);
+        lastIndex = index;
      }
    }
-    if (!this._listElement.firstChild) {
+    if (this._breakpoints.length === 0) {
      this._emptyElement.classList.remove('hidden');
-      this._listElement.classList.add('hidden');
+      this.setDefaultFocusedElement(this._emptyElement);
+      this._list.element.classList.add('hidden');
+    } else if (lastIndex >= 0) {
+      const breakpointToSelect = this._breakpoints.at(lastIndex);
+      if (breakpointToSelect) {
+        this._list.selectItem(breakpointToSelect);
+      }
+    }
+    if (hadFocus) {
+      this.focus();
    }
  }

@@ -100,43 +223,18 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {
   * @param {!SDK.DOMDebuggerModel.DOMBreakpoint} breakpoint
   */
  _addBreakpoint(breakpoint) {
-    const element = createElementWithClass('div', 'breakpoint-entry');
-    element.addEventListener('contextmenu', this._contextMenu.bind(this, breakpoint), true);
-
-    const checkboxLabel = UI.CheckboxLabel.create('', breakpoint.enabled);
-    const checkboxElement = checkboxLabel.checkboxElement;
-    checkboxElement.addEventListener('click', this._checkboxClicked.bind(this, breakpoint), false);
-    element.appendChild(checkboxLabel);
-
-    const labelElement = createElementWithClass('div', 'dom-breakpoint');
-    element.appendChild(labelElement);
-
-    const description = createElement('div');
-    const breakpointTypeLabel = BreakpointTypeLabels.get(breakpoint.type);
-    description.textContent = breakpointTypeLabel;
-    const linkifiedNode = createElementWithClass('monospace');
-    linkifiedNode.style.display = 'block';
-    labelElement.appendChild(linkifiedNode);
-    Common.Linkifier.linkify(breakpoint.node).then(linkified => {
-      linkifiedNode.appendChild(linkified);
-      UI.ARIAUtils.setAccessibleName(checkboxElement, ls`${breakpointTypeLabel}: ${linkified.deepTextContent()}`);
-    });
-    labelElement.appendChild(description);
-
-    const item = {breakpoint: breakpoint, element: element, checkbox: checkboxElement};
-    element._item = item;
-    this._items.set(breakpoint, item);
-
-    let currentElement = this._listElement.firstChild;
-    while (currentElement) {
-      if (currentElement._item && currentElement._item.breakpoint.type < breakpoint.type) {
-        break;
+    this._breakpoints.insertWithComparator(breakpoint, (breakpointA, breakpointB) => {
+      if (breakpointA.type > breakpointB.type) {
+        return -1;
      }
-      currentElement = currentElement.nextSibling;
+      if (breakpointA.type < breakpointB.type) {
+        return 1;
+      }
+      return 0;
+    });
+    if (!this.hasFocus()) {
+      this._list.selectItem(this._breakpoints.at(0));
    }
-    this._listElement.insertBefore(element, currentElement);
-    this._emptyElement.classList.add('hidden');
-    this._listElement.classList.remove('hidden');
  }

  /**
@@ -145,6 +243,8 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {
   */
  _contextMenu(breakpoint, event) {
    const contextMenu = new UI.ContextMenu(event);
+    contextMenu.defaultSection().appendItem(
+        ls`Reveal DOM node in Elements panel`, Common.Revealer.reveal.bind(null, breakpoint.node));
    contextMenu.defaultSection().appendItem(Common.UIString('Remove breakpoint'), () => {
      breakpoint.domDebuggerModel.removeDOMBreakpoint(breakpoint.node, breakpoint.type);
    });
@@ -156,13 +256,10 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {

  /**
   * @param {!SDK.DOMDebuggerModel.DOMBreakpoint} breakpoint
+   * @param {!Event} event
   */
-  _checkboxClicked(breakpoint) {
-    const item = this._items.get(breakpoint);
-    if (!item) {
-      return;
-    }
-    breakpoint.domDebuggerModel.toggleDOMBreakpoint(breakpoint, item.checkbox.checked);
+  _checkboxClicked(breakpoint, event) {
+    breakpoint.domDebuggerModel.toggleDOMBreakpoint(breakpoint, event.target.checked);
  }

  /**
@@ -175,13 +272,15 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {

  _update() {
    const details = UI.context.flavor(SDK.DebuggerPausedDetails);
+    if (this._highlightedBreakpoint) {
+      const oldHighlightedBreakpoint = this._highlightedBreakpoint;
+      delete this._highlightedBreakpoint;
+      this._list.refreshItem(oldHighlightedBreakpoint);
+    }
    if (!details || !details.auxData || details.reason !== SDK.DebuggerModel.BreakReason.DOM) {
-      if (this._highlightedElement) {
-        this._highlightedElement.classList.remove('breakpoint-hit');
-        delete this._highlightedElement;
-      }
      return;
    }
+
    const domDebuggerModel = details.debuggerModel.target().model(SDK.DOMDebuggerModel);
    if (!domDebuggerModel) {
      return;
@@ -191,18 +290,15 @@ export class DOMBreakpointsSidebarPane extends UI.VBox {
      return;
    }

-    let element = null;
-    for (const item of this._items.values()) {
-      if (item.breakpoint.node === data.node && item.breakpoint.type === data.type) {
-        element = item.element;
+    for (const breakpoint of this._breakpoints) {
+      if (breakpoint.node === data.node && breakpoint.type === data.type) {
+        this._highlightedBreakpoint = breakpoint;
      }
    }
-    if (!element) {
-      return;
+    if (this._highlightedBreakpoint) {
+      this._list.refreshItem(this._highlightedBreakpoint);
    }
    UI.viewManager.showView('sources.domBreakpoints');
-    element.classList.add('breakpoint-hit');
-    this._highlightedElement = element;
  }
 }

@@ -15,6 +15,9 @@
  <message name="IDS_DEVTOOLS_3ea566249a507705d9a7ff4d3bd31440" desc="Screen reader description of a hit breakpoint in the Sources panel">
    breakpoint hit
  </message>
+  <message name="IDS_DEVTOOLS_57aab6f08efb5f158365bef707ea951d" desc="A context menu item in the DOM Breakpoints sidebar that reveals the node on which the current breakpoint is set.">
+    Reveal DOM node in Elements panel
+  </message>
  <message name="IDS_DEVTOOLS_59eaf6955f44a94237b6d26911c1d983" desc="A context menu item in the DOMBreakpoints Sidebar Pane of the JavaScript Debugging pane in the Sources panel or the DOM Breakpoints pane in the Elements panel">
    Break on
  </message>
@@ -27,6 +30,9 @@
  <message name="IDS_DEVTOOLS_9f76c421048cb58ab03988d2ce1c813e" desc="Label for a button in the sources panel that refreshes the list of global event listeners.">
    Refresh global listeners
  </message>
+  <message name="IDS_DEVTOOLS_8fc1dde0c79d8adcaf674b3d679d3596" desc="Accessibility label for hit breakpoints in the Sources panel.">
+    <ph name="CHECKEDSTATETEXT">$1s<ex>checked</ex></ph> breakpoint hit
+  </message>
  <message name="IDS_DEVTOOLS_b839f802a330e4d4145cb182e6767f45" desc="Text in XHRBreakpoints Sidebar Pane of the JavaScript Debugging pane in the Sources panel or the DOM Breakpoints pane in the Elements panel">
    Any XHR or fetch
  </message>
@@ -39,6 +45,9 @@
  <message name="IDS_DEVTOOLS_e30c4292775f68b2bc9eb3957a69f899" desc="Title of the 'Global Listeners' tool in the bottom sidebar of the Sources tool">
    Global Listeners
  </message>
+  <message name="IDS_DEVTOOLS_e4fc5497303105b60ef0b8c90be3fbd2" desc="Accessibility label for the DOM breakpoints list in the Sources panel">
+    DOM Breakpoints list
+  </message>
  <message name="IDS_DEVTOOLS_ebdb33cde9015fa4b294220ee89cff00" desc="Title of the 'Event Listener Breakpoints' tool in the bottom sidebar of the Sources tool">
    Event Listener Breakpoints
  </message>
@@ -21,6 +21,10 @@
    padding: 2px 0;
 }

+.breakpoint-entry[data-keyboard-focus="true"] {
+    background-color: var(--focus-bg-color);
+}
+
 .breakpoint-list .breakpoint-entry:hover {
    background-color: #eee;
 }
@@ -124,6 +124,9 @@
  <message name="IDS_DEVTOOLS_36917e785bd31d786ab9dd7790a9a4c2" desc="Text for a heap profile type">
    JS Heap
  </message>
+  <message name="IDS_DEVTOOLS_3793ea52a7be2d7deafd858fda50775c" desc="Text exposed to screen readers on checked items.">
+    checked
+  </message>
  <message name="IDS_DEVTOOLS_382b0f5185773fa0f67a8ed8056c7759" desc="Text for something not available">
    N/A
  </message>
@@ -427,6 +430,9 @@
  <message name="IDS_DEVTOOLS_acc24772ac31677d076f17d9002b57cd" desc="Text to collapse children of a parent group">
    Collapse children
  </message>
+  <message name="IDS_DEVTOOLS_ad91a1da588ce256d30b1af38f78f84e" desc="Text exposed to screen readers on unchecked items.">
+    unchecked
+  </message>
  <message name="IDS_DEVTOOLS_af4bb376939e77df0e7c2332b837a866" desc="Text that refers to closure as a programming term">
    Closure
  </message>
@@ -489,7 +489,6 @@ export default class ListControl {
    const newItem = this._selectedItem;
    const newElement = this._selectedIndex !== -1 ? this._elementAtIndex(index) : null;

-    UI.ARIAUtils.setActiveDescendant(this.element, newElement);
    this._delegate.selectedItemChanged(oldItem, newItem, /** @type {?Element} */ (oldElement), newElement);
    if (!this._delegate.updateSelectedItemARIA(/** @type {?Element} */ (oldElement), newElement)) {
      if (oldElement) {
@@ -63,9 +63,6 @@
  <message name="IDS_DEVTOOLS_2ea54663ae9e3a586132a34a98d85ccf" desc="Text for the title of asynchronous function calls group in Call Stack">
    Async Call
  </message>
-  <message name="IDS_DEVTOOLS_3793ea52a7be2d7deafd858fda50775c" desc="Accessibility label for checked items in the SoftContextMenu">
-    checked
-  </message>
  <message name="IDS_DEVTOOLS_3a39eb38e879f66d1c57dedd478272da" desc="label to open link externally">
    Open in new tab
  </message>
@@ -240,9 +237,6 @@
  <message name="IDS_DEVTOOLS_a950da8e902568f9a6879abe4313efe7" desc="Text content of content element">
    Once page is reloaded, <ph name="LOCKED_1">DevTools</ph> will automatically reconnect.
  </message>
-  <message name="IDS_DEVTOOLS_ad91a1da588ce256d30b1af38f78f84e" desc="Accessibility label for unchecked items in the SoftContextMenu">
-    unchecked
-  </message>
  <message name="IDS_DEVTOOLS_addec426932e71323700afa1911f8f1c" desc="Text on a button to show more info in the infobar">
    more
  </message>
@@ -1,39 +1,8 @@
 Testing accessibility in the DOM breakpoints pane.
 Setting DOM breakpoints.
-DOM breakpoints container text content: DOM BreakpointsSubtree modifiedNode removedNo breakpoints
-DOM breakpoints pane text content: Subtree modifiedNode removedNo breakpoints
+DOM breakpoints container text content: DOM BreakpointsNo breakpointsSubtree modifiedcheckedNode removedunchecked
+DOM breakpoints pane text content: No breakpointsSubtree modifiedcheckedNode removedunchecked
 Running the axe-core linter on the DOM breakpoints pane.
-aXe violations: [
-  {
-    "ruleDescription": "Ensures every form element has a label",
-    "helpUrl": "https://dequeuniversity.com/rules/axe/3.3/label?application=axeAPI",
-    "ruleId": "label",
-    "impact": "critical",
-    "failedNodes": [
-      {
-        "target": [
-          [
-            ".flex-none.flex-auto.vbox:nth-child(7) > .flex-auto.vbox",
-            ".breakpoint-entry:nth-child(1) > span[is=\"dt-checkbox\"]",
-            "#ui-checkbox-label9"
-          ]
-        ],
-        "html": "<input type=\"checkbox\" id=\"ui-checkbox-label9\">",
-        "failureSummary": "Fix any of the following:\n  aria-label attribute does not exist or is empty\n  aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty\n  Form element does not have an implicit (wrapped) <label>\n  Form element does not have an explicit <label>\n  Element has no title attribute or the title attribute is empty"
-      },
-      {
-        "target": [
-          [
-            ".flex-none.flex-auto.vbox:nth-child(7) > .flex-auto.vbox",
-            ".breakpoint-entry:nth-child(2) > span[is=\"dt-checkbox\"]",
-            "#ui-checkbox-label10"
-          ]
-        ],
-        "html": "<input type=\"checkbox\" id=\"ui-checkbox-label10\">",
-        "failureSummary": "Fix any of the following:\n  aria-label attribute does not exist or is empty\n  aria-labelledby attribute does not exist, references elements that do not exist or references elements that are empty\n  Form element does not have an implicit (wrapped) <label>\n  Form element does not have an explicit <label>\n  Element has no title attribute or the title attribute is empty"
-      }
-    ]
-  }
-]
+aXe violations: []


@@ -36,7 +36,6 @@
  TestRunner.addResult(
      'Running the axe-core linter on the DOM breakpoints pane.');

-  //TODO(crbug.com/1004940): expected.txt file has 'label' exceptions
  await AxeCoreTestRunner.runValidation(domBreakpointContainer.element);
  TestRunner.completeTest();
 })();