diff --git a/core/block.js b/core/block.js index c33d68433..6f51bc33f 100644 --- a/core/block.js +++ b/core/block.js @@ -200,29 +200,40 @@ Blockly.Block = function(workspace, prototypeName, opt_id) { workspace.addTopBlock(this); workspace.addTypedBlock(this); - // Call an initialization function, if it exists. - if (typeof this.init == 'function') { - this.init(); + // All events fired should be part of the same group. + // Any events fired during init should not be undoable, + // so that block creation is atomic. + var existingGroup = Blockly.Events.getGroup(); + if (!existingGroup) { + Blockly.Events.setGroup(true); } + var initialUndoFlag = Blockly.Events.recordUndo; + + try { + // Call an initialization function, if it exists. + if (typeof this.init == 'function') { + Blockly.Events.recordUndo = false; + this.init(); + Blockly.Events.recordUndo = initialUndoFlag; + } + + // Fire a create event. + if (Blockly.Events.isEnabled()) { + Blockly.Events.fire(new Blockly.Events.BlockCreate(this)); + } + + } finally { + if (!existingGroup) { + Blockly.Events.setGroup(false); + } + // In case init threw, recordUndo flag should still be reset. + Blockly.Events.recordUndo = initialUndoFlag; + } + // Record initial inline state. /** @type {boolean|undefined} */ this.inputsInlineDefault = this.inputsInline; - // Fire a create event. - if (Blockly.Events.isEnabled()) { - var existingGroup = Blockly.Events.getGroup(); - if (!existingGroup) { - Blockly.Events.setGroup(true); - } - try { - Blockly.Events.fire(new Blockly.Events.BlockCreate(this)); - } finally { - if (!existingGroup) { - Blockly.Events.setGroup(false); - } - } - - } // Bind an onchange function, if it exists. if (typeof this.onchange == 'function') { this.setOnChange(this.onchange); diff --git a/core/flyout_base.js b/core/flyout_base.js index bf59b3e7c..1a3c6f966 100644 --- a/core/flyout_base.js +++ b/core/flyout_base.js @@ -801,12 +801,15 @@ Blockly.Flyout.prototype.createBlock = function(originalBlock) { if (Blockly.Events.isEnabled()) { Blockly.Events.setGroup(true); - Blockly.Events.fire(new Blockly.Events.Create(newBlock)); // Fire a VarCreate event for each (if any) new variable created. for (var i = 0; i < newVariables.length; i++) { var thisVariable = newVariables[i]; Blockly.Events.fire(new Blockly.Events.VarCreate(thisVariable)); } + + // Block events come after var events, in case they refer to newly created + // variables. + Blockly.Events.fire(new Blockly.Events.Create(newBlock)); } if (this.autoClose) { this.hide(); diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 532125196..fc78017fc 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -1750,4 +1750,34 @@ suite('Blocks', function() { }); }); }); + + suite('Initialization', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([ + { + "type": "init_test_block", + "message0": "" + }, + ]); + }); + test('recordUndo is reset even if init throws', function() { + // The test could pass if init is never called, + // so we assert init was called to be safe. + var initCalled = false; + var recordUndoDuringInit; + Blockly.Blocks['init_test_block'].init = function() { + initCalled = true; + recordUndoDuringInit = Blockly.Events.recordUndo; + throw new Error(); + }; + chai.assert.throws(function() { + this.workspace.newBlock('init_test_block'); + }.bind(this)); + chai.assert.isFalse(recordUndoDuringInit, + 'recordUndo should be false during block init function'); + chai.assert.isTrue(Blockly.Events.recordUndo, + 'recordUndo should be reset to true after init'); + chai.assert.isTrue(initCalled, 'expected init function to be called'); + }); + }); }); diff --git a/tests/mocha/event_test.js b/tests/mocha/event_test.js index f88aafb93..77fd0c291 100644 --- a/tests/mocha/event_test.js +++ b/tests/mocha/event_test.js @@ -32,10 +32,16 @@ suite('Events', function() { function createSimpleTestBlock(workspace) { // Disable events while constructing the block: this is a test of the - // Blockly.Event constructors, not the block constructor.s + // Blockly.Event constructors, not the block constructors. + // Set the group id to avoid an extra call to genUid. Blockly.Events.disable(); - var block = new Blockly.Block( - workspace, 'simple_test_block'); + try { + Blockly.Events.setGroup('unused'); + var block = new Blockly.Block( + workspace, 'simple_test_block'); + } finally { + Blockly.Events.setGroup(false); + } Blockly.Events.enable(); return block; }