fix: Fix bug that could cause renders to clobber the undo stack

packages/blockly/core/render_management.ts

@@ -15,8 +15,13 @@ const rootBlocks = new Set<BlockSvg>();
 /** The set of all blocks in need of rendering. */
 const dirtyBlocks = new WeakSet<BlockSvg>();
 
-/** A map from queued blocks to the event group from when they were queued. */
-const eventGroups = new WeakMap<BlockSvg, string>();
+/**
+ * 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);
   }

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'
+          );
+        }),
+      );
+    });
+  });
 });