Enforce connection preconditions for setParent (#4999)

* Fix error conditions for setParent #4989

* Error now thrown when calling `a.setParent(b)` on non-null b if the output/previous connection of a is not connected to an input/next connection of b without removing block from old parent's child list.
* Commented out error for when calling `a.setParent(null)` if a is connected to superior block.
* Adjusted comment to reflect that blocks were no longer being disconnected in this method.
* Also changed == to === per #4924 (`newParent` and `this.parentBlock_` must both be instances of `Blockly.Block`).

* Fix error conditions for setParent #4989

* Error now thrown when calling `a.setParent(b)` on non-null b if the output/previous connection of a is not connected to an input/next connection of b without removing block from old parent's child list.
* Error now thrown when calling `a.setParent(null)` if a is connected to a superior block.
* Adjusted comment to reflect that blocks were no longer being disconnected in this method.
* Also changed == to === per #4924 (`newParent` and `this.parentBlock_` must both be instances of `Blockly.Block`).

* Fix error conditions for setParent #4989

* Error now thrown when calling `a.setParent(b)` on non-null b if the output/previous connection of a is not connected to an input/next connection of b without removing block from old parent's child list.
* Commented out error for when calling `a.setParent(null)` if a is connected to superior block.
* Adjusted comment to reflect that blocks were no longer being disconnected in this method.
* Also changed == to === per google#4924 (`newParent` and `this.parentBlock_` must both be instances of `Blockly.Block`).
* Fixed lint errors.

* Fix error conditions for setParent google#4989

* Error now thrown when calling `a.setParent(b)` on non-null b if the output/previous connection of a is not connected to an input/next connection of b without removing block from old parent's child list.
* Commented out error for when calling `a.setParent(null)` if a is connected to superior block.
* Adjusted comment to reflect that blocks were no longer being disconnected in this method.
* Also changed == to === per google#4924 (`newParent` and `this.parentBlock_` must both be instances of `Blockly.Block`).
* Fixed lint errors.
* Adjusted comment.

* Removed unnecessary set to null/added tests

* One is failing (commented out), will investigate later

* Lint fix

* Removed failing test that correctly fails

* Update comments to conform to style guide

Capitalize first letter, period at end
This commit is contained in:
jschanker
2021-07-13 16:56:42 -04:00
committed by GitHub
parent 2b6b89dc54
commit de1b3214be
2 changed files with 114 additions and 11 deletions

View File

@@ -742,25 +742,33 @@ Blockly.Block.prototype.getChildren = function(ordered) {
* @package
*/
Blockly.Block.prototype.setParent = function(newParent) {
if (newParent == this.parentBlock_) {
if (newParent === this.parentBlock_) {
return;
}
// Check that block is connected to new parent if new parent is not null and
// that block is not connected to superior one if new parent is null.
var connection = this.previousConnection || this.outputConnection;
var isConnected = !!(connection && connection.targetBlock());
if (isConnected && newParent && connection.targetBlock() !== newParent) {
throw Error('Block connected to superior one that is not new parent.');
} else if (!isConnected && newParent) {
throw Error('Block not connected to new parent.');
} else if (isConnected && !newParent) {
throw Error('Cannot set parent to null while block is still connected to' +
' superior block.');
}
if (this.parentBlock_) {
// Remove this block from the old parent's child list.
Blockly.utils.arrayRemove(this.parentBlock_.childBlocks_, this);
// Disconnect from superior blocks.
if (this.previousConnection && this.previousConnection.isConnected()) {
throw Error('Still connected to previous block.');
}
if (this.outputConnection && this.outputConnection.isConnected()) {
throw Error('Still connected to parent block.');
}
this.parentBlock_ = null;
// This block hasn't actually moved on-screen, so there's no need to update
// its connection locations.
// its connection locations.
} else {
// Remove this block from the workspace's list of top-most blocks.
// New parent must be non-null so remove this block from the workspace's
// list of top-most blocks.
this.workspace.removeTopBlock(this);
}

View File

@@ -890,6 +890,96 @@ suite('Blocks', function() {
chai.assert.equal(this.getNext().length, 6);
});
});
suite('Setting Parent Block', function() {
setup(function() {
this.printBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(
'<block type="text_print">' +
' <value name="TEXT">' +
' <block type="text_join">' +
' <mutation items="2"></mutation>' +
' <value name="ADD0">' +
' <block type="text">' +
' </block>' +
' </value>' +
' </block>' +
' </value>' +
'</block>'
), this.workspace);
this.textJoinBlock = this.printBlock.getInputTargetBlock('TEXT');
this.textBlock = this.textJoinBlock.getInputTargetBlock('ADD0');
});
function assertBlockIsOnlyChild(parent, child, inputName) {
chai.assert.equal(parent.getChildren().length, 1);
chai.assert.equal(parent.getInputTargetBlock(inputName), child);
chai.assert.equal(child.getParent(), parent);
}
function assertNonParentAndOrphan(nonParent, orphan, inputName) {
chai.assert.equal(nonParent.getChildren().length, 0);
chai.assert.isNull(nonParent.getInputTargetBlock('TEXT'));
chai.assert.isNull(orphan.getParent());
}
function assertOriginalSetup() {
assertBlockIsOnlyChild(this.printBlock, this.textJoinBlock, 'TEXT');
assertBlockIsOnlyChild(this.textJoinBlock, this.textBlock, 'ADD0');
}
test('Setting to connected parent', function() {
chai.assert.doesNotThrow(this.textJoinBlock.setParent
.bind(this.textJoinBlock, this.printBlock));
assertOriginalSetup.call(this);
});
test('Setting to new parent after connecting to it', function() {
this.textJoinBlock.outputConnection.disconnect();
this.textBlock.outputConnection
.connect(this.printBlock.getInput('TEXT').connection);
chai.assert.doesNotThrow(this.textBlock.setParent
.bind(this.textBlock, this.printBlock));
assertBlockIsOnlyChild(this.printBlock, this.textBlock, 'TEXT');
});
test('Setting to new parent while connected to other block', function() {
// Setting to grandparent with no available input connection.
chai.assert.throws(this.textBlock.setParent
.bind(this.textBlock, this.printBlock));
this.textJoinBlock.outputConnection.disconnect();
// Setting to block with available input connection.
chai.assert.throws(this.textBlock.setParent
.bind(this.textBlock, this.printBlock));
assertNonParentAndOrphan(this.printBlock, this.textJoinBlock, 'TEXT');
assertBlockIsOnlyChild(this.textJoinBlock, this.textBlock, 'ADD0');
});
test('Setting to same parent after disconnecting from it', function() {
this.textJoinBlock.outputConnection.disconnect();
chai.assert.throws(this.textJoinBlock.setParent
.bind(this.textJoinBlock, this.printBlock));
assertNonParentAndOrphan(this.printBlock, this.textJoinBlock, 'TEXT');
});
test('Setting to new parent when orphan', function() {
this.textBlock.outputConnection.disconnect();
// When new parent has no available input connection.
chai.assert.throws(this.textBlock.setParent
.bind(this.textBlock, this.printBlock));
this.textJoinBlock.outputConnection.disconnect();
// When new parent has available input connection.
chai.assert.throws(this.textBlock.setParent
.bind(this.textBlock, this.printBlock));
assertNonParentAndOrphan(this.printBlock, this.textJoinBlock, 'TEXT');
assertNonParentAndOrphan(this.printBlock, this.textBlock, 'TEXT');
assertNonParentAndOrphan(this.textJoinBlock, this.textBlock, 'ADD0');
});
test('Setting parent to null after disconnecting', function() {
this.textBlock.outputConnection.disconnect();
chai.assert.doesNotThrow(this.textBlock.setParent
.bind(this.textBlock, null));
assertNonParentAndOrphan(this.textJoinBlock, this.textBlock, 'ADD0');
});
test('Setting parent to null without disconnecting', function() {
chai.assert.throws(this.textBlock.setParent
.bind(this.textBlock, null));
assertOriginalSetup.call(this);
});
});
suite('Remove Connections Programmatically', function() {
test('Output', function() {
var block = createRenderedBlock(this.workspace, 'row_block');
@@ -1106,11 +1196,16 @@ suite('Blocks', function() {
});
suite('Getting/Setting Field (Values)', function() {
setup(function() {
this.workspace = Blockly.inject('blocklyDiv');
this.block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(
'<block type="text"><field name = "TEXT">test</field></block>'
), this.workspace);
});
teardown(function() {
workspaceTeardown.call(this, this.workspace);
});
test('Getting Field', function() {
chai.assert.instanceOf(this.block.getField('TEXT'), Blockly.Field);
});