From e1179808fd3bf0908adfefc36ab367cfb33feee0 Mon Sep 17 00:00:00 2001
From: Ben Henning <bhenning@google.com>
Date: Wed, 14 May 2025 10:50:00 -0700
Subject: [PATCH] fix: Ensure cursor syncs with more than just focused blocks
 (#9032)

## The basics

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

## The details
### Resolves

Fixes https://github.com/google/blockly-keyboard-experimentation/issues/499

### Proposed Changes

This ensures that non-blocks which hold active focus correctly update `LineCursor`'s internal state.

### Reason for Changes

This is outright a correction in how `LineCursor` has worked up until now, and is now possible after several recent changes (most notably #9004). #9004 updated selection to be more explicitly generic (and based on `IFocusableNode`) which means `LineCursor` should also properly support more than just blocks when synchronizing with focus (in place of selection), particularly since lots of non-block things can be focusable.

What's interesting is that this change isn't strictly necessary, even if it is a reasonable correction and improvement in the robustness of `LineCursor`. Essentially everywhere navigation is handled results in a call to `setCurNode` which correctly sets the cursor's internal state (with no specific correction from focus since only blocks were checked and we already ensure that selecting a block correctly focuses it).

### Test Coverage

It would be nice to add test coverage specifically for the cursor cases, but it seems largely unnecessary since:
1. The main failure cases are test-specific (as mentioned above).
2. These flows are better left tested as part of broader accessibility testing (per #8915).

This has been tested with a cursory playthrough of some basic scenarios (block movement, insertion, deletion, copy & paste, context menus, and interacting with fields).

### Documentation

No new documentation should be needed here.

### Additional Information

This is expected to only affect keyboard navigation plugin behaviors, particularly plugin tests.

It may be worth updating `LineCursor` to completely reflect current focus state rather than holding an internal variable. This, in turn, may end up simplifying solving issues like #8793 (but not necessarily).
---
 core/keyboard_nav/line_cursor.ts | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/core/keyboard_nav/line_cursor.ts b/core/keyboard_nav/line_cursor.ts
index 71c61b4f3..9d83f6554 100644
--- a/core/keyboard_nav/line_cursor.ts
+++ b/core/keyboard_nav/line_cursor.ts
@@ -374,7 +374,15 @@ export class LineCursor extends Marker {
    * @returns The current field, connection, or block the cursor is on.
    */
   override getCurNode(): IFocusableNode | null {
-    this.updateCurNodeFromFocus();
+    // Ensure the current node matches what's currently focused.
+    const focused = getFocusManager().getFocusedNode();
+    const block = this.getSourceBlockFromNode(focused);
+    if (!block || block.workspace === this.workspace) {
+      // If the current focused node corresponds to a block then ensure that it
+      // belongs to the correct workspace for this cursor.
+      this.setCurNode(focused);
+    }
+
     return super.getCurNode();
   }
 
@@ -401,20 +409,6 @@ export class LineCursor extends Marker {
     }
   }
 
-  /**
-   * Updates the current node to match what's currently focused.
-   */
-  private updateCurNodeFromFocus() {
-    const focused = getFocusManager().getFocusedNode();
-
-    if (focused instanceof BlockSvg) {
-      const block: BlockSvg | null = focused;
-      if (block && block.workspace === this.workspace) {
-        this.setCurNode(block);
-      }
-    }
-  }
-
   /**
    * Get the first navigable node on the workspace, or null if none exist.
    *