google · BenHenning · May 2, 2025 · Apr 21, 2025 · Apr 21, 2025 · Apr 22, 2025
@@ -120,6 +120,7 @@ import * as inputs from './inputs.js';
 import {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
 import {LabelFlyoutInflater} from './label_flyout_inflater.js';
 import {SeparatorFlyoutInflater} from './separator_flyout_inflater.js';
+import {FocusableTreeTraverser} from './utils/focusable_tree_traverser.js';
 
 import {Input} from './inputs/input.js';
 import {InsertionMarkerPreviewer} from './insertion_marker_previewer.js';
@@ -527,6 +528,7 @@ export {
   FlyoutMetricsManager,
   FlyoutSeparator,
   FocusManager,
+  FocusableTreeTraverser,
   CodeGenerator as Generator,
   Gesture,
   Grid,

@@ -65,26 +65,15 @@ export class FocusManager {
   constructor(
     addGlobalEventListener: (type: string, listener: EventListener) => void,
   ) {
-    // Register root document focus listeners for tracking when focus leaves all
-    // tracked focusable trees.
-    addGlobalEventListener('focusin', (event) => {
-      if (!(event instanceof FocusEvent)) return;
-
-      // The target that now has focus.
-      const activeElement = document.activeElement;
+    // Note that 'element' here is the element *gaining* focus.
+    const maybeFocus = (element: Element | EventTarget | null) => {
       let newNode: IFocusableNode | null | undefined = null;
-      if (
-        activeElement instanceof HTMLElement ||
-        activeElement instanceof SVGElement
-      ) {
-        // If the target losing focus maps to any tree, then it should be
-        // updated. Per the contract of findFocusableNodeFor only one tree
-        // should claim the element.
+      if (element instanceof HTMLElement || element instanceof SVGElement) {
+        // If the target losing or gaining focus maps to any tree, then it
+        // should be updated. Per the contract of findFocusableNodeFor only one
+        // tree should claim the element, so the search can be exited early.
         for (const tree of this.registeredTrees) {
-          newNode = FocusableTreeTraverser.findFocusableNodeFor(
-            activeElement,
-            tree,
-          );
+          newNode = FocusableTreeTraverser.findFocusableNodeFor(element, tree);
           if (newNode) break;
         }
       }
@@ -103,6 +92,26 @@ export class FocusManager {
       } else {
         this.defocusCurrentFocusedNode();
       }
+    };
+
+    // Register root document focus listeners for tracking when focus leaves all
+    // tracked focusable trees. Note that focusin and focusout can be somewhat
+    // overlapping in the information that they provide. This is fine because
+    // they both aim to check for focus changes on the element gaining or having
+    // received focus, and maybeFocus should behave relatively deterministic.
+    addGlobalEventListener('focusin', (event) => {
+      if (!(event instanceof FocusEvent)) return;
+
+      // When something receives focus, always use the current active element as
+      // the source of truth.
+      maybeFocus(document.activeElement);
+    });
+    addGlobalEventListener('focusout', (event) => {
+      if (!(event instanceof FocusEvent)) return;
+
+      // When something loses focus, it seems that document.activeElement may
+      // not necessarily be correct. Instead, use relatedTarget.
+      maybeFocus(event.relatedTarget);
     });
   }
 
@@ -247,7 +256,7 @@ export class FocusManager {
 
     const prevNode = this.focusedNode;
     const prevTree = prevNode?.getFocusableTree();
-    if (prevNode && prevTree !== nextTree) {
+    if (prevNode) {
       this.passivelyFocusNode(prevNode, nextTree);
     }
 
@@ -374,7 +383,9 @@ export class FocusManager {
     // node's focusable element (which *is* allowed to be invisible until the
     // node needs to be focused).
     this.lockFocusStateChanges = true;
-    node.getFocusableTree().onTreeFocus(node, prevTree);
+    if (node.getFocusableTree() !== prevTree) {
+      node.getFocusableTree().onTreeFocus(node, prevTree);
+    }
     node.onNodeFocus();
     this.lockFocusStateChanges = false;
 
@@ -399,11 +410,15 @@ export class FocusManager {
     nextTree: IFocusableTree | null,
   ): void {
     this.lockFocusStateChanges = true;
-    node.getFocusableTree().onTreeBlur(nextTree);
+    if (node.getFocusableTree() !== nextTree) {
+      node.getFocusableTree().onTreeBlur(nextTree);
+    }
     node.onNodeBlur();
     this.lockFocusStateChanges = false;
 
-    this.setNodeToVisualPassiveFocus(node);
+    if (node.getFocusableTree() !== nextTree) {
+      this.setNodeToVisualPassiveFocus(node);
+    }
   }
 
   /**
@@ -441,19 +456,24 @@ export class FocusManager {
     dom.removeClass(element, FocusManager.ACTIVE_FOCUS_NODE_CSS_CLASS_NAME);
     dom.removeClass(element, FocusManager.PASSIVE_FOCUS_NODE_CSS_CLASS_NAME);
   }
-}
 
-let focusManager: FocusManager | null = null;
+  private static focusManager: FocusManager | null = null;
 
-/**
- * Returns the page-global FocusManager.
- *
- * The returned instance is guaranteed to not change across function calls, but
- * may change across page loads.
- */
-export function getFocusManager(): FocusManager {
-  if (!focusManager) {
-    focusManager = new FocusManager(document.addEventListener);
+  /**
+   * Returns the page-global FocusManager.
+   *
+   * The returned instance is guaranteed to not change across function calls, but
+   * may change across page loads.
+   */
+  static getFocusManager(): FocusManager {
+    if (!FocusManager.focusManager) {
+      FocusManager.focusManager = new FocusManager(document.addEventListener);
+    }
+    return FocusManager.focusManager;
   }
-  return focusManager;
+}
+
+/** Convenience function for FocusManager.getFocusManager. */
+export function getFocusManager(): FocusManager {
+  return FocusManager.getFocusManager();
 }
@@ -18,11 +18,9 @@ import {BlockSvg} from '../block_svg.js';
 import * as common from '../common.js';
 import type {Connection} from '../connection.js';
 import {ConnectionType} from '../connection_type.js';
-import type {Abstract} from '../events/events_abstract.js';
-import {Click, ClickTarget} from '../events/events_click.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 {isFocusableNode} from '../interfaces/i_focusable_node.js';
 import * as registry from '../registry.js';
 import type {MarkerSvg} from '../renderers/common/marker_svg.js';
 import type {PathObject} from '../renderers/zelos/path_object.js';
@@ -70,23 +68,12 @@ export class LineCursor extends Marker {
     options?: Partial<CursorOptions>,
   ) {
     super();
-    // Bind changeListener to facilitate future disposal.
-    this.changeListener = this.changeListener.bind(this);
-    this.workspace.addChangeListener(this.changeListener);
     // Regularise options and apply defaults.
     this.options = {...defaultOptions, ...options};
 
     this.isZelos = workspace.getRenderer() instanceof Renderer;
   }
 
-  /**
-   * Clean up this cursor.
-   */
-  dispose() {
-    this.workspace.removeChangeListener(this.changeListener);
-    super.dispose();
-  }
-
   /**
    * Moves the cursor to the next previous connection, next connection or block
    * in the pre order traversal. Finds the next node in the pre order traversal.
@@ -331,8 +318,8 @@ export class LineCursor extends Marker {
    * @param node The current position in the AST.
    * @param isValid A function true/false depending on whether the given node
    *     should be traversed.
-   * @param loop Whether to loop around to the beginning of the workspace if
-   *     novalid node was found.
+   * @param loop Whether to loop around to the beginning of the workspace if no
+   *     valid node was found.
    * @returns The next node in the traversal.
    */
   getNextNode(
@@ -385,8 +372,8 @@ export class LineCursor extends Marker {
    * @param node The current position in the AST.
    * @param isValid A function true/false depending on whether the given node
    *     should be traversed.
-   * @param loop Whether to loop around to the end of the workspace if no
-   *     valid node was found.
+   * @param loop Whether to loop around to the end of the workspace if no valid
+   *     node was found.
    * @returns The previous node in the traversal or null if no previous node
    *     exists.
    */
@@ -527,7 +514,12 @@ export class LineCursor extends Marker {
    * @returns The current field, connection, or block the cursor is on.
    */
   override getCurNode(): ASTNode | null {
-    this.updateCurNodeFromSelection();
+    if (!this.updateCurNodeFromFocus()) {
+      // Fall back to selection if focus fails to sync. This can happen for
+      // non-focusable nodes or for cases when focus may not properly propagate
+      // (such as for mouse clicks).
+      this.updateCurNodeFromSelection();
+    }
     return super.getCurNode();
   }
 
@@ -572,16 +564,15 @@ export class LineCursor extends Marker {
    * this.drawMarker() instead of this.drawer.draw() directly.
    *
    * @param newNode The new location of the cursor.
-   * @param updateSelection If true (the default) we'll update the selection
-   *     too.
    */
-  override setCurNode(newNode: ASTNode | null, updateSelection = true) {
-    if (updateSelection) {
-      this.updateSelectionFromNode(newNode);
-    }
-
+  override setCurNode(newNode: ASTNode | null) {
     super.setCurNode(newNode);
 
+    const newNodeLocation = newNode?.getLocation();
+    if (isFocusableNode(newNodeLocation)) {
+      getFocusManager().focusNode(newNodeLocation);
+    }
+
     // Try to scroll cursor into view.
     if (newNode?.getType() === ASTNode.types.BLOCK) {
       const block = newNode.getLocation() as BlockSvg;
@@ -709,32 +700,6 @@ export class LineCursor extends Marker {
     }
   }
 
-  /**
-   * Event listener that syncs the cursor location to the selected block on
-   * SELECTED events.
-   *
-   * This does not run early enough in all cases so `getCurNode()` also updates
-   * the node from the selection.
-   *
-   * @param event The `Selected` event.
-   */
-  private changeListener(event: Abstract) {
-    switch (event.type) {
-      case EventType.SELECTED:
-        this.updateCurNodeFromSelection();
-        break;
-      case EventType.CLICK: {
-        const click = event as Click;
-        if (
-          click.workspaceId === this.workspace.id &&
-          click.targetType === ClickTarget.WORKSPACE
-        ) {
-          this.setCurNode(null);
-        }
-      }
-    }
-  }
-
   /**
    * Updates the current node to match the selection.
    *
@@ -749,7 +714,7 @@ export class LineCursor extends Marker {
     const selected = common.getSelected();
 
     if (selected === null && curNode?.getType() === ASTNode.types.BLOCK) {
-      this.setCurNode(null, false);
+      this.setCurNode(null);
       return;
     }
     if (selected?.workspace !== this.workspace) {
@@ -770,36 +735,35 @@ export class LineCursor extends Marker {
         block = block.getParent();
       }
       if (block) {
-        this.setCurNode(ASTNode.createBlockNode(block), false);
+        this.setCurNode(ASTNode.createBlockNode(block));
       }
     }
   }
 
   /**
-   * Updates the selection from the node.
+   * Updates the current node to match what's currently focused.
    *
-   * Clears the selection for non-block nodes.
-   * Clears the selection for shadow blocks as the selection is drawn on
-   * the parent but the cursor will be drawn on the shadow block itself.
-   * We need to take care not to later clear the current node due to that null
-   * selection, so we track the latest selection we're in sync with.
-   *
-   * @param newNode The new node.
+   * @returns Whether the current node has been set successfully from the
+   *     current focused node.
    */
-  private updateSelectionFromNode(newNode: ASTNode | null) {
-    if (newNode?.getType() === ASTNode.types.BLOCK) {
-      if (common.getSelected() !== newNode.getLocation()) {
-        eventUtils.disable();
-        common.setSelected(newNode.getLocation() as BlockSvg);
-        eventUtils.enable();
-      }
-    } else {
-      if (common.getSelected()) {
-        eventUtils.disable();
-        common.setSelected(null);
-        eventUtils.enable();
+  private updateCurNodeFromFocus(): boolean {
+    const focused = getFocusManager().getFocusedNode();
+
+    if (focused instanceof BlockSvg) {
+      const block: BlockSvg | null = focused;
+      if (block && block.workspace === this.workspace) {
+        if (block.getRootBlock() === block && this.workspace.isFlyout) {
+          // This block actually represents a stack. Note that this is needed
+          // because ASTNode special cases stack for cross-block navigation.
+          this.setCurNode(ASTNode.createStackNode(block));
+        } else {
+          this.setCurNode(ASTNode.createBlockNode(block));
+        }
+        return true;
       }
     }
+
+    return false;
   }
 
   /**

@@ -6,6 +6,7 @@
 
 import {ASTNode} from '../../build/src/core/keyboard_nav/ast_node.js';
 import {assert} from '../../node_modules/chai/chai.js';
+import {createRenderedBlock} from './test_helpers/block_definitions.js';
 import {
   sharedTestSetup,
   sharedTestTeardown,
@@ -63,11 +64,11 @@ suite('Cursor', function () {
       ]);
       this.workspace = Blockly.inject('blocklyDiv', {});
       this.cursor = this.workspace.getCursor();
-      const blockA = this.workspace.newBlock('input_statement');
-      const blockB = this.workspace.newBlock('input_statement');
-      const blockC = this.workspace.newBlock('input_statement');
-      const blockD = this.workspace.newBlock('input_statement');
-      const blockE = this.workspace.newBlock('field_input');
+      const blockA = createRenderedBlock(this.workspace, 'input_statement');
+      const blockB = createRenderedBlock(this.workspace, 'input_statement');
+      const blockC = createRenderedBlock(this.workspace, 'input_statement');
+      const blockD = createRenderedBlock(this.workspace, 'input_statement');
+      const blockE = createRenderedBlock(this.workspace, 'field_input');
 
       blockA.nextConnection.connect(blockB.previousConnection);
       blockA.inputList[0].connection.connect(blockE.outputConnection);
@@ -105,12 +106,12 @@ suite('Cursor', function () {
       );
     });
 
-    test('In - From output connection', function () {
+    test('In - From attached input connection', function () {
       const fieldBlock = this.blocks.E;
-      const outputNode = ASTNode.createConnectionNode(
-        fieldBlock.outputConnection,
+      const inputConnectionNode = ASTNode.createConnectionNode(
+        this.blocks.A.inputList[0].connection,
       );
-      this.cursor.setCurNode(outputNode);
+      this.cursor.setCurNode(inputConnectionNode);
       this.cursor.in();
       const curNode = this.cursor.getCurNode();
       assert.equal(curNode.getLocation(), fieldBlock);