fix: Make up/previous navigation consistent with down/next. (#9437)

* fix: Make up/previous navigation consistent with down/next.

* fix: Don't visit nested input blocks when moving up/previous.

Author: gonfunko

core/keyboard_nav/block_navigation_policy.ts

@@ -137,9 +137,9 @@ function getBlockNavigationCandidates(
           .lastConnectionInStack(false)
           ?.getSourceBlock();
         if (lastStackBlock) {
-          // When navigating backward, the last block in a stack in a statement
-          // input is navigable.
-          candidates.push(lastStackBlock);
+          // When navigating backward, the last next connection in a stack in a
+          // statement input is navigable.
+          candidates.push(lastStackBlock.nextConnection);
         }
       } else {
         // When navigating forward, a child block connected to a statement
@@ -198,9 +198,13 @@ export function navigateStacks(current: ISelectable, delta: number) {
   }
 
   // When navigating to a previous block stack, our previous sibling is the last
-  // block in it.
+  // block or nested next connection in it.
   if (delta < 0 && result instanceof BlockSvg) {
-    return result.lastConnectionInStack(false)?.getSourceBlock() ?? result;
+    result = result.lastConnectionInStack(false)?.getSourceBlock() ?? result;
+
+    if (result instanceof BlockSvg && result.statementInputCount > 0) {
+      result = getBlockNavigationCandidates(result, false).at(-1) ?? result;
+    }
   }
 
   return result;

core/keyboard_nav/line_cursor.ts

@@ -338,8 +338,44 @@ export class LineCursor extends Marker {
             return true;
           }
 
-          const current = this.getSourceBlockFromNode(this.getCurNode());
-          if (candidate instanceof BlockSvg && current instanceof BlockSvg) {
+          const currentNode = this.getCurNode();
+          if (direction === NavigationDirection.PREVIOUS) {
+            // Don't visit rightmost/nested blocks in statement blocks when
+            // navigating to the previous block.
+            if (
+              currentNode instanceof RenderedConnection &&
+              currentNode.type === ConnectionType.NEXT_STATEMENT &&
+              !currentNode.getParentInput() &&
+              candidate !== currentNode.getSourceBlock()
+            ) {
+              return false;
+            }
+
+            // Don't visit the first value/input block in a block with statement
+            // inputs when navigating to the previous block. This is consistent
+            // with the behavior when navigating to the next block and avoids
+            // duplicative screen reader narration. Also don't visit value
+            // blocks nested in non-statement inputs.
+            if (
+              candidate instanceof BlockSvg &&
+              candidate.outputConnection?.targetConnection
+            ) {
+              const parentInput =
+                candidate.outputConnection.targetConnection.getParentInput();
+              if (
+                !parentInput?.getSourceBlock().statementInputCount ||
+                parentInput?.getSourceBlock().inputList[0] === parentInput
+              ) {
+                return false;
+              }
+            }
+          }
+
+          const currentBlock = this.getSourceBlockFromNode(currentNode);
+          if (
+            candidate instanceof BlockSvg &&
+            currentBlock instanceof BlockSvg
+          ) {
             // If the candidate's parent uses inline inputs, disallow the
             // candidate; it follows that it must be on the same row as its
             // parent.
@@ -352,13 +388,13 @@ export class LineCursor extends Marker {
             // block, disallow it; it cannot be on a different row than the
             // current block.
             if (
-              current === this.getCurNode() &&
-              candidateParents.has(current)
+              currentBlock === this.getCurNode() &&
+              candidateParents.has(currentBlock)
             ) {
               return false;
             }
 
-            const currentParents = this.getParents(current);
+            const currentParents = this.getParents(currentBlock);
 
             const sharedParents = currentParents.intersection(candidateParents);
             // Allow the candidate if it and the current block have no parents