Skip to content

fix: Clean up accessibility node hierarchy (experimental) #9449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BenHenning merged 17 commits into from
Dec 18, 2025
Merged

fix: Clean up accessibility node hierarchy (experimental) #9449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b1d3d6d
feat: Clean up a11y node hierarchy.
BenHenning Oct 30, 2025
a7f7810
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Nov 4, 2025
f81e2b2
fix: Fix CI failures.
BenHenning Nov 12, 2025
cc5d002
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Nov 26, 2025
7e1b4d5
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Dec 4, 2025
18200c9
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Dec 8, 2025
eeaff73
chore: Simplify connectiong gating logic.
BenHenning Dec 8, 2025
5e28067
chore: Reduce complexity.
BenHenning Dec 9, 2025
4a6f102
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Dec 11, 2025
14cd72c
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Dec 11, 2025
8dacb41
fix: Broken tests due to connection change.
BenHenning Dec 12, 2025
c19eaa5
chore: Lint fixes.
BenHenning Dec 12, 2025
71a9678
fix: Fix output connections in C-shaped blocks.
BenHenning Dec 12, 2025
be3e909
Merge branch 'add-screen-reader-support-experimental' into clean-up-n…
BenHenning Dec 16, 2025
3c9df1f
chore: Remove redundant check.
BenHenning Dec 16, 2025
a063709
chore: Attempt a different approach.
BenHenning Dec 17, 2025
f9e745f
chore: Remove ownership code.
BenHenning Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/block_svg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ export class BlockSvg
this.workspace.getCanvas().appendChild(svg);
}
this.initialized = true;
this.recomputeAriaLabel();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/rendered_connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ export class RenderedConnection

/** See IFocusableNode.canBeFocused. */
canBeFocused(): boolean {
return true;
return this.findHighlightSvg() !== null;
Copy link
Contributor

@maribethb maribethb Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels weird to me but I can't totally articulate why. I guess it feels like we're relying on some rendering detail to make a decision about something that feels more like a business logic decision. But I'm not sure in which scenarios the highlight Svg wouldn't exist, so maybe that's okay.

Copy link
Collaborator Author

@BenHenning BenHenning Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that unease is valid, and I think I also share it. We compromised on how the highlight SVG is managed for focus which is why there's a tight coupling between the SVG and focus-related operations. This should increase the safety of interacting with RenderedConnection since getFocusableElement will throw if the SVG is missing since it's the focusable element. Ideally we would change the lifecycle management to always guarantee the SVG's presence and rely only on visibility rather than presence but, as I understand it, the highlight SVG is created separately from the class (due to it being created asynchronously in the rendering pipeline rather than part of much of the rest of the DOM).

}

private findHighlightSvg(): SVGPathElement | null {
Expand Down
2 changes: 1 addition & 1 deletion core/utils/dom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function createSvgElement<T extends SVGElement>(
opt_parent.appendChild(e);
}
if (name === Svg.SVG || name === Svg.G) {
aria.setRole(e, aria.Role.PRESENTATION);
aria.setRole(e, aria.Role.GENERIC);
}
return e;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/mocha/cursor_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ suite('Cursor', function () {
});
suite('one empty block', function () {
setup(function () {
this.blockA = this.workspace.newBlock('empty_block');
this.blockA = createRenderedBlock(this.workspace, 'empty_block');
});
teardown(function () {
this.workspace.clear();
Expand All @@ -359,7 +359,7 @@ suite('Cursor', function () {

suite('one stack block', function () {
setup(function () {
this.blockA = this.workspace.newBlock('stack_block');
this.blockA = createRenderedBlock(this.workspace, 'stack_block');
});
teardown(function () {
this.workspace.clear();
Expand All @@ -376,7 +376,7 @@ suite('Cursor', function () {

suite('one row block', function () {
setup(function () {
this.blockA = this.workspace.newBlock('row_block');
this.blockA = createRenderedBlock(this.workspace, 'row_block');
});
teardown(function () {
this.workspace.clear();
Expand All @@ -392,7 +392,7 @@ suite('Cursor', function () {
});
suite('one c-hat block', function () {
setup(function () {
this.blockA = this.workspace.newBlock('c_hat_block');
this.blockA = createRenderedBlock(this.workspace, 'c_hat_block');
});
teardown(function () {
this.workspace.clear();
Expand Down
123 changes: 75 additions & 48 deletions tests/mocha/navigation_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
*/

import {assert} from '../../node_modules/chai/index.js';
import {createRenderedBlock} from './test_helpers/block_definitions.js';
import {
sharedTestSetup,
sharedTestTeardown,
workspaceTeardown,
} from './test_helpers/setup_teardown.js';

suite('Navigation', function () {
Expand Down Expand Up @@ -89,13 +89,28 @@ suite('Navigation', function () {
]);
this.workspace = Blockly.inject('blocklyDiv', {});
this.navigator = this.workspace.getNavigator();
const statementInput1 = this.workspace.newBlock('input_statement');
const statementInput2 = this.workspace.newBlock('input_statement');
const statementInput3 = this.workspace.newBlock('input_statement');
const statementInput4 = this.workspace.newBlock('input_statement');
const fieldWithOutput = this.workspace.newBlock('field_input');
const doubleValueInput = this.workspace.newBlock('double_value_input');
const valueInput = this.workspace.newBlock('value_input');
const statementInput1 = createRenderedBlock(
this.workspace,
'input_statement',
);
const statementInput2 = createRenderedBlock(
this.workspace,
'input_statement',
);
const statementInput3 = createRenderedBlock(
this.workspace,
'input_statement',
);
const statementInput4 = createRenderedBlock(
this.workspace,
'input_statement',
);
const fieldWithOutput = createRenderedBlock(this.workspace, 'field_input');
const doubleValueInput = createRenderedBlock(
this.workspace,
'double_value_input',
);
const valueInput = createRenderedBlock(this.workspace, 'value_input');

statementInput1.nextConnection.connect(statementInput2.previousConnection);
statementInput1.inputList[0].connection.connect(
Expand Down Expand Up @@ -355,13 +370,25 @@ suite('Navigation', function () {
'helpUrl': '',
},
]);
const noNextConnection = this.workspace.newBlock('top_connection');
const fieldAndInputs = this.workspace.newBlock('fields_and_input');
const twoFields = this.workspace.newBlock('two_fields');
const fieldAndInputs2 = this.workspace.newBlock('fields_and_input2');
const noPrevConnection = this.workspace.newBlock('start_block');
const hiddenField = this.workspace.newBlock('hidden_field');
const hiddenInput = this.workspace.newBlock('hidden_input');
const noNextConnection = createRenderedBlock(
this.workspace,
'top_connection',
);
const fieldAndInputs = createRenderedBlock(
this.workspace,
'fields_and_input',
);
const twoFields = createRenderedBlock(this.workspace, 'two_fields');
const fieldAndInputs2 = createRenderedBlock(
this.workspace,
'fields_and_input2',
);
const noPrevConnection = createRenderedBlock(
this.workspace,
'start_block',
);
const hiddenField = createRenderedBlock(this.workspace, 'hidden_field');
const hiddenInput = createRenderedBlock(this.workspace, 'hidden_input');
this.blocks.noNextConnection = noNextConnection;
this.blocks.fieldAndInputs = fieldAndInputs;
this.blocks.twoFields = twoFields;
Expand All @@ -373,28 +400,47 @@ suite('Navigation', function () {
hiddenField.inputList[0].fieldRow[1].setVisible(false);
hiddenInput.inputList[1].setVisible(false);

const dummyInput = this.workspace.newBlock('dummy_input');
const dummyInputValue = this.workspace.newBlock('dummy_inputValue');
const fieldWithOutput2 = this.workspace.newBlock('field_input');
const dummyInput = createRenderedBlock(this.workspace, 'dummy_input');
const dummyInputValue = createRenderedBlock(
this.workspace,
'dummy_inputValue',
);
const fieldWithOutput2 = createRenderedBlock(
this.workspace,
'field_input',
);
this.blocks.dummyInput = dummyInput;
this.blocks.dummyInputValue = dummyInputValue;
this.blocks.fieldWithOutput2 = fieldWithOutput2;

const secondBlock = this.workspace.newBlock('input_statement');
const outputNextBlock = this.workspace.newBlock('output_next');
const secondBlock = createRenderedBlock(
this.workspace,
'input_statement',
);
const outputNextBlock = createRenderedBlock(
this.workspace,
'output_next',
);
this.blocks.secondBlock = secondBlock;
this.blocks.outputNextBlock = outputNextBlock;

const buttonBlock = this.workspace.newBlock('buttons', 'button_block');
const buttonInput1 = this.workspace.newBlock(
const buttonBlock = createRenderedBlock(
this.workspace,
'buttons',
'button_block',
);
const buttonInput1 = createRenderedBlock(
this.workspace,
'field_input',
'button_input1',
);
const buttonInput2 = this.workspace.newBlock(
const buttonInput2 = createRenderedBlock(
this.workspace,
'field_input',
'button_input2',
);
const buttonNext = this.workspace.newBlock(
const buttonNext = createRenderedBlock(
this.workspace,
'input_statement',
'button_next',
);
Expand All @@ -420,15 +466,6 @@ suite('Navigation', function () {
this.workspace.cleanUp();
});
suite('Next', function () {
setup(function () {
this.singleBlockWorkspace = new Blockly.Workspace();
const singleBlock = this.singleBlockWorkspace.newBlock('two_fields');
this.blocks.singleBlock = singleBlock;
});
teardown(function () {
workspaceTeardown.call(this, this.singleBlockWorkspace);
});

test('fromPreviousToBlock', function () {
const prevConnection = this.blocks.statementInput1.previousConnection;
const nextNode = this.navigator.getNextSibling(prevConnection);
Expand Down Expand Up @@ -471,8 +508,6 @@ suite('Navigation', function () {
});
test('fromFieldToNestedBlock', function () {
const field = this.blocks.statementInput1.inputList[0].fieldRow[1];
const inputConnection =
this.blocks.statementInput1.inputList[0].connection;
const nextNode = this.navigator.getNextSibling(field);
assert.equal(nextNode, this.blocks.fieldWithOutput);
});
Expand Down Expand Up @@ -576,7 +611,6 @@ suite('Navigation', function () {
const prevNode = this.navigator.getPreviousSibling(
this.blocks.fieldWithOutput,
);
const outputConnection = this.blocks.fieldWithOutput.outputConnection;
assert.equal(prevNode, [...this.blocks.statementInput1.getFields()][1]);
});
test('fromNextToBlock', function () {
Expand Down Expand Up @@ -617,14 +651,12 @@ suite('Navigation', function () {
assert.isNull(prevNode);
});
test('fromFieldToInput', function () {
const outputBlock = this.workspace.newBlock('field_input');
const outputBlock = createRenderedBlock(this.workspace, 'field_input');
this.blocks.fieldAndInputs2.inputList[0].connection.connect(
outputBlock.outputConnection,
);

const field = this.blocks.fieldAndInputs2.inputList[1].fieldRow[0];
const inputConnection =
this.blocks.fieldAndInputs2.inputList[0].connection;
const prevNode = this.navigator.getPreviousSibling(field);
assert.equal(prevNode, outputBlock);
});
Expand Down Expand Up @@ -701,18 +733,13 @@ suite('Navigation', function () {
});

suite('In', function () {
setup(function () {
this.emptyWorkspace = Blockly.inject(document.createElement('div'), {});
});
teardown(function () {
workspaceTeardown.call(this, this.emptyWorkspace);
});

test('fromInputToOutput', function () {
// The first child is the connected block since the connection itself
// cannot be navigated to directly.
const input = this.blocks.statementInput1.inputList[0];
const inNode = this.navigator.getFirstChild(input.connection);
const outputConnection = this.blocks.fieldWithOutput.outputConnection;
assert.equal(inNode, outputConnection);
const connectedBlock = this.blocks.fieldWithOutput;
assert.equal(inNode, connectedBlock);
});
test('fromInputToNull', function () {
const input = this.blocks.statementInput2.inputList[0];
Expand Down
4 changes: 2 additions & 2 deletions tests/mocha/test_helpers/block_definitions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ export function createTestBlock() {
};
}

export function createRenderedBlock(workspaceSvg, type) {
const block = workspaceSvg.newBlock(type);
export function createRenderedBlock(workspaceSvg, type, opt_id) {
const block = workspaceSvg.newBlock(type, opt_id);
block.initSvg();
block.render();
return block;
Expand Down
Loading