fix: Synchronize gestures and focus (#8981)

## The basics

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

## The details
### Resolves

Fixes #8952
Fixes #8950
Fixes #8971

Fixes a bunch of other keyboard + mouse synchronization issues found during the testing discussed in RaspberryPiFoundation/blockly-keyboard-experimentation#482 (comment).

### Proposed Changes

This introduces a number of changes to:
- Ensure that gestures which change selection state also change focus.
- Ensure that ephemeral focus is more robust against certain classes of failures.

### Reason for Changes

There are some ephemeral focus issues that can come up with certain actions (like clicking or dragging) don't properly change focus. Beyond that, some users will likely mix clicking and keyboard navigation, so it's essential that focus and selection state stay in sync when switching between these two types of navigation modalities.

Other changes:
- Drop-down div was actually incorrectly releasing ephemeral focus before animated closes finish which could reset focus afterwards, breaking focus state.
- Both drop-down and widget divs have been updated to only return focus _after_ marking the workspace as focused since the last focused node should always be the thing returned.
- In a number of gesture cases selection has been removed. This is due to `BlockSvg` self-managing its selection state based on focus, so focusing is sufficient. I've also centralized some of the focusing calls (such as putting one in `bringBlockToFront` to ensure focusing happens after potential DOM changes).
- Similarly, `BlockSvg`'s own `bringToFront` has been updated to automatically restore focus after the operation completes. Since `bringToFront` can always result in focus loss, this provides robustness to ensure focus is restored.
- Block pasting now ensures that focus is properly set, however a delay is needed due to additional rendering changes that need to complete (I didn't dig deeply into the _why_ of this).
- Dragging has been updated to always focus the moved block if it's not in the process of being deleted.
- There was some selection resetting logic removed from gesture's `doWorkspaceClick` function. As far as I can tell, this won't be needed anymore since blocks self-regulate their selection state now.
- `FocusManager` has a new extra check for ephemeral focus to catch a specific class of failure where the browser takes away focus immediately after it's returned. This can happen if there are delay timing situations (like animations) which result in a focused node being deleted (which then results in the document body receiving focus). The robustness check is possibly not needed, but it help discover the animation issue with drop-down div and shows some promise for helping to fix the variables-closing-flyout problem.

Some caveats:
- Some undo/redo steps can still result in focus being lost. This may introduce some regressions for selection state, and definitely introduces some annoyances with keyboard navigation. More work will be needed to understand how to better redirect focus (and to what) in cases when blocks disappear.
- There are many other places where focus is being forced or selection state being overwritten that could, in theory, cause issues with focus state. These may need to be fixed in the future.
- There's a lot of redundancy with `focusNode` being called more than it needs to be. `FocusManager` does avoid calling `focus()` more than once for the same node, but it's possible for focus state to switch between multiple nodes or elements even for a single gesture (for example, due to bringing the block to the front causing a DOM refresh). While the eventual consistency nature of the manager means this isn't a real problem, it may cause problems with screen reader output in the future and warrant another pass at reducing `focusNode` calls (particularly for gestures and the click event pipeline).

### Test Coverage

This PR is largely relying on existing tests for regression verification, though no new tests have been added for the specific regression cases.

#8910 is tracking improving `FocusManager` tests which could include some cases for the new ephemeral focus improvements.

#8915 is tracking general accessibility testing which could include adding tests for these specific regression cases.

### Documentation

No new documentation is expected to be needed here.

### Additional Information

These changes originate from both #8875 and from a branch @rachel-fenichel created to investigate some of the failures this PR addresses. These changes have also been verified against both Core's playground and the keyboard navigation plugin's test environment.

Author: BenHenning

core/block_svg.ts

@@ -34,6 +34,7 @@ import type {BlockMove} from './events/events_block_move.js';
 import {EventType} from './events/type.js';
 import * as eventUtils from './events/utils.js';
 import {FieldLabel} from './field_label.js';
+import {getFocusManager} from './focus_manager.js';
 import {IconType} from './icons/icon_types.js';
 import {MutatorIcon} from './icons/mutator_icon.js';
 import {WarningIcon} from './icons/warning_icon.js';
@@ -1290,6 +1291,7 @@ export class BlockSvg
    *     adjusting its parents.
    */
   bringToFront(blockOnly = false) {
+    const previouslyFocused = getFocusManager().getFocusedNode();
     /* eslint-disable-next-line @typescript-eslint/no-this-alias */
     let block: this | null = this;
     if (block.isDeadOrDying()) {
@@ -1306,6 +1308,13 @@ export class BlockSvg
       if (blockOnly) break;
       block = block.getParent();
     } while (block);
+    if (previouslyFocused) {
+      // Bringing a block to the front of the stack doesn't fundamentally change
+      // the logical structure of the page, but it does change element ordering
+      // which can take automatically take away focus from a node. Ensure focus
+      // is restored to avoid a discontinuity.
+      getFocusManager().focusNode(previouslyFocused);
+    }
   }
 
   /**

core/clipboard/block_paster.ts

@@ -5,12 +5,14 @@
  */
 
 import {BlockSvg} from '../block_svg.js';
-import * as common from '../common.js';
+import {IFocusableNode} from '../blockly.js';
 import {config} from '../config.js';
 import {EventType} from '../events/type.js';
 import * as eventUtils from '../events/utils.js';
+import {getFocusManager} from '../focus_manager.js';
 import {ICopyData} from '../interfaces/i_copyable.js';
 import {IPaster} from '../interfaces/i_paster.js';
+import * as renderManagement from '../render_management.js';
 import {State, append} from '../serialization/blocks.js';
 import {Coordinate} from '../utils/coordinate.js';
 import {WorkspaceSvg} from '../workspace_svg.js';
@@ -55,7 +57,13 @@ export class BlockPaster implements IPaster<BlockCopyData, BlockSvg> {
     if (eventUtils.isEnabled() && !block.isShadow()) {
       eventUtils.fire(new (eventUtils.get(EventType.BLOCK_CREATE))(block));
     }
-    common.setSelected(block);
+
+    // Sometimes there's a delay before the block is fully created and ready for
+    // focusing, so wait slightly before focusing the newly pasted block.
+    const nodeToFocus: IFocusableNode = block;
+    renderManagement
+      .finishQueuedRenders()
+      .then(() => getFocusManager().focusNode(nodeToFocus));
     return block;
   }
 }

core/dragging/dragger.ts

@@ -8,11 +8,13 @@ import * as blockAnimations from '../block_animations.js';
 import {BlockSvg} from '../block_svg.js';
 import {ComponentManager} from '../component_manager.js';
 import * as eventUtils from '../events/utils.js';
+import {getFocusManager} from '../focus_manager.js';
 import {IDeletable, isDeletable} from '../interfaces/i_deletable.js';
 import {IDeleteArea} from '../interfaces/i_delete_area.js';
 import {IDragTarget} from '../interfaces/i_drag_target.js';
 import {IDraggable} from '../interfaces/i_draggable.js';
 import {IDragger} from '../interfaces/i_dragger.js';
+import {isFocusableNode} from '../interfaces/i_focusable_node.js';
 import * as registry from '../registry.js';
 import {Coordinate} from '../utils/coordinate.js';
 import {WorkspaceSvg} from '../workspace_svg.js';
@@ -129,6 +131,12 @@ export class Dragger implements IDragger {
       root.dispose();
     }
     eventUtils.setGroup(false);
+
+    if (!wouldDelete && isFocusableNode(this.draggable)) {
+      // Ensure focusable nodes that have finished dragging (but aren't being
+      // deleted) end with focus and selection.
+      getFocusManager().focusNode(this.draggable);
+    }
   }
 
   // We need to special case blocks for now so that we look at the root block

core/dropdowndiv.ts

@@ -629,10 +629,6 @@ export function hide() {
   animateOutTimer = setTimeout(function () {
     hideWithoutAnimation();
   }, ANIMATION_TIME * 1000);
-  if (returnEphemeralFocus) {
-    returnEphemeralFocus();
-    returnEphemeralFocus = null;
-  }
   if (onHide) {
     onHide();
     onHide = null;
@@ -648,10 +644,6 @@ export function hideWithoutAnimation() {
     clearTimeout(animateOutTimer);
   }
 
-  if (returnEphemeralFocus) {
-    returnEphemeralFocus();
-    returnEphemeralFocus = null;
-  }
   if (onHide) {
     onHide();
     onHide = null;
@@ -660,6 +652,11 @@ export function hideWithoutAnimation() {
   owner = null;
 
   (common.getMainWorkspace() as WorkspaceSvg).markFocused();
+
+  if (returnEphemeralFocus) {
+    returnEphemeralFocus();
+    returnEphemeralFocus = null;
+  }
 }
 
 /**

core/focus_manager.ts

@@ -56,17 +56,20 @@ export class FocusManager {
    */
   static readonly PASSIVE_FOCUS_NODE_CSS_CLASS_NAME = 'blocklyPassiveFocus';
 
-  focusedNode: IFocusableNode | null = null;
-  registeredTrees: Array<IFocusableTree> = [];
+  private focusedNode: IFocusableNode | null = null;
+  private previouslyFocusedNode: IFocusableNode | null = null;
+  private registeredTrees: Array<IFocusableTree> = [];
 
   private currentlyHoldsEphemeralFocus: boolean = false;
   private lockFocusStateChanges: boolean = false;
+  private recentlyLostAllFocus: boolean = false;
 
   constructor(
     addGlobalEventListener: (type: string, listener: EventListener) => void,
   ) {
     // Note that 'element' here is the element *gaining* focus.
     const maybeFocus = (element: Element | EventTarget | null) => {
+      this.recentlyLostAllFocus = !element;
       let newNode: IFocusableNode | null | undefined = null;
       if (element instanceof HTMLElement || element instanceof SVGElement) {
         // If the target losing or gaining focus maps to any tree, then it
@@ -164,7 +167,7 @@ export class FocusManager {
     const root = tree.getRootFocusableNode();
     if (focusedNode) this.removeHighlight(focusedNode);
     if (this.focusedNode === focusedNode || this.focusedNode === root) {
-      this.focusedNode = null;
+      this.updateFocusedNode(null);
     }
     this.removeHighlight(root);
   }
@@ -277,7 +280,7 @@ export class FocusManager {
       // Only change the actively focused node if ephemeral state isn't held.
       this.activelyFocusNode(focusableNode, prevTree ?? null);
     }
-    this.focusedNode = focusableNode;
+    this.updateFocusedNode(focusableNode);
   }
 
   /**
@@ -328,6 +331,22 @@ export class FocusManager {
 
       if (this.focusedNode) {
         this.activelyFocusNode(this.focusedNode, null);
+
+        // Even though focus was restored, check if it's lost again. It's
+        // possible for the browser to force focus away from all elements once
+        // the ephemeral element disappears. This ensures focus is restored.
+        const capturedNode = this.focusedNode;
+        setTimeout(() => {
+          // These checks are set up to minimize the risk that a legitimate
+          // focus change occurred within the delay that this would override.
+          if (
+            !this.focusedNode &&
+            this.previouslyFocusedNode === capturedNode &&
+            this.recentlyLostAllFocus
+          ) {
+            this.focusNode(capturedNode);
+          }
+        }, 0);
       }
     };
   }
@@ -348,6 +367,17 @@ export class FocusManager {
     }
   }
 
+  /**
+   * Updates the internally tracked focused node to the specified node, or null
+   * if focus is being lost. This also updates previous focus tracking.
+   *
+   * @param newFocusedNode The new node to set as focused.
+   */
+  private updateFocusedNode(newFocusedNode: IFocusableNode | null) {
+    this.previouslyFocusedNode = this.focusedNode;
+    this.focusedNode = newFocusedNode;
+  }
+
   /**
    * Defocuses the current actively focused node tracked by the manager, iff
    * there's a node being tracked and the manager doesn't have ephemeral focus.
@@ -358,7 +388,7 @@ export class FocusManager {
     // restored upon exiting ephemeral focus mode.
     if (this.focusedNode && !this.currentlyHoldsEphemeralFocus) {
       this.passivelyFocusNode(this.focusedNode, null);
-      this.focusedNode = null;
+      this.updateFocusedNode(null);
     }
   }
 

core/gesture.ts

@@ -25,6 +25,7 @@ import * as dropDownDiv from './dropdowndiv.js';
 import {EventType} from './events/type.js';
 import * as eventUtils from './events/utils.js';
 import type {Field} from './field.js';
+import {getFocusManager} from './focus_manager.js';
 import type {IBubble} from './interfaces/i_bubble.js';
 import {IDraggable, isDraggable} from './interfaces/i_draggable.js';
 import {IDragger} from './interfaces/i_dragger.js';
@@ -289,7 +290,7 @@ export class Gesture {
       // The start block is no longer relevant, because this is a drag.
       this.startBlock = null;
       this.targetBlock = this.flyout.createBlock(this.targetBlock);
-      common.setSelected(this.targetBlock);
+      getFocusManager().focusNode(this.targetBlock);
       return true;
     }
     return false;
@@ -734,6 +735,7 @@ export class Gesture {
       this.startComment.showContextMenu(e);
     } else if (this.startWorkspace_ && !this.flyout) {
       this.startWorkspace_.hideChaff();
+      getFocusManager().focusNode(this.startWorkspace_);
       this.startWorkspace_.showContextMenu(e);
     }
 
@@ -762,9 +764,12 @@ export class Gesture {
     this.mostRecentEvent = e;
 
     if (!this.startBlock && !this.startBubble && !this.startComment) {
-      // Selection determines what things start drags. So to drag the workspace,
-      // we need to deselect anything that was previously selected.
-      common.setSelected(null);
+      // Ensure the workspace is selected if nothing else should be. Note that
+      // this is focusNode() instead of focusTree() because if any active node
+      // is focused in the workspace it should be defocused.
+      getFocusManager().focusNode(ws);
+    } else if (this.startBlock) {
+      getFocusManager().focusNode(this.startBlock);
     }
 
     this.doStart(e);
@@ -865,13 +870,18 @@ export class Gesture {
       );
     }
 
+    // Note that the order is important here: bringing a block to the front will
+    // cause it to become focused and showing the field editor will capture
+    // focus ephemerally. It's important to ensure that focus is properly
+    // restored back to the block after field editing has completed.
+    this.bringBlockToFront();
+
     // Only show the editor if the field's editor wasn't already open
     // right before this gesture started.
     const dropdownAlreadyOpen = this.currentDropdownOwner === this.startField;
     if (!dropdownAlreadyOpen) {
       this.startField.showEditor(this.mostRecentEvent);
     }
-    this.bringBlockToFront();
   }
 
   /** Execute an icon click. */
@@ -901,6 +911,9 @@ export class Gesture {
         const newBlock = this.flyout.createBlock(this.targetBlock);
         newBlock.snapToGrid();
         newBlock.bumpNeighbours();
+
+        // If a new block was added, make sure that it's correctly focused.
+        getFocusManager().focusNode(newBlock);
       }
     } else {
       if (!this.startWorkspace_) {
@@ -928,11 +941,7 @@ export class Gesture {
    * @param _e A pointerup event.
    */
   private doWorkspaceClick(_e: PointerEvent) {
-    const ws = this.creatorWorkspace;
-    if (common.getSelected()) {
-      common.getSelected()!.unselect();
-    }
-    this.fireWorkspaceClick(this.startWorkspace_ || ws);
+    this.fireWorkspaceClick(this.startWorkspace_ || this.creatorWorkspace);
   }
 
   /* End functions defining what actions to take to execute clicks on each type
@@ -947,6 +956,8 @@ export class Gesture {
   private bringBlockToFront() {
     // Blocks in the flyout don't overlap, so skip the work.
     if (this.targetBlock && !this.flyout) {
+      // Always ensure the block being dragged/clicked has focus.
+      getFocusManager().focusNode(this.targetBlock);
       this.targetBlock.bringToFront();
     }
   }
@@ -1023,7 +1034,6 @@ export class Gesture {
     // If the gesture already went through a bubble, don't set the start block.
     if (!this.startBlock && !this.startBubble) {
       this.startBlock = block;
-      common.setSelected(this.startBlock);
       if (block.isInFlyout && block !== block.getRootBlock()) {
         this.setTargetBlock(block.getRootBlock());
       } else {
@@ -1046,6 +1056,7 @@ export class Gesture {
       this.setTargetBlock(block.getParent()!);
     } else {
       this.targetBlock = block;
+      getFocusManager().focusNode(block);
     }
   }
 

core/widgetdiv.ts

@@ -131,10 +131,6 @@ export function hide() {
   div.style.display = 'none';
   div.style.left = '';
   div.style.top = '';
-  if (returnEphemeralFocus) {
-    returnEphemeralFocus();
-    returnEphemeralFocus = null;
-  }
   if (dispose) {
     dispose();
     dispose = null;
@@ -150,6 +146,11 @@ export function hide() {
     themeClassName = '';
   }
   (common.getMainWorkspace() as WorkspaceSvg).markFocused();
+
+  if (returnEphemeralFocus) {
+    returnEphemeralFocus();
+    returnEphemeralFocus = null;
+  }
 }
 
 /**