From a38adb669741a9a2403aca675c7792838630822e Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Mon, 3 Aug 2026 15:11:28 -0700 Subject: [PATCH] fix: Fix bug that could cause renders to clobber the undo stack --- packages/blockly/core/render_management.ts | 25 +++++-- .../tests/mocha/render_management_test.js | 71 +++++++++++++++++++ 2 files changed, 90 insertions(+), 6 deletions(-) diff --git a/packages/blockly/core/render_management.ts b/packages/blockly/core/render_management.ts index e916ead8cfd..a9fadef3d0b 100644 --- a/packages/blockly/core/render_management.ts +++ b/packages/blockly/core/render_management.ts @@ -15,8 +15,13 @@ const rootBlocks = new Set(); /** The set of all blocks in need of rendering. */ const dirtyBlocks = new WeakSet(); -/** A map from queued blocks to the event group from when they were queued. */ -const eventGroups = new WeakMap(); +/** + * A map from queued blocks to the event context from when they were queued. + */ +const eventContexts = new WeakMap< + BlockSvg, + {group: string; recordUndo: boolean} +>(); /** * The promise which resolves after the current set of renders is completed. Or @@ -107,7 +112,10 @@ function alwaysImmediatelyRender() { */ function queueBlock(block: BlockSvg) { dirtyBlocks.add(block); - eventGroups.set(block, eventUtils.getGroup()); + eventContexts.set(block, { + group: eventUtils.getGroup(), + recordUndo: eventUtils.getRecordUndo(), + }); const parent = block.getParent(); if (parent) { queueBlock(parent); @@ -144,12 +152,17 @@ function doRenders(workspace?: WorkspaceSvg) { } for (const block of blocks) { const oldGroup = eventUtils.getGroup(); - const newGroup = eventGroups.get(block); - if (newGroup) eventUtils.setGroup(newGroup); + const oldRecordUndo = eventUtils.getRecordUndo(); + const context = eventContexts.get(block); + if (context) { + if (context.group) eventUtils.setGroup(context.group); + eventUtils.setRecordUndo(context.recordUndo); + } block.bumpNeighbours(); eventUtils.setGroup(oldGroup); + eventUtils.setRecordUndo(oldRecordUndo); } for (const block of blocks) { @@ -162,7 +175,7 @@ function doRenders(workspace?: WorkspaceSvg) { function dequeueBlock(block: BlockSvg) { rootBlocks.delete(block); dirtyBlocks.delete(block); - eventGroups.delete(block); + eventContexts.delete(block); for (const child of block.getChildren(false)) { dequeueBlock(child); } diff --git a/packages/blockly/tests/mocha/render_management_test.js b/packages/blockly/tests/mocha/render_management_test.js index 243a58132ec..2adcb252e6c 100644 --- a/packages/blockly/tests/mocha/render_management_test.js +++ b/packages/blockly/tests/mocha/render_management_test.js @@ -126,4 +126,75 @@ suite('Render Management', function () { assert.isFalse(block2.hasRendered, 'Expected block2 to not be rendered'); }); }); + + suite('Post-render bumpNeighbours', function () { + setup(function () { + this.workspace = Blockly.inject('blocklyDiv', {}); + + // Create and init two identical overlapping blocks so that the first + // render will need to bump one. + this.block = this.workspace.newBlock('controls_if'); + this.block.initSvg(); + + const block2 = this.workspace.newBlock('controls_if'); + block2.initSvg(); + }); + + test('does not record undo event when the render was queued with recordUndo disabled', function () { + Blockly.Events.setRecordUndo(false); + Blockly.renderManagement.queueRender(this.block); + Blockly.Events.setRecordUndo(true); + + Blockly.renderManagement.triggerQueuedRenders(); + + assert.isFalse( + this.workspace.getUndoStack().some((item) => { + return ( + item.type === 'move' && + item.reason[0] === 'bump' && + item.blockId === this.block.id + ); + }), + ); + }); + + test('records undo event when the render was queued with recordUndo enabled', function () { + Blockly.Events.setRecordUndo(true); + Blockly.renderManagement.queueRender(this.block); + Blockly.Events.setRecordUndo(false); + + Blockly.renderManagement.triggerQueuedRenders(); + + assert.isTrue( + this.workspace.getUndoStack().some((item) => { + return ( + item.type === 'move' && + item.reason[0] === 'bump' && + item.blockId === this.block.id + ); + }), + ); + }); + + test('associates bump undo events with the event group from when the render was queued', function () { + Blockly.Events.setRecordUndo(true); + Blockly.Events.setGroup('test-group'); + Blockly.renderManagement.queueRender(this.block); + Blockly.Events.setGroup(false); + Blockly.Events.setRecordUndo(false); + + Blockly.renderManagement.triggerQueuedRenders(); + + assert.isTrue( + this.workspace.getUndoStack().some((item) => { + return ( + item.type === 'move' && + item.reason[0] === 'bump' && + item.blockId === this.block.id && + item.group === 'test-group' + ); + }), + ); + }); + }); });