fix: redo disconnect from shadow bug

core/connection.js

@@ -760,22 +760,25 @@ Connection.prototype.createShadowBlock_ = function(attemptToConnect) {
     blockShadow = blocks.loadInternal(
         shadowState,
         parentBlock.workspace,
-        attemptToConnect ? this : undefined,
-        true);
+        {
+          parentConnection: attemptToConnect ? this : undefined,
+          isShadow: true,
+          recordUndo: false,
+        });
     return blockShadow;
   }
   
   if (shadowDom) {
     blockShadow = Xml.domToBlock(shadowDom, parentBlock.workspace);
     if (attemptToConnect) {
-      if (this.type == Blockly.connectionTypes.INPUT_VALUE) {
+      if (this.type == connectionTypes.INPUT_VALUE) {
         if (!blockShadow.outputConnection) {
           throw new Error('Shadow block is missing an output connection');
         }
         if (!this.connect(blockShadow.outputConnection)) {
           throw new Error('Could not connect shadow block to connection');
         }
-      } else if (this.type == Blockly.connectionTypes.NEXT_STATEMENT) {
+      } else if (this.type == connectionTypes.NEXT_STATEMENT) {
         if (!blockShadow.previousConnection) {
           throw new Error('Shadow block is missing previous connection');
         }

core/serialization/blocks.js

@@ -259,7 +259,7 @@ const saveConnection = function(connection) {
   if (!shadow && !child) {
     return null;
   }
-  var state = Object.create(null);
+  const state = Object.create(null);
   if (shadow) {
     state['shadow'] = shadow;
   }
@@ -279,21 +279,49 @@ const saveConnection = function(connection) {
  * @return {!Block} The block that was just loaded.
  */
 const load = function(state, workspace, {recordUndo = false} = {}) {
+  return loadInternal(state, workspace, {recordUndo});
+};
+exports.load = load;
+
+/**
+ * Loads the block represented by the given state into the given workspace.
+ * This is defined internally so that the extra parameters don't clutter our
+ * external API.
+ * But it is exported so that other places within Blockly can call it directly
+ * with the extra paramters.
+ * @param {!State} state The state of a block to deserialize into the workspace.
+ * @param {!Workspace} workspace The workspace to add the block to.
+ * @param {{parentConnection: (!Connection|undefined), isShadow:
+ *     (boolean|undefined), recordUndo: (boolean|undefined)}=} param1
+ *     parentConnection: If provided, the system will attempt to connect the
+ *       block to this connection after it is created. Undefined by default.
+ *     isShadow: The block will be set to a shadow block after it is created.
+ *       False by default.
+ *     recordUndo: If true, events triggered by this function will be undo-able
+ *       by the user. False by default.
+ * @return {!Block} The block that was just loaded.
+ */
+const loadInternal = function(
+    state,
+    workspace,
+    {
+      parentConnection = undefined,
+      isShadow = false,
+      recordUndo = false
+    } = {}
+) {
   const prevRecordUndo = Events.getRecordUndo();
   Events.setRecordUndo(recordUndo);
   const existingGroup = Events.getGroup();
   if (!existingGroup) {
     Events.setGroup(true);
   }
-
-  // We only want to fire an event for the top block.
   Events.disable();
 
-  const block = loadInternal(state, workspace);
+  const block = loadPrivate(state, workspace, {parentConnection, isShadow});
 
   Events.enable();
   Events.fire(new (Events.get(Events.BLOCK_CREATE))(block));
-
   Events.setGroup(existingGroup);
   Events.setRecordUndo(prevRecordUndo);
   
@@ -309,22 +337,32 @@ const load = function(state, workspace, {recordUndo = false} = {}) {
 
   return block;
 };
-exports.load = load;
+/** @package */
+exports.loadInternal = loadInternal;
 
 /**
  * Loads the block represented by the given state into the given workspace.
- * This is defined internally so that the extra optional parameter doesn't
- * clutter our external API.
+ * This is defined privately so that it can be called recursively without firing
+ * eroneous events. Events (and other things we only want to occur on the top
+ * block) are handled by loadInternal.
  * @param {!State} state The state of a block to deserialize into the workspace.
  * @param {!Workspace} workspace The workspace to add the block to.
- * @param {!Connection=} parentConnection The optional parent connection to
- *     attach the block to.
- * @param {boolean} isShadow Whether the block we are loading is a shadow block
- *     or not.
+ * @param {{parentConnection: (!Connection|undefined), isShadow:
+ *     (boolean|undefined), recordUndo: (boolean|undefined)}=} param1
+ *     parentConnection: If provided, the system will attempt to connect the
+ *       block to this connection after it is created. Undefined by default.
+ *     isShadow: The block will be set to a shadow block after it is created.
+ *       False by default.
  * @return {!Block} The block that was just loaded.
  */
-const loadInternal = function(
-    state, workspace, parentConnection = undefined, isShadow = false) {
+const loadPrivate = function(
+    state,
+    workspace,
+    {
+      parentConnection = undefined,
+      isShadow = false,
+    } = {}
+) {
   if (!state['type']) {
     throw new MissingBlockType(state);
   }
@@ -340,10 +378,9 @@ const loadInternal = function(
   loadInputBlocks(block, state);
   loadNextBlocks(block, state);
   initBlock(block, workspace.rendered);
+
   return block;
 };
-/** @package */
-exports.loadInternal = loadInternal;
 
 /**
  * Applies any coordinate information available on the state object to the
@@ -533,10 +570,10 @@ const loadConnection = function(connection, connectionState) {
     connection.setShadowState(connectionState['shadow']);
   }
   if (connectionState['block']) {
-    loadInternal(
+    loadPrivate(
         connectionState['block'],
         connection.getSourceBlock().workspace,
-        connection);
+        {parentConnection: connection});
   }
 };
 

tests/deps.mocha.js

@@ -307,6 +307,7 @@ goog.addDependency('../../generators/python/variables_dynamic.js', ['Blockly.Pyt
 goog.addDependency('../../tests/mocha/.mocharc.js', [], []);
 goog.addDependency('../../tests/mocha/astnode_test.js', ['Blockly.test.astNode'], ['Blockly.test.helpers'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/block_change_event_test.js', ['Blockly.test.blockChangeEvent'], ['Blockly.test.helpers'], {'lang': 'es6', 'module': 'goog'});
+goog.addDependency('../../tests/mocha/block_create_event_test.js', ['Blockly.test.blockCreateEvent'], ['Blockly.test.helpers'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/block_json_test.js', ['Blockly.test.blockJson'], [], {'lang': 'es5', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/block_test.js', ['Blockly.test.blocks'], ['Blockly.test.helpers'], {'lang': 'es6', 'module': 'goog'});
 goog.addDependency('../../tests/mocha/comment_test.js', ['Blockly.test.comments'], ['Blockly.test.helpers'], {'lang': 'es6', 'module': 'goog'});

tests/mocha/block_change_event_test.js

@@ -1,4 +1,3 @@
-
 /**
  * @license
  * Copyright 2021 Google LLC

tests/mocha/block_create_event_test.js

@@ -0,0 +1,58 @@
+/**
+ * @license
+ * Copyright 2021 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+goog.module('Blockly.test.blockCreateEvent');
+
+const {assertEventFired, sharedTestSetup, sharedTestTeardown} = goog.require('Blockly.test.helpers');
+
+
+suite('Block Create Event', function() {
+  setup(function() {
+    sharedTestSetup.call(this);
+    this.workspace = new Blockly.Workspace();
+  });
+
+  teardown(function() {
+    sharedTestTeardown.call(this);
+  });
+
+  test('Create shadow on disconnect', function() {
+    Blockly.Events.disable();
+    const block = Blockly.serialization.blocks.load(
+        {
+          "type": "text_print",
+          "inputs": {
+            "TEXT": {
+              "shadow": {
+                "type": "text",
+                "id": "shadowId",
+                "fields": {
+                  "TEXT": "abc"
+                }
+              },
+              "block": {
+                "type": "text",
+                "fields": {
+                  "TEXT": ""
+                }
+              }
+            }
+          }
+        },
+        this.workspace);
+    Blockly.Events.enable();
+    block.getInput('TEXT').connection.disconnect();
+    assertEventFired(
+        this.eventsFireStub,
+        Blockly.Events.BlockCreate,
+        {'recordUndo': false},
+        this.workspace.id,
+        'shadowId');
+    const calls = this.eventsFireStub.getCalls();
+    const event = calls[calls.length - 1].args[0];
+    chai.assert.equal(event.xml.tagName, 'shadow');
+  });
+});

tests/mocha/index.html

@@ -54,6 +54,7 @@
     <script>
       goog.require('Blockly.test.astNode');
       goog.require('Blockly.test.blockChangeEvent');
+      goog.require('Blockly.test.blockCreateEvent');
       goog.require('Blockly.test.blockJson');
       goog.require('Blockly.test.blocks');
       goog.require('Blockly.test.comments');