fix: insertion markers and change events to work with JSO hooks (#5378)

* fix: add tests for fixing change events

* fix: change events and insertion markers

* fix: build:

* fix: remove duplicate code

* fix: requires

core/events/block_events.js

@@ -193,23 +193,43 @@ Blockly.Events.BlockChange.prototype.run = function(forward) {
       block.setInputsInline(!!value);
       break;
     case 'mutation':
-      var oldMutation = '';
-      if (block.mutationToDom) {
-        var oldMutationDom = block.mutationToDom();
-        oldMutation = oldMutationDom && Blockly.Xml.domToText(oldMutationDom);
-      }
-      if (block.domToMutation) {
-        var dom = Blockly.Xml.textToDom(/** @type {string} */ (value) || '<mutation/>');
-        block.domToMutation(dom);
+      var oldState = Blockly.Events.BlockChange.getExtraBlockState_(
+          /** @type {!Blockly.BlockSvg} */ (block));
+      if (block.loadExtraState) {
+        block.loadExtraState(JSON.parse(/** @type {string} */ (value) || '{}'));
+      } else if (block.domToMutation) {
+        block.domToMutation(
+            Blockly.Xml.textToDom(
+                /** @type {string} */ (value) || '<mutation/>'));
       }
       Blockly.Events.fire(new Blockly.Events.BlockChange(
-          block, 'mutation', null, oldMutation, value));
+          block, 'mutation', null, oldState, value));
       break;
     default:
       console.warn('Unknown change type: ' + this.element);
   }
 };
 
+// TODO (#5397): Encapsulate this in the BlocklyMutationChange event when
+//    refactoring change events.
+/**
+ * Returns the extra state of the given block (either as XML or a JSO, depending
+ * on the block's definition).
+ * @param {!Blockly.BlockSvg} block The block to get the extra state of.
+ * @return {string} A stringified version of the extra state of the given block.
+ * @package
+ */
+Blockly.Events.BlockChange.getExtraBlockState_ = function(block) {
+  if (block.saveExtraState) {
+    var state = block.saveExtraState();
+    return state ? JSON.stringify(state) : '';
+  } else if (block.mutationToDom) {
+    var state = block.mutationToDom();
+    return state ? Blockly.Xml.domToText(state) : '';
+  }
+  return '';
+};
+
 /**
  * Class for a block creation event.
  * @param {!Blockly.Block=} opt_block The created block.  Undefined for a blank

core/field.js

@@ -856,11 +856,11 @@ Blockly.Field.prototype.setValue = function(newValue) {
     return;
   }
 
+  this.doValueUpdate_(newValue);
   if (source && Blockly.Events.isEnabled()) {
     Blockly.Events.fire(new (Blockly.Events.get(Blockly.Events.BLOCK_CHANGE))(
         source, 'field', this.name || null, oldValue, newValue));
   }
-  this.doValueUpdate_(newValue);
   if (this.isDirty_) {
     this.forceRerender();
   }

core/insertion_marker_manager.js

@@ -277,7 +277,12 @@ Blockly.InsertionMarkerManager.prototype.createMarkerBlock_ = function(sourceBlo
   try {
     var result = this.workspace_.newBlock(imType);
     result.setInsertionMarker(true);
-    if (sourceBlock.mutationToDom) {
+    if (sourceBlock.saveExtraState) {
+      var state = sourceBlock.saveExtraState();
+      if (state) {
+        result.loadExtraState(state);
+      }
+    } else if (sourceBlock.mutationToDom) {
       var oldMutationDom = sourceBlock.mutationToDom();
       if (oldMutationDom) {
         result.domToMutation(oldMutationDom);

core/mutator.js

@@ -28,7 +28,6 @@ goog.require('Blockly.utils.Svg');
 goog.require('Blockly.utils.toolbox');
 goog.require('Blockly.utils.xml');
 goog.require('Blockly.WorkspaceSvg');
-goog.require('Blockly.Xml');
 
 goog.requireType('Blockly.Block');
 goog.requireType('Blockly.BlockSvg');
@@ -413,9 +412,8 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) {
   // When the mutator's workspace changes, update the source block.
   if (this.rootBlock_.workspace == this.workspace_) {
     Blockly.Events.setGroup(true);
-    var block = this.block_;
-    var oldMutationDom = block.mutationToDom();
-    var oldMutation = oldMutationDom && Blockly.Xml.domToText(oldMutationDom);
+    var block = /** @type {!Blockly.BlockSvg} */ (this.block_);
+    var oldExtraState = Blockly.Events.BlockChange.getExtraBlockState_(block);
 
     // Switch off rendering while the source block is rebuilt.
     var savedRendered = block.rendered;
@@ -433,11 +431,10 @@ Blockly.Mutator.prototype.workspaceChanged_ = function(e) {
       block.render();
     }
 
-    var newMutationDom = block.mutationToDom();
-    var newMutation = newMutationDom && Blockly.Xml.domToText(newMutationDom);
-    if (oldMutation != newMutation) {
+    var newExtraState = Blockly.Events.BlockChange.getExtraBlockState_(block);
+    if (oldExtraState != newExtraState) {
       Blockly.Events.fire(new (Blockly.Events.get(Blockly.Events.BLOCK_CHANGE))(
-          block, 'mutation', null, oldMutation, newMutation));
+          block, 'mutation', null, oldExtraState, newExtraState));
       // Ensure that any bump is part of this mutation's event group.
       var group = Blockly.Events.getGroup();
       setTimeout(function() {

tests/mocha/.eslintrc.json

@@ -30,6 +30,7 @@
         "defineRowBlock": true,
         "defineStackBlock": true,
         "defineStatementBlock": true,
+        "defineMutatorBlocks": true,
         "dispatchPointerEvent": true,
         "createFireChangeListenerSpy": true,
         "createGenUidStubWithReturns": true,

tests/mocha/block_change_event_test.js

@@ -0,0 +1,69 @@
+
+/**
+ * @license
+ * Copyright 2021 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+suite('Block Change Event', function() {
+  setup(function() {
+    sharedTestSetup.call(this);
+    this.workspace = new Blockly.Workspace();
+  });
+
+  teardown(function() {
+    sharedTestTeardown.call(this);
+  });
+
+  suite('Undo and Redo', function() {
+    suite('Mutation', function() {
+      setup(function() {
+        defineMutatorBlocks();
+      });
+
+      teardown(function() {
+        Blockly.Extensions.unregister('xml_mutator');
+        Blockly.Extensions.unregister('jso_mutator');
+      });
+
+      suite('XML', function() {
+        test('Undo', function() {
+          const block = this.workspace.newBlock('xml_block', 'block_id');
+          block.domToMutation(
+              Blockly.Xml.textToDom('<mutation hasInput="true"/>'));
+          const blockChange = new Blockly.Events.BlockChange(
+              block, 'mutation', null, '', '<mutation hasInput="true"/>');
+          blockChange.run(false);
+          chai.assert.isFalse(block.hasInput);
+        });
+
+        test('Redo', function() {
+          const block = this.workspace.newBlock('xml_block', 'block_id');
+          const blockChange = new Blockly.Events.BlockChange(
+              block, 'mutation', null, '', '<mutation hasInput="true"/>');
+          blockChange.run(true);
+          chai.assert.isTrue(block.hasInput);
+        });
+      });
+
+      suite('JSO', function() {
+        test('Undo', function() {
+          const block = this.workspace.newBlock('jso_block', 'block_id');
+          block.loadExtraState({hasInput: true});
+          const blockChange = new Blockly.Events.BlockChange(
+              block, 'mutation', null, '', '{"hasInput":true}');
+          blockChange.run(false);
+          chai.assert.isFalse(block.hasInput);
+        });
+
+        test('Redo', function() {
+          const block = this.workspace.newBlock('jso_block', 'block_id');
+          const blockChange = new Blockly.Events.BlockChange(
+              block, 'mutation', null, '', '{"hasInput":true}');
+          blockChange.run(true);
+          chai.assert.isTrue(block.hasInput);
+        });
+      });
+    });
+  });
+});

tests/mocha/index.html

@@ -54,6 +54,7 @@
     <script src="toolbox_helper.js"></script>
 
     <script src="astnode_test.js"></script>
+    <script src="block_change_event_test.js"></script>
     <script src="block_json_test.js"></script>
     <script src="block_test.js"></script>
     <script src="comment_test.js"></script>
@@ -90,6 +91,7 @@
     <script src="keydown_test.js"></script>
     <script src="logic_ternary_test.js"></script>
     <script src="metrics_test.js"></script>
+    <script src="mutator_test.js"></script>
     <script src="names_test.js"></script>
     <script src="procedures_test_helpers.js"></script>
     <script src="procedures_test.js"></script>

tests/mocha/mutator_test.js

@@ -0,0 +1,66 @@
+
+/**
+ * @license
+ * Copyright 2021 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+suite('Mutator', function() {
+  setup(function() {
+    sharedTestSetup.call(this);
+  });
+
+  suite('Firing change event', function() {
+    setup(function() {
+      this.workspace = Blockly.inject('blocklyDiv', {});
+      defineMutatorBlocks();
+    });
+
+    teardown(function() {
+      Blockly.Extensions.unregister('xml_mutator');
+      Blockly.Extensions.unregister('jso_mutator');
+      sharedTestTeardown.call(this);
+    });
+
+    test('No change', function() {
+      var block = createRenderedBlock(this.workspace, 'xml_block');
+      block.mutator.setVisible(true);
+      var mutatorWorkspace = block.mutator.getWorkspace();
+      // Trigger mutator change listener.
+      createRenderedBlock(mutatorWorkspace, 'checkbox_block');
+      chai.assert.isTrue(
+          this.eventsFireStub.getCalls().every(
+              ({args}) =>
+                args[0].type !== Blockly.Events.BLOCK_CHANGE ||
+                args[0].element !== 'mutation'));
+    });
+
+    test('XML', function() {
+      var block = createRenderedBlock(this.workspace, 'xml_block');
+      block.mutator.setVisible(true);
+      var mutatorWorkspace = block.mutator.getWorkspace();
+      mutatorWorkspace.getBlockById('check_block')
+          .setFieldValue('TRUE', 'CHECK');
+      chai.assert.isTrue(
+          this.eventsFireStub.getCalls().some(
+              ({args}) =>
+                args[0].type === Blockly.Events.BLOCK_CHANGE &&
+                args[0].element === 'mutation' &&
+                /<mutation.*><\/mutation>/.test(args[0].newValue)));
+    });
+
+    test('JSO', function() {
+      var block = createRenderedBlock(this.workspace, 'jso_block');
+      block.mutator.setVisible(true);
+      var mutatorWorkspace = block.mutator.getWorkspace();
+      mutatorWorkspace.getBlockById('check_block')
+          .setFieldValue('TRUE', 'CHECK');
+      chai.assert.isTrue(
+          this.eventsFireStub.getCalls().some(
+              ({args}) =>
+                args[0].type === Blockly.Events.BLOCK_CHANGE &&
+                args[0].element === 'mutation' &&
+                args[0].newValue === '{"hasInput":true}'));
+    });
+  });
+});

tests/mocha/test_helpers.js

@@ -497,6 +497,7 @@ function defineStatementBlock() {
     "helpUrl": ""
   }]);
 }
+
 function defineBasicBlockWithField() {
   Blockly.defineBlocksWithJsonArray([{
     "type": "test_field_block",
@@ -511,6 +512,99 @@ function defineBasicBlockWithField() {
   }]);
 }
 
+function defineMutatorBlocks() {
+  Blockly.defineBlocksWithJsonArray([
+    {
+      'type': 'xml_block',
+      'mutator': 'xml_mutator'
+    },
+    {
+      'type': 'jso_block',
+      'mutator': 'jso_mutator'
+    },
+    {
+      'type': 'checkbox_block',
+      'message0': '%1',
+      'args0': [
+        {
+          'type': 'field_checkbox',
+          'name': 'CHECK'
+        }
+      ]
+    }
+  ]);
+
+  const xmlMutator = {
+    hasInput: false,
+
+    mutationToDom: function() {
+      var mutation = Blockly.utils.xml.createElement('mutation');
+      mutation.setAttribute('hasInput', this.hasInput);
+      return mutation;
+    },
+
+    domToMutation: function(mutation) {
+      this.hasInput = mutation.getAttribute('hasInput') == 'true';
+      this.updateShape();
+    },
+
+    decompose: function(workspace) {
+      var topBlock = workspace.newBlock('checkbox_block', 'check_block');
+      topBlock.initSvg();
+      topBlock.render();
+      return topBlock;
+    },
+
+    compose: function(topBlock) {
+      this.hasInput = topBlock.getFieldValue('CHECK') == 'TRUE';
+      this.updateShape();
+    },
+
+    updateShape: function() {
+      if (this.hasInput && !this.getInput('INPUT')) {
+        this.appendValueInput('INPUT');
+      } else if (!this.hasInput && this.getInput('INPUT')) {
+        this.removeInput('INPUT');
+      }
+    }
+  };
+  Blockly.Extensions.registerMutator('xml_mutator', xmlMutator);
+
+  const jsoMutator = {
+    hasInput: false,
+
+    saveExtraState: function() {
+      return {hasInput: this.hasInput};
+    },
+
+    loadExtraState: function(state) {
+      this.hasInput = state.hasInput || false;
+      this.updateShape();
+    },
+
+    decompose: function(workspace) {
+      var topBlock = workspace.newBlock('checkbox_block', 'check_block');
+      topBlock.initSvg();
+      topBlock.render();
+      return topBlock;
+    },
+
+    compose: function(topBlock) {
+      this.hasInput = topBlock.getFieldValue('CHECK') == 'TRUE';
+      this.updateShape();
+    },
+
+    updateShape: function() {
+      if (this.hasInput && !this.getInput('INPUT')) {
+        this.appendValueInput('INPUT');
+      } else if (!this.hasInput && this.getInput('INPUT')) {
+        this.removeInput('INPUT');
+      }
+    }
+  };
+  Blockly.Extensions.registerMutator('jso_mutator', jsoMutator);
+}
+
 function createTestBlock() {
   return {
     id: 'test',