Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce connection preconditions for setParent #4999

Merged
merged 8 commits into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions core/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -741,25 +741,33 @@ Blockly.Block.prototype.getChildren = function(ordered) {
* @param {Blockly.Block} newParent New parent block.
*/
Blockly.Block.prototype.setParent = function(newParent) {
if (newParent == this.parentBlock_) {
if (newParent === this.parentBlock_) {
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
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_) {
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
// 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);
}

Expand Down
95 changes: 95 additions & 0 deletions tests/mocha/block_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
jschanker marked this conversation as resolved.
Show resolved Hide resolved
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');
Expand Down Expand Up @@ -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);
});
Expand Down