fix: skip empty label fields and dummy/end row inputs for move announcements (#9847)

* fix: insert custom input label before children

* fix: do not add empty field labels to block label

* fix: revert ariaLabelProvider changes

* fix: add override designation

* fix: update test after merge conflict

packages/blockly/core/block_aria_composer.ts

@@ -123,6 +123,10 @@ export function configureAriaRole(block: BlockSvg) {
  * `lookback` attribute is specified, all of the fields on the row immediately
  * above the Input will be used instead.
  *
+ * Empty field labels are excluded because they don't provide useful context.
+ * Fields should generally have a helpful label, but there are exceptions, such
+ * as when empty label fields are used to control the layout of a block.
+ *
  * @internal
  * @param input The Input to compute a description/context label for.
  * @param lookback If true, will use labels for fields on the previous row if
@@ -146,7 +150,7 @@ export function computeFieldRowLabel(
       return computeFieldRowLabel(inputs[index - 1], lookback, verbosity);
     }
   }
-  return fieldRowLabel;
+  return fieldRowLabel.filter((label) => !!label);
 }
 
 /**
@@ -249,11 +253,7 @@ export function getInputLabels(
  * @param input The input that defines the end of the subset.
  * @returns A list of field/input labels for the given block.
  */
-export function getInputLabelsSubset(
-  block: BlockSvg,
-  input: Input,
-  verbosity = Verbosity.STANDARD,
-): string[] {
+function getInputLabelsSubset(block: BlockSvg, input: Input): string[] {
   const inputIndex = block.inputList.indexOf(input);
   if (inputIndex === -1) {
     throw new Error(
@@ -271,7 +271,7 @@ export function getInputLabelsSubset(
     .filter((input) => input.isVisible())
     .map(
       (input) =>
-        input.getLabel(verbosity) ||
+        input.getLabel(Verbosity.TERSE) ||
         Msg['INPUT_LABEL_INDEX'].replace(
           '%1',
           (input.getIndex() + 1).toString(),
@@ -404,11 +404,7 @@ function computeMoveConnectionLabel(
   // If the input doesn't have a custom ARIA label, compute one using the labels from
   // nearby fields.
   if (!inputLabel) {
-    const labels = getInputLabelsSubset(
-      conn.getSourceBlock(),
-      input,
-      Verbosity.TERSE,
-    );
+    const labels = getInputLabelsSubset(conn.getSourceBlock(), input);
     if (!labels.length) return baseLabel;
 
     inputLabel = labels.join(', ');

packages/blockly/core/field_label.ts

@@ -81,6 +81,42 @@ export class FieldLabel extends Field<string> {
     }
   }
 
+  /**
+   * Computes a descriptive ARIA label to represent this field with configurable
+   * verbosity.
+   *
+   * A 'verbose' label includes type information, if available, whereas a
+   * non-verbose label only contains the field's value.
+   *
+   * Note that this will always return the latest representation of the field's
+   * label which may differ from any previously set ARIA label for the field
+   * itself. Implementations are largely responsible for ensuring that the
+   * field's ARIA label is set correctly at relevant moments in the field's
+   * lifecycle (such as when its value changes).
+   *
+   * Finally, it is never guaranteed that implementations use the label returned
+   * by this method for their actual ARIA label. Some implementations may rely
+   * on other contexts to convey information like the field's value. Example:
+   * checkboxes represent their checked/non-checked status (i.e. value) through
+   * a separate ARIA property.
+   *
+   * Unlike other built-in fields, FieldLabel does return an empty string when its
+   * value is empty. This is because empty labels are sometimes used for layout
+   * purposes.
+   *
+   * @param includeTypeInfo Whether to include the field's type information in
+   *     the returned label, if available.
+   */
+  override computeAriaLabel(includeTypeInfo: boolean = true): string {
+    const ariaTypeName = includeTypeInfo ? this.getAriaTypeName() : null;
+    const ariaValue = this.getAriaValue() ?? '';
+
+    if (ariaTypeName) {
+      return `${ariaTypeName}: ${ariaValue}`;
+    }
+    return ariaValue;
+  }
+
   /**
    * Ensure that the input value casts to a valid string.
    *

packages/blockly/core/inputs/input.ts

@@ -418,12 +418,17 @@ export class Input {
   }
 
   /**
-   * Returns the index of this input on its source block.
+   * Returns the index of this input, excluding inputs without connections, on its
+   * source block.
    *
    * @internal
    */
   getIndex(): number {
-    const inputs = this.getSourceBlock().inputList;
-    return inputs.indexOf(this);
+    const noConnectionInputTypes = [inputTypes.DUMMY, inputTypes.END_ROW];
+    const allInputs = this.getSourceBlock().inputList;
+    const allConnectionInputs = allInputs.filter(
+      (input) => !noConnectionInputTypes.includes(input.type),
+    );
+    return allConnectionInputs.indexOf(this);
   }
 }

packages/blockly/tests/mocha/keyboard_movement_test.js

@@ -1403,6 +1403,50 @@ suite('Keyboard-driven movement', function () {
 
         cancelMove(this.workspace);
       });
+      test('ignores dummy inputs when disambiguating', function () {
+        const subListBlock = this.workspace.newBlock('lists_getSublist');
+        subListBlock.initSvg();
+        subListBlock.render();
+        const mathBlock = this.workspace.newBlock('math_number');
+        mathBlock.initSvg();
+        mathBlock.render();
+
+        Blockly.getFocusManager().focusNode(mathBlock);
+        startMove(this.workspace);
+        this.clock.tick(10);
+        this.moveAndAssert(
+          moveRight,
+          ['Moving', 'to', 'list, get sub-list from', 'input 2'],
+          ['input 3'],
+        );
+        this.moveAndAssert(
+          moveRight,
+          ['Moving', 'to', 'list, get sub-list from', 'input 3'],
+          ['input 4'],
+        );
+
+        cancelMove(this.workspace);
+      });
+      test('ignores end row inputs when disambiguating', function () {
+        const compare = this.workspace.newBlock('logic_compare');
+        compare.appendDummyInput('END_ROW');
+        compare.moveInputBefore('END_ROW', 'A');
+        compare.initSvg();
+        compare.render();
+        const boolean = this.workspace.newBlock('logic_boolean');
+        boolean.initSvg();
+        boolean.render();
+
+        Blockly.getFocusManager().focusNode(boolean);
+        startMove(this.workspace);
+        this.clock.tick(10);
+        this.moveAndAssert(
+          moveRight,
+          ['Moving', 'to', 'input 1', '='],
+          [this.getBlockLabel(boolean)],
+        );
+        cancelMove(this.workspace);
+      });
     });
   });