fix!: rendered meaning (#7747)

* fix: make rendered strictly for differentiating blocksvgs * chore: fix references to rendered * chore: fix tests * chore: delete TODO
2026-01-07 17:10:11 +01:00 · 2024-01-10 11:13:30 -08:00
parent f317068aeb
commit 43f6df92a3
8 changed files with 168 additions and 241 deletions
--- a/core/block.ts
+++ b/core/block.ts
@@ -202,7 +202,8 @@ export class Block implements IASTNodeLocation, IDeletable {
  /** Name of the type of hat. */
  hat?: string;

-  rendered: boolean | null = null;
+  /** Is this block a BlockSVG? */
+  readonly rendered: boolean = false;

  /**
   * String for block help, or function that returns a URL. Null for no help.
--- a/core/block_svg.ts
+++ b/core/block_svg.ts
@@ -117,7 +117,10 @@ export class BlockSvg
  style: BlockStyle;
  /** @internal */
  pathObject: IPathObject;
-  override rendered = false;
+
+  /** Is this block a BlockSVG? */
+  override readonly rendered = true;
+
  private visuallyDisabled = false;

  /**
@@ -650,12 +653,6 @@ export class BlockSvg
   * @internal
   */
  updateComponentLocations(blockOrigin: Coordinate) {
-    if (!this.rendered) {
-      // Rendering is required to lay out the blocks.
-      // This is probably an invisible block attached to a collapsed block.
-      return;
-    }
-
    if (!this.dragging) this.updateConnectionLocations(blockOrigin);
    this.updateIconLocations(blockOrigin);
    this.updateFieldLocations(blockOrigin);
@@ -789,7 +786,7 @@ export class BlockSvg
    Tooltip.dispose();
    ContextMenu.hide();

-    if (animate && this.rendered) {
+    if (animate) {
      this.unplug(healStack);
      blockAnimations.disposeUiEffect(this);
    }
@@ -806,8 +803,6 @@ export class BlockSvg
    if (this.isDeadOrDying()) return;
    super.disposeInternal();

-    this.rendered = false;
-
    if (common.getSelected() === this) {
      this.unselect();
      this.workspace.cancelCurrentGesture();
@@ -991,13 +986,11 @@ export class BlockSvg

    if (icon instanceof MutatorIcon) this.mutator = icon;

-    if (this.rendered) {
-      icon.initView(this.createIconPointerDownListener(icon));
-      icon.applyColour();
-      icon.updateEditable();
-      this.queueRender();
-      this.bumpNeighbours();
-    }
+    icon.initView(this.createIconPointerDownListener(icon));
+    icon.applyColour();
+    icon.updateEditable();
+    this.queueRender();
+    this.bumpNeighbours();

    return icon;
  }
@@ -1021,10 +1014,9 @@ export class BlockSvg

    if (type.equals(MutatorIcon.TYPE)) this.mutator = null;

-    if (this.rendered) {
-      this.queueRender();
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
+
    return removed;
  }

@@ -1036,7 +1028,7 @@ export class BlockSvg
  override setEnabled(enabled: boolean) {
    if (this.isEnabled() !== enabled) {
      super.setEnabled(enabled);
-      if (this.rendered && !this.getInheritedDisabled()) {
+      if (!this.getInheritedDisabled()) {
        this.updateDisabled();
      }
    }
@@ -1049,9 +1041,6 @@ export class BlockSvg
   * @param highlighted True if highlighted.
   */
  setHighlighted(highlighted: boolean) {
-    if (!this.rendered) {
-      return;
-    }
    this.pathObject.updateHighlighted(highlighted);
  }

@@ -1183,11 +1172,8 @@ export class BlockSvg
    opt_check?: string | string[] | null,
  ) {
    super.setPreviousStatement(newBoolean, opt_check);
-
-    if (this.rendered) {
-      this.queueRender();
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
  }

  /**
@@ -1202,11 +1188,8 @@ export class BlockSvg
    opt_check?: string | string[] | null,
  ) {
    super.setNextStatement(newBoolean, opt_check);
-
-    if (this.rendered) {
-      this.queueRender();
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
  }

  /**
@@ -1221,11 +1204,8 @@ export class BlockSvg
    opt_check?: string | string[] | null,
  ) {
    super.setOutput(newBoolean, opt_check);
-
-    if (this.rendered) {
-      this.queueRender();
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
  }

  /**
@@ -1235,11 +1215,8 @@ export class BlockSvg
   */
  override setInputsInline(newBoolean: boolean) {
    super.setInputsInline(newBoolean);
-
-    if (this.rendered) {
-      this.queueRender();
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
  }

  /**
@@ -1253,13 +1230,8 @@ export class BlockSvg
   */
  override removeInput(name: string, opt_quiet?: boolean): boolean {
    const removed = super.removeInput(name, opt_quiet);
-
-    if (this.rendered) {
-      this.queueRender();
-      // Removing an input will cause the block to change shape.
-      this.bumpNeighbours();
-    }
-
+    this.queueRender();
+    this.bumpNeighbours();
    return removed;
  }

@@ -1271,23 +1243,15 @@ export class BlockSvg
   */
  override moveNumberedInputBefore(inputIndex: number, refIndex: number) {
    super.moveNumberedInputBefore(inputIndex, refIndex);
-
-    if (this.rendered) {
-      this.queueRender();
-      // Moving an input will cause the block to change shape.
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
  }

  /** @override */
  override appendInput(input: Input): Input {
    super.appendInput(input);
-
-    if (this.rendered) {
-      this.queueRender();
-      // Adding an input will cause the block to change shape.
-      this.bumpNeighbours();
-    }
+    this.queueRender();
+    this.bumpNeighbours();
    return input;
  }

@@ -1341,28 +1305,25 @@ export class BlockSvg
   * Returns connections originating from this block.
   *
   * @param all If true, return all connections even hidden ones.
-   *     Otherwise, for a non-rendered block return an empty list, and for a
-   *     collapsed block don't return inputs connections.
+   *     Otherwise, for a collapsed block don't return inputs connections.
   * @returns Array of connections.
   * @internal
   */
  override getConnections_(all: boolean): RenderedConnection[] {
    const myConnections = [];
-    if (all || this.rendered) {
-      if (this.outputConnection) {
-        myConnections.push(this.outputConnection);
-      }
-      if (this.previousConnection) {
-        myConnections.push(this.previousConnection);
-      }
-      if (this.nextConnection) {
-        myConnections.push(this.nextConnection);
-      }
-      if (all || !this.collapsed_) {
-        for (let i = 0, input; (input = this.inputList[i]); i++) {
-          if (input.connection) {
-            myConnections.push(input.connection as RenderedConnection);
-          }
+    if (this.outputConnection) {
+      myConnections.push(this.outputConnection);
+    }
+    if (this.previousConnection) {
+      myConnections.push(this.previousConnection);
+    }
+    if (this.nextConnection) {
+      myConnections.push(this.nextConnection);
+    }
+    if (all || !this.collapsed_) {
+      for (let i = 0, input; (input = this.inputList[i]); i++) {
+        if (input.connection) {
+          myConnections.push(input.connection as RenderedConnection);
        }
      }
    }
@@ -1573,7 +1534,6 @@ export class BlockSvg
   * @internal
   */
  renderEfficiently() {
-    this.rendered = true;
    dom.startTextWidthCache();

    if (this.isCollapsed()) {
--- a/core/inputs/input.ts
+++ b/core/inputs/input.ts
@@ -124,7 +124,6 @@ export class Input {

    if (this.sourceBlock.rendered) {
      (this.sourceBlock as BlockSvg).queueRender();
-      // Adding a field will cause the block to change shape.
      this.sourceBlock.bumpNeighbours();
    }
    return index;
@@ -146,7 +145,6 @@ export class Input {
        this.fieldRow.splice(i, 1);
        if (this.sourceBlock.rendered) {
          (this.sourceBlock as BlockSvg).queueRender();
-          // Removing a field will cause the block to change shape.
          this.sourceBlock.bumpNeighbours();
        }
        return true;
@@ -274,7 +272,7 @@ export class Input {

  /** Initialize the fields on this input. */
  init() {
-    if (!this.sourceBlock.workspace.rendered) {
+    if (!this.sourceBlock.rendered) {
      return; // Headless blocks don't need fields initialized.
    }
    for (let i = 0; i < this.fieldRow.length; i++) {
--- a/core/insertion_marker_manager.ts
+++ b/core/insertion_marker_manager.ts
@@ -176,18 +176,16 @@ export class InsertionMarkerManager {
    eventUtils.enable();
    const {local, closest} = this.activeCandidate;
    local.connect(closest);
-    if (this.topBlock.rendered) {
-      const inferiorConnection = local.isSuperior() ? closest : local;
-      const rootBlock = this.topBlock.getRootBlock();
+    const inferiorConnection = local.isSuperior() ? closest : local;
+    const rootBlock = this.topBlock.getRootBlock();

-      finishQueuedRenders().then(() => {
-        blockAnimations.connectionUiEffect(inferiorConnection.getSourceBlock());
-        // bringToFront is incredibly expensive. Delay until the next frame.
-        setTimeout(() => {
-          rootBlock.bringToFront();
-        }, 0);
-      });
-    }
+    finishQueuedRenders().then(() => {
+      blockAnimations.connectionUiEffect(inferiorConnection.getSourceBlock());
+      // bringToFront is incredibly expensive. Delay until the next frame.
+      setTimeout(() => {
+        rootBlock.bringToFront();
+      }, 0);
+    });
  }

  /**
--- a/core/rendered_connection.ts
+++ b/core/rendered_connection.ts
@@ -502,16 +502,12 @@ export class RenderedConnection extends Connection {
    const parent = parentConnection.getSourceBlock() as BlockSvg;
    const child = childConnection.getSourceBlock() as BlockSvg;
    super.disconnectInternal(setParent);
-    // Rerender the parent so that it may reflow.
-    if (parent.rendered) {
-      parent.queueRender();
-    }
-    if (child.rendered) {
-      child.updateDisabled();
-      child.queueRender();
-      // Reset visibility, since the child is now a top block.
-      child.getSvgRoot().style.display = 'block';
-    }
+
+    parent.queueRender();
+    child.updateDisabled();
+    child.queueRender();
+    // Reset visibility, since the child is now a top block.
+    child.getSvgRoot().style.display = 'block';
  }

  /**
@@ -554,29 +550,10 @@ export class RenderedConnection extends Connection {

    const parentBlock = this.getSourceBlock();
    const childBlock = renderedChildConnection.getSourceBlock();
-    const parentRendered = parentBlock.rendered;
-    const childRendered = childBlock.rendered;

-    if (parentRendered) {
-      parentBlock.updateDisabled();
-    }
-    if (childRendered) {
-      childBlock.updateDisabled();
-    }
-    if (parentRendered && childRendered) {
-      if (
-        this.type === ConnectionType.NEXT_STATEMENT ||
-        this.type === ConnectionType.PREVIOUS_STATEMENT
-      ) {
-        // Child block may need to square off its corners if it is in a stack.
-        // Rendering a child will render its parent.
-        childBlock.queueRender();
-      } else {
-        // Child block does not change shape.  Rendering the parent node will
-        // move its connected children into position.
-        parentBlock.queueRender();
-      }
-    }
+    parentBlock.updateDisabled();
+    childBlock.updateDisabled();
+    childBlock.queueRender();

    // The input the child block is connected to (if any).
    const parentInput = parentBlock.getInputWithBlock(childBlock);
@@ -617,9 +594,7 @@ export class RenderedConnection extends Connection {
   */
  override setCheck(check: string | string[] | null): RenderedConnection {
    super.setCheck(check);
-    if (this.sourceBlock_.rendered) {
-      this.sourceBlock_.queueRender();
-    }
+    this.sourceBlock_.queueRender();
    return this;
  }
 }
--- a/tests/mocha/icon_test.js
+++ b/tests/mocha/icon_test.js
@@ -68,24 +68,6 @@ suite('Icon', function () {
        );
      });

-      test('initView is called by headful blocks during initSvg', function () {
-        const workspace = createWorkspaceSvg();
-        const block = createUninitializedBlock(workspace);
-        const icon = new MockIcon();
-        const initViewSpy = sinon.spy(icon, 'initView');
-
-        block.addIcon(icon);
-        chai.assert.isFalse(
-          initViewSpy.called,
-          'Expected initView to not be called before initing svg',
-        );
-        block.initSvg();
-        chai.assert.isTrue(
-          initViewSpy.calledOnce,
-          'Expected initView to be called',
-        );
-      });
-
      test(
        'initView is called by headful blocks that are currently ' +
          'rendered when the icon is added',
@@ -120,24 +102,6 @@ suite('Icon', function () {
        );
      });

-      test('applyColour is called by headful blocks during initSvg', function () {
-        const workspace = createWorkspaceSvg();
-        const block = createUninitializedBlock(workspace);
-        const icon = new MockIcon();
-        const applyColourSpy = sinon.spy(icon, 'applyColour');
-
-        block.addIcon(icon);
-        chai.assert.isFalse(
-          applyColourSpy.called,
-          'Expected applyCOlour to not be called before initing svg',
-        );
-        block.initSvg();
-        chai.assert.isTrue(
-          applyColourSpy.calledOnce,
-          'Expected applyColour to be called',
-        );
-      });
-
      test(
        'applyColour is called by headful blocks that are currently ' +
          'rendered when the icon is added',
@@ -231,24 +195,6 @@ suite('Icon', function () {
        );
      });

-      test('updateEditable is called by headful blocks during initSvg', function () {
-        const workspace = createWorkspaceSvg();
-        const block = createUninitializedBlock(workspace);
-        const icon = new MockIcon();
-        const updateEditableSpy = sinon.spy(icon, 'updateEditable');
-
-        block.addIcon(icon);
-        chai.assert.isFalse(
-          updateEditableSpy.called,
-          'Expected updateEditable to not be called before initing svg',
-        );
-        block.initSvg();
-        chai.assert.isTrue(
-          updateEditableSpy.calledOnce,
-          'Expected updateEditable to be called',
-        );
-      });
-
      test(
        'updateEditable is called by headful blocks that are currently ' +
          'rendered when the icon is added',
--- a/tests/mocha/test_helpers/workspace.js
+++ b/tests/mocha/test_helpers/workspace.js
@@ -781,7 +781,9 @@ export function testAWorkspace() {
        const xml = Blockly.utils.xml.textToDom(xmlText);
        Blockly.Xml.domToBlock(xml, this.workspace);
        this.workspace.getTopBlocks()[0].dispose(false);
+        this.clock.runAll();
        this.workspace.undo();
+        this.clock.runAll();
        const newXml = Blockly.Xml.workspaceToDom(this.workspace);
        assertNodesEqual(newXml.firstChild, xml);
      }
@@ -909,11 +911,14 @@ export function testAWorkspace() {
      function testUndoConnect(xmlText, parentId, childId, func) {
        const xml = Blockly.utils.xml.textToDom(xmlText);
        Blockly.Xml.domToWorkspace(xml, this.workspace);
+        this.clock.runAll();

        const parent = this.workspace.getBlockById(parentId);
        const child = this.workspace.getBlockById(childId);
        func.call(this, parent, child);
+        this.clock.runAll();
        this.workspace.undo();
+        this.clock.runAll();

        const newXml = Blockly.Xml.workspaceToDom(this.workspace);
        assertNodesEqual(newXml, xml);
@@ -922,8 +927,8 @@ export function testAWorkspace() {
      test('Stack', function () {
        const xml =
          '<xml>' +
-          '  <block type="stack_block" id="1"></block>' +
-          '  <block type="stack_block" id="2"></block>' +
+          '  <block type="stack_block" id="1" x="10" y="10"></block>' +
+          '  <block type="stack_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -934,8 +939,8 @@ export function testAWorkspace() {
      test('Row', function () {
        const xml =
          '<xml>' +
-          '  <block type="row_block" id="1"></block>' +
-          '  <block type="row_block" id="2"></block>' +
+          '  <block type="row_block" id="1" x="10" y="10"></block>' +
+          '  <block type="row_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -946,8 +951,8 @@ export function testAWorkspace() {
      test('Statement', function () {
        const xml =
          '<xml>' +
-          '  <block type="statement_block" id="1"></block>' +
-          '  <block type="stack_block" id="2"></block>' +
+          '  <block type="statement_block" id="1" x="10" y="10"></block>' +
+          '  <block type="stack_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -960,12 +965,12 @@ export function testAWorkspace() {
      test('Stack w/ child', function () {
        const xml =
          '<xml>' +
-          '  <block type="stack_block" id="1">' +
+          '  <block type="stack_block" id="1" x="10" y="10">' +
          '    <next>' +
          '      <block type="stack_block" id="3"></block>' +
          '    </next>' +
          '  </block>' +
-          '  <block type="stack_block" id="2"></block>' +
+          '  <block type="stack_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -976,12 +981,12 @@ export function testAWorkspace() {
      test('Row w/ child', function () {
        const xml =
          '<xml>' +
-          '  <block type="row_block" id="1">' +
+          '  <block type="row_block" id="1" x="10" y="10">' +
          '    <value name="INPUT">' +
          '      <block type="row_block" id="3"></block>' +
          '    </value>' +
          '  </block>' +
-          '  <block type="row_block" id="2"></block>' +
+          '  <block type="row_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -992,12 +997,12 @@ export function testAWorkspace() {
      test('Statement w/ child', function () {
        const xml =
          '<xml>' +
-          '  <block type="statement_block" id="1">' +
+          '  <block type="statement_block" id="1" x="10" y="10">' +
          '    <statement name="STATEMENT">' +
          '      <block type="stack_block" id="3"></block>' +
          '    </statement>' +
          '  </block>' +
-          '  <block type="stack_block" id="2"></block>' +
+          '  <block type="stack_block" id="2" x="100" y="100"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -1010,12 +1015,12 @@ export function testAWorkspace() {
      test('Stack w/ shadow', function () {
        const xml =
          '<xml>' +
-          '  <block type="stack_block" id="1">' +
+          '  <block type="stack_block" id="1" x="10" y="10">' +
          '    <next>' +
          '      <shadow type="stack_block" id="3"></shadow>' +
          '    </next>' +
          '  </block>' +
-          '  <block type="stack_block" id="2"></block>' +
+          '  <block type="stack_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -1026,12 +1031,12 @@ export function testAWorkspace() {
      test('Row w/ shadow', function () {
        const xml =
          '<xml>' +
-          '  <block type="row_block" id="1">' +
+          '  <block type="row_block" id="1" x="10" y="10">' +
          '    <value name="INPUT">' +
          '      <shadow type="row_block" id="3"></shadow>' +
          '    </value>' +
          '  </block>' +
-          '  <block type="row_block" id="2"></block>' +
+          '  <block type="row_block" id="2" x="50" y="50"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -1042,12 +1047,12 @@ export function testAWorkspace() {
      test('Statement w/ shadow', function () {
        const xml =
          '<xml>' +
-          '  <block type="statement_block" id="1">' +
+          '  <block type="statement_block" id="1" x="10" y="10">' +
          '    <statement name="STATEMENT">' +
          '      <shadow type="stack_block" id="3"></shadow>' +
          '    </statement>' +
          '  </block>' +
-          '  <block type="stack_block" id="2"></block>' +
+          '  <block type="stack_block" id="2" x="100" y="100"></block>' +
          '</xml>';

        testUndoConnect.call(this, xml, '1', '2', (parent, child) => {
@@ -1102,6 +1107,7 @@ export function testAWorkspace() {
      function testUndoDisconnect(xmlText, childId) {
        const xml = Blockly.utils.xml.textToDom(xmlText);
        Blockly.Xml.domToWorkspace(xml, this.workspace);
+        this.clock.runAll();

        const child = this.workspace.getBlockById(childId);
        if (child.outputConnection) {
@@ -1109,7 +1115,9 @@ export function testAWorkspace() {
        } else {
          child.previousConnection.disconnect();
        }
+        this.clock.runAll();
        this.workspace.undo();
+        this.clock.runAll();

        const newXml = Blockly.Xml.workspaceToDom(this.workspace);
        assertNodesEqual(newXml, xml);
@@ -1262,8 +1270,10 @@ export function testAWorkspace() {
      suite('createVariable', function () {
        test('Undo only', function () {
          createTwoVarsDifferentTypes(this.workspace);
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
          chai.assert.isNull(this.workspace.getVariableById('id2'));

@@ -1274,8 +1284,10 @@ export function testAWorkspace() {

        test('Undo and redo', function () {
          createTwoVarsDifferentTypes(this.workspace);
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
          chai.assert.isNull(this.workspace.getVariableById('id2'));

@@ -1300,14 +1312,18 @@ export function testAWorkspace() {
      suite('deleteVariableById', function () {
        test('Undo only no usages', function () {
          createTwoVarsDifferentTypes(this.workspace);
+          this.clock.runAll();
          this.workspace.deleteVariableById('id1');
          this.workspace.deleteVariableById('id2');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
        });
@@ -1316,15 +1332,19 @@ export function testAWorkspace() {
          createTwoVarsDifferentTypes(this.workspace);
          // Create blocks to refer to both of them.
          createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
+          this.clock.runAll();
          this.workspace.deleteVariableById('id1');
          this.workspace.deleteVariableById('id2');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name2');
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name2');
          assertBlockVarModelName(this.workspace, 1, 'name1');
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
@@ -1333,24 +1353,31 @@ export function testAWorkspace() {

        test('Reference exists no usages', function () {
          createTwoVarsDifferentTypes(this.workspace);
+          this.clock.runAll();
          this.workspace.deleteVariableById('id1');
          this.workspace.deleteVariableById('id2');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

          this.workspace.undo(true);
+          this.clock.runAll();
          // Expect that both variables are deleted
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          chai.assert.isNull(this.workspace.getVariableById('id2'));

          this.workspace.undo();
+          this.clock.runAll();
          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

          this.workspace.undo(true);
+          this.clock.runAll();
          // Expect that variable 'id2' is recreated
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
@@ -1360,28 +1387,35 @@ export function testAWorkspace() {
          createTwoVarsDifferentTypes(this.workspace);
          // Create blocks to refer to both of them.
          createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
+          this.clock.runAll();
          this.workspace.deleteVariableById('id1');
          this.workspace.deleteVariableById('id2');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name2');
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

          this.workspace.undo(true);
+          this.clock.runAll();
          // Expect that both variables are deleted
          chai.assert.equal(this.workspace.getTopBlocks(false).length, 0);
          chai.assert.isNull(this.workspace.getVariableById('id1'));
          chai.assert.isNull(this.workspace.getVariableById('id2'));

          this.workspace.undo();
+          this.clock.runAll();
          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name2');
          assertBlockVarModelName(this.workspace, 1, 'name1');
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
          assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

          this.workspace.undo(true);
+          this.clock.runAll();
          // Expect that variable 'id2' is recreated
          assertBlockVarModelName(this.workspace, 0, 'name2');
          chai.assert.isNull(this.workspace.getVariableById('id1'));
@@ -1391,6 +1425,7 @@ export function testAWorkspace() {
        test('Delete same variable twice no usages', function () {
          this.workspace.createVariable('name1', 'type1', 'id1');
          this.workspace.deleteVariableById('id1');
+          this.clock.runAll();
          const workspace = this.workspace;
          assertWarnings(() => {
            workspace.deleteVariableById('id1');
@@ -1406,21 +1441,26 @@ export function testAWorkspace() {

          // Undo delete
          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');

          // Redo delete
          this.workspace.undo(true);
+          this.clock.runAll();
          chai.assert.isNull(this.workspace.getVariableById('id1'));

          // Redo delete, nothing should happen
          this.workspace.undo(true);
+          this.clock.runAll();
          chai.assert.isNull(this.workspace.getVariableById('id1'));
        });

        test('Delete same variable twice with usages', function () {
          this.workspace.createVariable('name1', 'type1', 'id1');
          createVarBlocksNoEvents(this.workspace, ['id1']);
+          this.clock.runAll();
          this.workspace.deleteVariableById('id1');
+          this.clock.runAll();
          const workspace = this.workspace;
          assertWarnings(() => {
            workspace.deleteVariableById('id1');
@@ -1437,16 +1477,19 @@ export function testAWorkspace() {

          // Undo delete
          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name1');
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');

          // Redo delete
          this.workspace.undo(true);
+          this.clock.runAll();
          chai.assert.equal(this.workspace.getTopBlocks(false).length, 0);
          chai.assert.isNull(this.workspace.getVariableById('id1'));

          // Redo delete, nothing should happen
          this.workspace.undo(true);
+          this.clock.runAll();
          chai.assert.equal(this.workspace.getTopBlocks(false).length, 0);
          chai.assert.isNull(this.workspace.getVariableById('id1'));
        });
@@ -1459,46 +1502,58 @@ export function testAWorkspace() {

        test('Reference exists no usages rename to name2', function () {
          this.workspace.renameVariableById('id1', 'name2');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');

          this.workspace.undo(true);
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name2', 'type1', 'id1');
        });

        test('Reference exists with usages rename to name2', function () {
          createVarBlocksNoEvents(this.workspace, ['id1']);
          this.workspace.renameVariableById('id1', 'name2');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name1');
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');

          this.workspace.undo(true);
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name2');
          assertVariableValues(this.workspace, 'name2', 'type1', 'id1');
        });

        test('Reference exists different capitalization no usages rename to Name1', function () {
          this.workspace.renameVariableById('id1', 'Name1');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');

          this.workspace.undo(true);
+          this.clock.runAll();
          assertVariableValues(this.workspace, 'Name1', 'type1', 'id1');
        });

        test('Reference exists different capitalization with usages rename to Name1', function () {
          createVarBlocksNoEvents(this.workspace, ['id1']);
          this.workspace.renameVariableById('id1', 'Name1');
+          this.clock.runAll();

          this.workspace.undo();
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'name1');
          assertVariableValues(this.workspace, 'name1', 'type1', 'id1');

          this.workspace.undo(true);
+          this.clock.runAll();
          assertBlockVarModelName(this.workspace, 0, 'Name1');
          assertVariableValues(this.workspace, 'Name1', 'type1', 'id1');
        });
@@ -1507,12 +1562,15 @@ export function testAWorkspace() {
          test('Same type no usages rename variable with id1 to name2', function () {
            this.workspace.createVariable('name2', 'type1', 'id2');
            this.workspace.renameVariableById('id1', 'name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type1', 'id2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name2', 'type1', 'id2');
            chai.assert.isNull(this.workspace.getVariableById('id1'));
          });
@@ -1521,14 +1579,17 @@ export function testAWorkspace() {
            this.workspace.createVariable('name2', 'type1', 'id2');
            createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
            this.workspace.renameVariableById('id1', 'name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertBlockVarModelName(this.workspace, 0, 'name1');
            assertBlockVarModelName(this.workspace, 1, 'name2');
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type1', 'id2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name2', 'type1', 'id2');
            chai.assert.isNull(this.workspace.getVariableById('id1'));
          });
@@ -1536,12 +1597,15 @@ export function testAWorkspace() {
          test('Same type different capitalization no usages rename variable with id1 to Name2', function () {
            this.workspace.createVariable('name2', 'type1', 'id2');
            this.workspace.renameVariableById('id1', 'Name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type1', 'id2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'Name2', 'type1', 'id2');
            chai.assert.isNull(this.workspace.getVariable('name1'));
          });
@@ -1550,14 +1614,17 @@ export function testAWorkspace() {
            this.workspace.createVariable('name2', 'type1', 'id2');
            createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
            this.workspace.renameVariableById('id1', 'Name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertBlockVarModelName(this.workspace, 0, 'name1');
            assertBlockVarModelName(this.workspace, 1, 'name2');
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type1', 'id2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'Name2', 'type1', 'id2');
            chai.assert.isNull(this.workspace.getVariableById('id1'));
            assertBlockVarModelName(this.workspace, 0, 'Name2');
@@ -1567,12 +1634,15 @@ export function testAWorkspace() {
          test('Different type no usages rename variable with id1 to name2', function () {
            this.workspace.createVariable('name2', 'type2', 'id2');
            this.workspace.renameVariableById('id1', 'name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name2', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
          });
@@ -1581,14 +1651,17 @@ export function testAWorkspace() {
            this.workspace.createVariable('name2', 'type2', 'id2');
            createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
            this.workspace.renameVariableById('id1', 'name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
            assertBlockVarModelName(this.workspace, 0, 'name1');
            assertBlockVarModelName(this.workspace, 1, 'name2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name2', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
            assertBlockVarModelName(this.workspace, 0, 'name2');
@@ -1598,12 +1671,15 @@ export function testAWorkspace() {
          test('Different type different capitalization no usages rename variable with id1 to Name2', function () {
            this.workspace.createVariable('name2', 'type2', 'id2');
            this.workspace.renameVariableById('id1', 'Name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'Name2', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
          });
@@ -1612,14 +1688,17 @@ export function testAWorkspace() {
            this.workspace.createVariable('name2', 'type2', 'id2');
            createVarBlocksNoEvents(this.workspace, ['id1', 'id2']);
            this.workspace.renameVariableById('id1', 'Name2');
+            this.clock.runAll();

            this.workspace.undo();
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'name1', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
            assertBlockVarModelName(this.workspace, 0, 'name1');
            assertBlockVarModelName(this.workspace, 1, 'name2');

            this.workspace.undo(true);
+            this.clock.runAll();
            assertVariableValues(this.workspace, 'Name2', 'type1', 'id1');
            assertVariableValues(this.workspace, 'name2', 'type2', 'id2');
            assertBlockVarModelName(this.workspace, 0, 'Name2');
--- a/tests/mocha/workspace_svg_test.js
+++ b/tests/mocha/workspace_svg_test.js
@@ -21,7 +21,7 @@ import {testAWorkspace} from './test_helpers/workspace.js';

 suite('WorkspaceSvg', function () {
  setup(function () {
-    sharedTestSetup.call(this);
+    this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock;
    const toolbox = document.getElementById('toolbox-categories');
    this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox});
    Blockly.defineBlocksWithJsonArray([
@@ -168,8 +168,7 @@ suite('WorkspaceSvg', function () {
  });

  suite('Viewport change events', function () {
-    function resetEventHistory(eventsFireStub, changeListenerSpy) {
-      eventsFireStub.resetHistory();
+    function resetEventHistory(changeListenerSpy) {
      changeListenerSpy.resetHistory();
    }
    function assertSpyFiredViewportEvent(spy, workspace, expectedProperties) {
@@ -187,7 +186,6 @@ suite('WorkspaceSvg', function () {
      );
    }
    function assertViewportEventFired(
-      eventsFireStub,
      changeListenerSpy,
      workspace,
      expectedEventCount = 1,
@@ -200,32 +198,25 @@ suite('WorkspaceSvg', function () {
        viewLeft: metrics.viewLeft,
        type: eventUtils.VIEWPORT_CHANGE,
      };
-      assertSpyFiredViewportEvent(
-        eventsFireStub,
-        workspace,
-        expectedProperties,
-      );
      assertSpyFiredViewportEvent(
        changeListenerSpy,
        workspace,
        expectedProperties,
      );
      sinon.assert.callCount(changeListenerSpy, expectedEventCount);
-      sinon.assert.callCount(eventsFireStub, expectedEventCount);
    }
    function runViewportEventTest(
      eventTriggerFunc,
-      eventsFireStub,
      changeListenerSpy,
      workspace,
      clock,
      expectedEventCount = 1,
    ) {
      clock.runAll();
-      resetEventHistory(eventsFireStub, changeListenerSpy);
+      resetEventHistory(changeListenerSpy);
      eventTriggerFunc();
+      clock.runAll();
      assertViewportEventFired(
-        eventsFireStub,
        changeListenerSpy,
        workspace,
        expectedEventCount,
@@ -243,7 +234,6 @@ suite('WorkspaceSvg', function () {
      test('setScale', function () {
        runViewportEventTest(
          () => this.workspace.setScale(2),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -252,7 +242,6 @@ suite('WorkspaceSvg', function () {
      test('zoom(50, 50, 1)', function () {
        runViewportEventTest(
          () => this.workspace.zoom(50, 50, 1),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -261,7 +250,6 @@ suite('WorkspaceSvg', function () {
      test('zoom(50, 50, -1)', function () {
        runViewportEventTest(
          () => this.workspace.zoom(50, 50, -1),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -270,7 +258,6 @@ suite('WorkspaceSvg', function () {
      test('zoomCenter(1)', function () {
        runViewportEventTest(
          () => this.workspace.zoomCenter(1),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -279,7 +266,6 @@ suite('WorkspaceSvg', function () {
      test('zoomCenter(-1)', function () {
        runViewportEventTest(
          () => this.workspace.zoomCenter(-1),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -291,7 +277,6 @@ suite('WorkspaceSvg', function () {
        block.render();
        runViewportEventTest(
          () => this.workspace.zoomToFit(),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -305,7 +290,6 @@ suite('WorkspaceSvg', function () {
        block.render();
        runViewportEventTest(
          () => this.workspace.centerOnBlock(block.id),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -314,7 +298,6 @@ suite('WorkspaceSvg', function () {
      test('scroll', function () {
        runViewportEventTest(
          () => this.workspace.scroll(50, 50),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -323,7 +306,6 @@ suite('WorkspaceSvg', function () {
      test('scrollCenter', function () {
        runViewportEventTest(
          () => this.workspace.scrollCenter(),
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -336,13 +318,12 @@ suite('WorkspaceSvg', function () {
        block.initSvg();
        block.render();
        this.clock.runAll();
-        resetEventHistory(this.eventsFireStub, this.changeListenerSpy);
+        resetEventHistory(this.changeListenerSpy);
        // Expect 2 events, 1 move, 1 viewport
        runViewportEventTest(
          () => {
            block.moveBy(1000, 1000);
          },
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,
@@ -366,7 +347,7 @@ suite('WorkspaceSvg', function () {
          '<block type="controls_if" x="188" y="163"></block>',
        );
        this.clock.runAll();
-        resetEventHistory(this.eventsFireStub, this.changeListenerSpy);
+        resetEventHistory(this.changeListenerSpy);
        // Add block in center of other blocks, not triggering scroll.
        Blockly.Xml.domToWorkspace(
          Blockly.utils.xml.textToDom(
@@ -375,11 +356,6 @@ suite('WorkspaceSvg', function () {
          this.workspace,
        );
        this.clock.runAll();
-        assertEventNotFired(
-          this.eventsFireStub,
-          Blockly.Events.ViewportChange,
-          {type: eventUtils.VIEWPORT_CHANGE},
-        );
        assertEventNotFired(
          this.changeListenerSpy,
          Blockly.Events.ViewportChange,
@@ -403,15 +379,10 @@ suite('WorkspaceSvg', function () {
          '<block type="controls_if" x="0" y="0"></block>',
        );
        this.clock.runAll();
-        resetEventHistory(this.eventsFireStub, this.changeListenerSpy);
+        resetEventHistory(this.changeListenerSpy);
        // Add block in center of other blocks, not triggering scroll.
        Blockly.Xml.domToWorkspace(xmlDom, this.workspace);
        this.clock.runAll();
-        assertEventNotFired(
-          this.eventsFireStub,
-          Blockly.Events.ViewportChange,
-          {type: eventUtils.VIEWPORT_CHANGE},
-        );
        assertEventNotFired(
          this.changeListenerSpy,
          Blockly.Events.ViewportChange,
@@ -436,7 +407,6 @@ suite('WorkspaceSvg', function () {
        // Expect 10 events, 4 create, 4 move, 1 viewport, 1 finished loading
        runViewportEventTest(
          addingMultipleBlocks,
-          this.eventsFireStub,
          this.changeListenerSpy,
          this.workspace,
          this.clock,