fix: Update focusNode to self correct focus (#9082)

## The basics

- [x] I [validated my changes](https://developers.google.com/blockly/guides/contribute/core#making_and_verifying_a_change)

## The details
### Resolves

Fixes RaspberryPiFoundation/blockly-keyboard-experimentation#87

### Proposed Changes

This updates `FocusManager.focusNode()` to automatically defocus its internal state if it detects that DOM focus (per `document.activeElement`) doesn't match its own internal focus.

It also updates `FocusManager` to avoid duplicate self calls to `focusNode()`.

### Reason for Changes

This is a robustness improvement for `focusNode` that is nice to keep as a "if all else fails" mechanism, but it's currently a hacky workaround to RaspberryPiFoundation/blockly-keyboard-experimentation#87. #9081 is tracking introducing a long-term fix for the desynchronizing problem, but that's likely to be potentially much harder to solve and this at least introduces a reasonable correction.

From a stability perspective, it seems likely that there are multiple classes of failures covered by this fix. Essentially the browser behavior difference in Firefox and Safari over Chrome is that the former do not fire a focus change event when a focused element is removed from the DOM (leading to `FocusManager` getting out of sync). There may be other such cases when a focus event isn't fired, so this robustness improvement at least ensures eventual consistency so long as `focusNode()` is called (and, fortunately, that's done a lot now).

While this is a nice robustness improvement, it's not a perfect replacement for a real fix. For the time between `FocusManager` getting out of sync and `focusNode` getting called, `getFocusedNode` will _not_ match the actual element holding focus. The primary class of issues known is when a DOM element is being moved, and in these cases `focusNode` _is_ called. If there are other such unknown cases where a desync can happen, **`getFocusedNode()` will remain wrong until a later `focusNode()` call**.

Note one other change: originally implemented but removed in earlier PRs for `FocusManager`, this change also includes ensuring `focusNode()` isn't called multiple times for a single request to focus a node. Current logic results in a call to `focusNode()` calling a node's `focus()` which then processes a second call to `focusNode()` (which is fully executed because `FocusManager.focusedNode` isn't updated until after the call to `focus()`). This doesn't actually correct any state, but it's more efficient and provides some resilience against potential logic issues from calling node/tree callbacks multiple times (which was observed up to 3 times in some cases).

### Test Coverage

This has been tested via the keyboard navigation experimental plugin's test environment (with Firefox), plus new unit tests. Note the new test for directly verifying desyncing logic is contrived, but it should be perfectly testing the exact scenario that's being observed on Firefox/Safari. A separate test was added for the existing behavior of focusing a different node still correcting `FocusManager` state even if it was desynced (the bug only happens for the same node being refocused).

New tests were also added for the various lifecycle callbacks (to ensure they aren't called multiple times).

All of the new tests were verified to fail without the two fixes in place (they were verified in isolation), minus the test for focusing a second node when desynced (since that should pass regardless of the new fixes).

Some basic simple playground testing was done, as well, just to verify nothing obvious was broken around selection, gestures, and copy/paste.

### Documentation

No new documentation should be needed here.

### Additional Information

This wasn't explicitly tested in Safari since I only have access to Chrome and Firefox, but I will ask someone else on the team to verify this for me after merging if it isn't checked sooner.

Author: BenHenning

core/focus_manager.ts

@@ -63,12 +63,16 @@ export class FocusManager {
   private currentlyHoldsEphemeralFocus: boolean = false;
   private lockFocusStateChanges: boolean = false;
   private recentlyLostAllFocus: boolean = false;
+  private isUpdatingFocusedNode: boolean = false;
 
   constructor(
     addGlobalEventListener: (type: string, listener: EventListener) => void,
   ) {
     // Note that 'element' here is the element *gaining* focus.
     const maybeFocus = (element: Element | EventTarget | null) => {
+      // Skip processing the event if the focused node is currently updating.
+      if (this.isUpdatingFocusedNode) return;
+
       this.recentlyLostAllFocus = !element;
       let newNode: IFocusableNode | null | undefined = null;
       if (element instanceof HTMLElement || element instanceof SVGElement) {
@@ -240,7 +244,23 @@ export class FocusManager {
    */
   focusNode(focusableNode: IFocusableNode): void {
     this.ensureManagerIsUnlocked();
-    if (this.focusedNode === focusableNode) return; // State is unchanged.
+    if (!this.currentlyHoldsEphemeralFocus) {
+      // Disable state syncing from DOM events since possible calls to focus()
+      // below will loop a call back to focusNode().
+      this.isUpdatingFocusedNode = true;
+    }
+
+    // Double check that state wasn't desynchronized in the background. See:
+    // https://github.com/google/blockly-keyboard-experimentation/issues/87.
+    // This is only done for the case where the same node is being focused twice
+    // since other cases should automatically correct (due to the rest of the
+    // routine running as normal).
+    const prevFocusedElement = this.focusedNode?.getFocusableElement();
+    const hasDesyncedState = prevFocusedElement !== document.activeElement;
+    if (this.focusedNode === focusableNode && !hasDesyncedState) {
+      return; // State is unchanged.
+    }
+
     if (!focusableNode.canBeFocused()) {
       // This node can't be focused.
       console.warn("Trying to focus a node that can't be focused.");
@@ -292,6 +312,10 @@ export class FocusManager {
       this.activelyFocusNode(nodeToFocus, prevTree ?? null);
     }
     this.updateFocusedNode(nodeToFocus);
+    if (!this.currentlyHoldsEphemeralFocus) {
+      // Reenable state syncing from DOM events.
+      this.isUpdatingFocusedNode = false;
+    }
   }
 
   /**

tests/mocha/focus_manager_test.js

@@ -419,6 +419,91 @@ suite('FocusManager', function () {
       const currentNode = this.focusManager.getFocusedNode();
       assert.strictEqual(currentNode, this.testFocusableTree1Node2);
     });
+
+    test('restores focus when element quietly loses focus', function () {
+      this.focusManager.registerTree(this.testFocusableTree1);
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+      // Remove the FocusManager's listeners to simulate not receiving a focus
+      // event when focus is lost. This can happen in Firefox and Safari when an
+      // element is removed and then re-added to the DOM. This is a contrived
+      // setup to achieve the same outcome on all browsers. For context, see:
+      // https://github.com/google/blockly-keyboard-experimentation/issues/87.
+      for (const registeredListener of this.globalDocumentEventListeners) {
+        const eventType = registeredListener.type;
+        const eventListener = registeredListener.listener;
+        document.removeEventListener(eventType, eventListener);
+      }
+      document.body.focus();
+
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+
+      const currentNode = this.focusManager.getFocusedNode();
+      const currentElem = currentNode?.getFocusableElement();
+      assert.strictEqual(currentNode, this.testFocusableTree1Node1);
+      assert.strictEqual(document.activeElement, currentElem);
+    });
+
+    test('restores focus when element and new node focused', function () {
+      this.focusManager.registerTree(this.testFocusableTree1);
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+      // Remove the FocusManager's listeners to simulate not receiving a focus
+      // event when focus is lost. This can happen in Firefox and Safari when an
+      // element is removed and then re-added to the DOM. This is a contrived
+      // setup to achieve the same outcome on all browsers. For context, see:
+      // https://github.com/google/blockly-keyboard-experimentation/issues/87.
+      for (const registeredListener of this.globalDocumentEventListeners) {
+        const eventType = registeredListener.type;
+        const eventListener = registeredListener.listener;
+        document.removeEventListener(eventType, eventListener);
+      }
+      document.body.focus();
+
+      this.focusManager.focusNode(this.testFocusableTree1Node2);
+
+      const currentNode = this.focusManager.getFocusedNode();
+      const currentElem = currentNode?.getFocusableElement();
+      assert.strictEqual(currentNode, this.testFocusableTree1Node2);
+      assert.strictEqual(document.activeElement, currentElem);
+    });
+
+    test('for unfocused node calls onNodeFocus once', function () {
+      sinon.spy(this.testFocusableTree1Node1, 'onNodeFocus');
+      this.focusManager.registerTree(this.testFocusableTree1);
+
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+
+      assert.strictEqual(this.testFocusableTree1Node1.onNodeFocus.callCount, 1);
+    });
+
+    test('for previously focused node calls onNodeBlur once', function () {
+      sinon.spy(this.testFocusableTree1Node1, 'onNodeBlur');
+      this.focusManager.registerTree(this.testFocusableTree1);
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+
+      this.focusManager.focusNode(this.testFocusableTree1Node2);
+
+      assert.strictEqual(this.testFocusableTree1Node1.onNodeBlur.callCount, 1);
+    });
+
+    test('for unfocused tree calls onTreeFocus once', function () {
+      sinon.spy(this.testFocusableTree1, 'onTreeFocus');
+      this.focusManager.registerTree(this.testFocusableTree1);
+
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+
+      assert.strictEqual(this.testFocusableTree1.onTreeFocus.callCount, 1);
+    });
+
+    test('for previously focused tree calls onTreeBlur once', function () {
+      sinon.spy(this.testFocusableTree1, 'onTreeBlur');
+      this.focusManager.registerTree(this.testFocusableTree1);
+      this.focusManager.registerTree(this.testFocusableTree2);
+      this.focusManager.focusNode(this.testFocusableTree1Node1);
+
+      this.focusManager.focusNode(this.testFocusableTree2Node1);
+
+      assert.strictEqual(this.testFocusableTree1.onTreeBlur.callCount, 1);
+    });
   });
 
   suite('getFocusManager()', function () {