Fix supertab freezing the app

Removes the infinite, app-freezing while-loop and handles the now-reachable errors.
signalapp · Sep 23, 2024 · abc6e9c · abc6e9c
1 parent 02e7a9e
commit abc6e9c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 27 deletions.
diff --git a/ts/services/addGlobalKeyboardShortcuts.ts b/ts/services/addGlobalKeyboardShortcuts.ts
@@ -42,7 +42,7 @@ export function addGlobalKeyboardShortcuts(): void {
       return;
     }
 
-    // Super tab :)
+    // Super tab :(
     if (
       (commandOrCtrl && key === 'F6') ||
       (commandOrCtrl && !shiftKey && (key === 't' || key === 'T'))
@@ -55,10 +55,7 @@ export function addGlobalKeyboardShortcuts(): void {
       const focusedIndexes: Array<number> = [];
 
       targets.forEach((target, index) => {
-        if (
-          (focusedElement != null && target === focusedElement) ||
-          target.contains(focusedElement)
-        ) {
+        if (target.contains(focusedElement)) {
           focusedIndexes.push(index);
         }
       });
@@ -76,20 +73,13 @@ export function addGlobalKeyboardShortcuts(): void {
       const focusedIndex = focusedIndexes.at(-1) ?? -1;
 
       const lastIndex = targets.length - 1;
-      const increment = shiftKey ? -1 : 1;
 
-      let index;
-      if (focusedIndex < 0 || focusedIndex >= lastIndex) {
-        index = 0;
+      let index: number | undefined;
+      if (focusedIndex < 0) {
+        index = shiftKey ? lastIndex : 0;
       } else {
-        index = focusedIndex + increment;
-      }
-
-      while (!targets[index]) {
-        index += increment;
-        if (index > lastIndex || index < 0) {
-          index = 0;
-        }
+        const increment = shiftKey ? lastIndex : 1;
+        index = (focusedIndex + increment) % targets.length;
       }
 
       const node = targets[index];
@@ -98,12 +88,15 @@ export function addGlobalKeyboardShortcuts(): void {
       if (firstFocusableElement) {
         firstFocusableElement.focus();
       } else {
-        const nodeInfo = Array.from(node.attributes)
-          .map(attr => `${attr.name}=${attr.value}`)
-          .join(',');
-        log.warn(
-          `supertab: could not find focus for DOM node ${node.nodeName}<${nodeInfo}>`
-        );
+        if (node) {
+          const nodeInfo = Array.from(node.attributes)
+            .map(attr => `${attr.name}=${attr.value}`)
+            .join(',');
+          log.warn(
+            `supertab: could not find focus for DOM node ${node.nodeName}<${nodeInfo}>`
+          );
+        }
+
         window.enterMouseMode();
         const { activeElement } = document;
         if (

diff --git a/ts/util/focusableSelectors.ts b/ts/util/focusableSelectors.ts
@@ -36,9 +36,9 @@ export const focusableSelector = focusableSelectors.join(', ');
  * is focusable.
  */
 export function matchOrQueryFocusable(
-  element: HTMLElement
-): HTMLElement | null {
-  return element.matches(focusableSelector)
+  element: HTMLElement | null | undefined
+): HTMLElement | null | undefined {
+  return element?.matches(focusableSelector)
     ? element
-    : element.querySelector(focusableSelector);
+    : element?.querySelector(focusableSelector);
 }