diff --git a/core/block.js b/core/block.js index 44bef90e3..a3d794a8c 100644 --- a/core/block.js +++ b/core/block.js @@ -226,6 +226,13 @@ Blockly.Block.obtain = function(workspace, prototypeName) { */ Blockly.Block.prototype.data = null; +/** + * Has this block been disposed of? + * @type {boolean} + * @package + */ +Blockly.Block.prototype.disposed = false; + /** * Colour of the block as HSV hue value (0-360) * This may be null if the block colour was not set via a hue number. @@ -316,15 +323,12 @@ Blockly.Block.prototype.dispose = function(healStack) { this.inputList.length = 0; // Dispose of any remaining connections (next/previous/output). var connections = this.getConnections_(true); - for (var i = 0; i < connections.length; i++) { - var connection = connections[i]; - if (connection.isConnected()) { - connection.disconnect(); - } - connections[i].dispose(); + for (var i = 0, connection; connection = connections[i]; i++) { + connection.dispose(); } } finally { Blockly.Events.enable(); + this.disposed = true; } }; @@ -1759,17 +1763,6 @@ Blockly.Block.prototype.moveNumberedInputBefore = function( Blockly.Block.prototype.removeInput = function(name, opt_quiet) { for (var i = 0, input; input = this.inputList[i]; i++) { if (input.name == name) { - if (input.connection && input.connection.isConnected()) { - input.connection.setShadowDom(null); - var block = input.connection.targetBlock(); - if (block.isShadow()) { - // Destroy any attached shadow block. - block.dispose(); - } else { - // Disconnect any attached normal block. - block.unplug(); - } - } input.dispose(); this.inputList.splice(i, 1); return; diff --git a/core/connection.js b/core/connection.js index 52e5f9f11..7594642a2 100644 --- a/core/connection.js +++ b/core/connection.js @@ -71,6 +71,13 @@ Blockly.Connection.REASON_SHADOW_PARENT = 6; */ Blockly.Connection.prototype.targetConnection = null; +/** + * Has this connection been disposed of? + * @type {boolean} + * @package + */ +Blockly.Connection.prototype.disposed = false; + /** * List of compatible value types. Null if all types are compatible. * @type {Array} @@ -233,15 +240,30 @@ Blockly.Connection.prototype.connect_ = function(childConnection) { }; /** - * Sever all links to this connection (not including from the source object). + * Dispose of this connection. Deal with connected blocks and remove this + * connection from the database. + * @package */ Blockly.Connection.prototype.dispose = function() { + + // isConnected returns true for shadows and non-shadows. if (this.isConnected()) { - throw Error('Disconnect connection before disposing of it.'); + this.setShadowDom(null); + var targetBlock = this.targetBlock(); + if (targetBlock.isShadow()) { + // Destroy the attached shadow block & its children. + targetBlock.dispose(); + } else { + // Disconnect the attached normal block. + targetBlock.unplug(); + } } + if (this.inDB_) { this.db_.removeConnection_(this); } + + this.disposed = true; }; /** diff --git a/core/field.js b/core/field.js index 09bb1f1df..51bde7c69 100644 --- a/core/field.js +++ b/core/field.js @@ -149,6 +149,7 @@ Blockly.Field.X_PADDING = 10; * @type {[type]} */ Blockly.Field.DEFAULT_TEXT_OFFSET = Blockly.Field.X_PADDING / 2; + /** * Name of field. Unique within each block. * Static labels are usually unnamed. @@ -156,6 +157,13 @@ Blockly.Field.DEFAULT_TEXT_OFFSET = Blockly.Field.X_PADDING / 2; */ Blockly.Field.prototype.name = undefined; +/** + * Has this field been disposed of? + * @type {boolean} + * @package + */ +Blockly.Field.prototype.disposed = false; + /** * Maximum characters of text to display before adding an ellipsis. * @type {number} diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 82ea90490..b0d87ed4f 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -1,27 +1,32 @@ - +/** + * @license + * Visual Blocks Editor + * + * Copyright 2019 Google Inc. + * https://developers.google.com/blockly/ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ suite('Blocks', function() { + setup(function() { + this.workspace = new Blockly.Workspace(); + }); + teardown(function() { + this.workspace.dispose(); + }); - suite('Unplug', function() { - function assertUnpluggedNoheal(blocks) { - // A has nothing connected to it. - assertEquals(0, blocks.A.getChildren().length); - // B and C are still connected. - assertEquals(blocks.B, blocks.C.getParent()); - // B is the top of its stack. - assertNull(blocks.B.getParent()); - } - - function assertUnpluggedHealed(blocks) { - // A and C are connected. - assertEquals(1, blocks.A.getChildren().length); - assertEquals(blocks.A, blocks.C.getParent()); - // B has nothing connected to it. - assertEquals(0, blocks.B.getChildren().length); - // B is the top of its stack. - assertNull(blocks.B.getParent()); - } - + suite('Connection Management', function() { setup(function() { Blockly.defineBlocksWithJsonArray([{ "type": "stack_block", @@ -40,136 +45,375 @@ suite('Blocks', function() { ], "output": null }]); - - this.workspace = new Blockly.Workspace(); }); - teardown(function() { delete Blockly.Blocks['stack_block']; delete Blockly.Blocks['row_block']; - - this.workspace.dispose(); }); - suite('Row', function() { - setup(function() { - var blockA = this.workspace.newBlock('row_block'); - var blockB = this.workspace.newBlock('row_block'); - var blockC = this.workspace.newBlock('row_block'); - - blockA.inputList[0].connection.connect(blockB.outputConnection); - blockB.inputList[0].connection.connect(blockC.outputConnection); - - assertEquals(blockB, blockC.getParent()); - - this.blocks = { - A: blockA, - B: blockB, - C: blockC - }; - }); - - test('Don\'t heal', function() { - this.blocks.B.unplug(false); - assertUnpluggedNoheal(this.blocks); - }); - - test('Heal', function() { - this.blocks.B.unplug(true); - // Each block has only one input, and the types work. - assertUnpluggedHealed(this.blocks); - }); - - test('Heal with bad checks', function() { - var blocks = this.blocks; - - // A and C can't connect, but both can connect to B. - blocks.A.inputList[0].connection.setCheck('type1'); - blocks.C.outputConnection.setCheck('type2'); - - // Each block has only one input, but the types don't work. - blocks.B.unplug(true); - assertUnpluggedNoheal(blocks); - }); - - test('Parent has multiple inputs', function() { - var blocks = this.blocks; - // Add extra input to parent - blocks.A.appendValueInput("INPUT").setCheck(null); - blocks.B.unplug(true); - assertUnpluggedHealed(blocks); - }); - - test('Middle block has multiple inputs', function() { - var blocks = this.blocks; - // Add extra input to middle block - blocks.B.appendValueInput("INPUT").setCheck(null); - blocks.B.unplug(true); - assertUnpluggedHealed(blocks); - }); - - test('Child block has multiple inputs', function() { - var blocks = this.blocks; - // Add extra input to child block - blocks.C.appendValueInput("INPUT").setCheck(null); - // Child block input count doesn't matter. - blocks.B.unplug(true); - assertUnpluggedHealed(blocks); - }); - }); - - - suite('Stack', function() { - setup(function() { - var blockA = this.workspace.newBlock('stack_block'); - var blockB = this.workspace.newBlock('stack_block'); - var blockC = this.workspace.newBlock('stack_block'); - - blockA.nextConnection.connect(blockB.previousConnection); - blockB.nextConnection.connect(blockC.previousConnection); - - assertEquals(blockB, blockC.getParent()); - - this.blocks = { - A: blockA, - B: blockB, - C: blockC - }; - }); - - test('Don\'t heal', function() { - this.blocks.B.unplug(); - assertUnpluggedNoheal(this.blocks); - }); - - test('Heal', function() { - this.blocks.B.unplug(true); - assertUnpluggedHealed(this.blocks); - }); - - test('Heal with bad checks', function() { - var blocks = this.blocks; - // A and C can't connect, but both can connect to B. - blocks.A.nextConnection.setCheck('type1'); - blocks.C.previousConnection.setCheck('type2'); - - // The types don't work. - blocks.B.unplug(true); - - // Stack blocks unplug before checking whether the types match. - // TODO (#1994): Check types before unplugging. + suite('Unplug', function() { + function assertUnpluggedNoheal(blocks) { // A has nothing connected to it. assertEquals(0, blocks.A.getChildren().length); - // B has nothing connected to it. - assertEquals(0, blocks.B.getChildren().length); - // C has nothing connected to it. - assertEquals(0, blocks.C.getChildren().length); - // A is the top of its stack. - assertNull(blocks.A.getParent()); + // B and C are still connected. + assertEquals(blocks.B, blocks.C.getParent()); // B is the top of its stack. assertNull(blocks.B.getParent()); - // C is the top of its stack. - assertNull(blocks.C.getParent()); + } + function assertUnpluggedHealed(blocks) { + // A and C are connected. + assertEquals(1, blocks.A.getChildren().length); + assertEquals(blocks.A, blocks.C.getParent()); + // B has nothing connected to it. + assertEquals(0, blocks.B.getChildren().length); + // B is the top of its stack. + assertNull(blocks.B.getParent()); + } + + suite('Row', function() { + setup(function() { + var blockA = this.workspace.newBlock('row_block'); + var blockB = this.workspace.newBlock('row_block'); + var blockC = this.workspace.newBlock('row_block'); + + blockA.inputList[0].connection.connect(blockB.outputConnection); + blockB.inputList[0].connection.connect(blockC.outputConnection); + + assertEquals(blockB, blockC.getParent()); + + this.blocks = { + A: blockA, + B: blockB, + C: blockC + }; + }); + + test('Don\'t heal', function() { + this.blocks.B.unplug(false); + assertUnpluggedNoheal(this.blocks); + }); + test('Heal', function() { + this.blocks.B.unplug(true); + // Each block has only one input, and the types work. + assertUnpluggedHealed(this.blocks); + }); + test('Heal with bad checks', function() { + var blocks = this.blocks; + + // A and C can't connect, but both can connect to B. + blocks.A.inputList[0].connection.setCheck('type1'); + blocks.C.outputConnection.setCheck('type2'); + + // Each block has only one input, but the types don't work. + blocks.B.unplug(true); + assertUnpluggedNoheal(blocks); + }); + test('A has multiple inputs', function() { + var blocks = this.blocks; + // Add extra input to parent + blocks.A.appendValueInput("INPUT").setCheck(null); + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); + }); + test('B has multiple inputs', function() { + var blocks = this.blocks; + // Add extra input to middle block + blocks.B.appendValueInput("INPUT").setCheck(null); + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); + }); + test('C has multiple inputs', function() { + var blocks = this.blocks; + // Add extra input to child block + blocks.C.appendValueInput("INPUT").setCheck(null); + // Child block input count doesn't matter. + blocks.B.unplug(true); + assertUnpluggedHealed(blocks); + }); + test('C is Shadow', function() { + var blocks = this.blocks; + blocks.C.setShadow(true); + blocks.B.unplug(true); + // Even though we're asking to heal, it will appear as if it has not + // healed because shadows always stay with the parent. + assertUnpluggedNoheal(blocks); + }); + }); + suite('Stack', function() { + setup(function() { + var blockA = this.workspace.newBlock('stack_block'); + var blockB = this.workspace.newBlock('stack_block'); + var blockC = this.workspace.newBlock('stack_block'); + + blockA.nextConnection.connect(blockB.previousConnection); + blockB.nextConnection.connect(blockC.previousConnection); + + assertEquals(blockB, blockC.getParent()); + + this.blocks = { + A: blockA, + B: blockB, + C: blockC + }; + }); + + test('Don\'t heal', function() { + this.blocks.B.unplug(); + assertUnpluggedNoheal(this.blocks); + }); + test('Heal', function() { + this.blocks.B.unplug(true); + assertUnpluggedHealed(this.blocks); + }); + test.skip('Heal with bad checks', function() { + var blocks = this.blocks; + // A and C can't connect, but both can connect to B. + blocks.A.nextConnection.setCheck('type1'); + blocks.C.previousConnection.setCheck('type2'); + + // The types don't work. + blocks.B.unplug(true); + + // TODO (#1994): Check types before unplugging. Currently + // everything disconnects, when C should stick with B + assertUnpluggedNoheal(); + }); + test('C is Shadow', function() { + var blocks = this.blocks; + blocks.C.setShadow(true); + blocks.B.unplug(true); + // Even though we're asking to heal, it will appear as if it has not + // healed because shadows always stay with the parent. + assertUnpluggedNoheal(blocks); + }); + }); + }); + suite('Dispose', function() { + function assertDisposedNoheal(blocks) { + chai.assert.isFalse(blocks.A.disposed); + // A has nothing connected to it. + chai.assert.equal(0, blocks.A.getChildren().length); + // B is disposed. + chai.assert.isTrue(blocks.B.disposed); + // And C is disposed. + chai.assert.isTrue(blocks.C.disposed); + } + function assertDisposedHealed(blocks) { + chai.assert.isFalse(blocks.A.disposed); + chai.assert.isFalse(blocks.C.disposed); + // A and C are connected. + assertEquals(1, blocks.A.getChildren().length); + assertEquals(blocks.A, blocks.C.getParent()); + // B is disposed. + chai.assert.isTrue(blocks.B.disposed); + } + + suite('Row', function() { + setup(function() { + var blockA = this.workspace.newBlock('row_block'); + var blockB = this.workspace.newBlock('row_block'); + var blockC = this.workspace.newBlock('row_block'); + + blockA.inputList[0].connection.connect(blockB.outputConnection); + blockB.inputList[0].connection.connect(blockC.outputConnection); + + assertEquals(blockB, blockC.getParent()); + + this.blocks = { + A: blockA, + B: blockB, + C: blockC + }; + }); + + test('Don\'t heal', function() { + this.blocks.B.dispose(false); + assertDisposedNoheal(this.blocks); + }); + test('Heal', function() { + this.blocks.B.dispose(true); + // Each block has only one input, and the types work. + assertDisposedHealed(this.blocks); + }); + test('Heal with bad checks', function() { + var blocks = this.blocks; + + // A and C can't connect, but both can connect to B. + blocks.A.inputList[0].connection.setCheck('type1'); + blocks.C.outputConnection.setCheck('type2'); + + // Each block has only one input, but the types don't work. + blocks.B.dispose(true); + assertDisposedNoheal(blocks); + }); + test('A has multiple inputs', function() { + var blocks = this.blocks; + // Add extra input to parent + blocks.A.appendValueInput("INPUT").setCheck(null); + blocks.B.dispose(true); + assertDisposedHealed(blocks); + }); + test('B has multiple inputs', function() { + var blocks = this.blocks; + // Add extra input to middle block + blocks.B.appendValueInput("INPUT").setCheck(null); + blocks.B.dispose(true); + assertDisposedHealed(blocks); + }); + test('C has multiple inputs', function() { + var blocks = this.blocks; + // Add extra input to child block + blocks.C.appendValueInput("INPUT").setCheck(null); + // Child block input count doesn't matter. + blocks.B.dispose(true); + assertDisposedHealed(blocks); + }); + test('C is Shadow', function() { + var blocks = this.blocks; + blocks.C.setShadow(true); + blocks.B.dispose(true); + // Even though we're asking to heal, it will appear as if it has not + // healed because shadows always get destroyed. + assertDisposedNoheal(blocks); + }); + }); + suite('Stack', function() { + setup(function() { + var blockA = this.workspace.newBlock('stack_block'); + var blockB = this.workspace.newBlock('stack_block'); + var blockC = this.workspace.newBlock('stack_block'); + + blockA.nextConnection.connect(blockB.previousConnection); + blockB.nextConnection.connect(blockC.previousConnection); + + assertEquals(blockB, blockC.getParent()); + + this.blocks = { + A: blockA, + B: blockB, + C: blockC + }; + }); + + test('Don\'t heal', function() { + this.blocks.B.dispose(); + assertDisposedNoheal(this.blocks); + }); + test('Heal', function() { + this.blocks.B.dispose(true); + assertDisposedHealed(this.blocks); + }); + test.skip('Heal with bad checks', function() { + var blocks = this.blocks; + // A and C can't connect, but both can connect to B. + blocks.A.nextConnection.setCheck('type1'); + blocks.C.previousConnection.setCheck('type2'); + + // The types don't work. + blocks.B.dispose(true); + + // TODO (#1994): Check types before unplugging. Current C gets + // left behind when it should get disposed with B. + assertDisposedNoheal(blocks); + }); + test('C is Shadow', function() { + var blocks = this.blocks; + blocks.C.setShadow(true); + blocks.B.dispose(true); + // Even though we're asking to heal, it will appear as if it has not + // healed because shadows always get destroyed. + assertDisposedNoheal(blocks); + }); + }); + }); + suite('Remove Input', function() { + setup(function() { + Blockly.defineBlocksWithJsonArray([ + { + "type": "value_block", + "message0": "%1", + "args0": [ + { + "type": "input_value", + "name": "VALUE" + } + ] + }, + { + "type": "statement_block", + "message0": "%1", + "args0": [ + { + "type": "input_statement", + "name": "STATEMENT" + } + ] + }, + ]); + }); + teardown(function() { + delete Blockly.Blocks['value_block']; + delete Blockly.Blocks['statement_block']; + }); + + suite('Value', function() { + setup(function() { + this.blockA = this.workspace.newBlock('value_block'); + }); + + test('No Connected', function() { + this.blockA.removeInput('VALUE'); + chai.assert.isNull(this.blockA.getInput('VALUE')); + }); + test('Block Connected', function() { + var blockB = this.workspace.newBlock('row_block'); + this.blockA.getInput('VALUE').connection + .connect(blockB.outputConnection); + + this.blockA.removeInput('VALUE'); + chai.assert.isFalse(blockB.disposed); + chai.assert.equal(this.blockA.getChildren().length, 0); + }); + test('Shadow Connected', function() { + var blockB = this.workspace.newBlock('row_block'); + blockB.setShadow(true); + this.blockA.getInput('VALUE').connection + .connect(blockB.outputConnection); + + this.blockA.removeInput('VALUE'); + chai.assert.isTrue(blockB.disposed); + chai.assert.equal(this.blockA.getChildren().length, 0); + }); + }); + suite('Statement', function() { + setup(function() { + this.blockA = this.workspace.newBlock('statement_block'); + }); + + test('No Connected', function() { + this.blockA.removeInput('STATEMENT'); + chai.assert.isNull(this.blockA.getInput('STATEMENT')); + }); + test('Block Connected', function() { + var blockB = this.workspace.newBlock('stack_block'); + this.blockA.getInput('STATEMENT').connection + .connect(blockB.previousConnection); + + this.blockA.removeInput('STATEMENT'); + chai.assert.isFalse(blockB.disposed); + chai.assert.equal(this.blockA.getChildren().length, 0); + }); + test('Shadow Connected', function() { + var blockB = this.workspace.newBlock('stack_block'); + blockB.setShadow(true); + this.blockA.getInput('STATEMENT').connection + .connect(blockB.previousConnection); + + this.blockA.removeInput('STATEMENT'); + console.log(blockB.disposed, blockB); + chai.assert.isTrue(blockB.disposed); + chai.assert.equal(this.blockA.getChildren().length, 0); + }); }); }); }); diff --git a/tests/mocha/connection_test.js b/tests/mocha/connection_test.js index b4d171927..9f7f726c6 100644 --- a/tests/mocha/connection_test.js +++ b/tests/mocha/connection_test.js @@ -1,4 +1,22 @@ - +/** + * @license + * Visual Blocks Editor + * + * Copyright 2019 Google Inc. + * https://developers.google.com/blockly/ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ suite('Connections', function() {