Disconnect child block before trying to connect (#2928)

* Disconnect child block before trying to connect

* Check original connections for errors before trying to connect

core/keyboard_nav/navigation.js

@@ -497,6 +497,27 @@ Blockly.navigation.modify = function() {
   // TODO: Make sure the cursor and marker end up in the right places.
 };
 
+/**
+ * If one of the connections source blocks is a child of the other, disconnect
+ * the child.
+ * @param {!Blockly.Connection} movingConnection The connection that is being
+ *     moved.
+ * @param {!Blockly.Connection} destConnection The connection to be moved to.
+ * @private
+ */
+Blockly.navigation.disconnectChild_ = function(movingConnection, destConnection) {
+  var movingBlock = movingConnection.getSourceBlock();
+  var destBlock = destConnection.getSourceBlock();
+
+  if (movingBlock.getRootBlock() == destBlock.getRootBlock()) {
+    if (movingBlock.getDescendants().indexOf(destBlock) > -1) {
+      Blockly.navigation.getInferiorConnection_(destConnection).disconnect();
+    } else {
+      Blockly.navigation.getInferiorConnection_(movingConnection).disconnect();
+    }
+  }
+};
+
 /**
  * If the two blocks are compatible move the moving connection to the target
  * connection and connect them.
@@ -511,6 +532,9 @@ Blockly.navigation.moveAndConnect_ = function(movingConnection, destConnection)
 
   if (destConnection.canConnectWithReason_(movingConnection) ==
       Blockly.Connection.CAN_CONNECT) {
+
+    Blockly.navigation.disconnectChild_(movingConnection, destConnection);
+
     if (!destConnection.isSuperior()) {
       var rootBlock = movingBlock.getRootBlock();
       rootBlock.positionNearConnection(movingConnection, destConnection);
@@ -588,16 +612,17 @@ Blockly.navigation.connect = function(movingConnection, destConnection) {
   } else if (movingSuperior && destInferior &&
       Blockly.navigation.moveAndConnect_(movingSuperior, destInferior)) {
     return true;
-  // If nothing else worked try connecting the given connections and report
-  // any errors.
+  } else if (Blockly.navigation.moveAndConnect_(movingConnection, destConnection)){
+    return true;
   } else {
     try {
-      destConnection.connect(movingConnection);
+      destConnection.checkConnection_(movingConnection);
     }
     catch (e) {
+      // If nothing worked report the error from the original connections.
       Blockly.navigation.warn('Connection failed with error: ' + e);
-      return false;
     }
+    return false;
   }
 };
 

tests/mocha/navigation_test.js

@@ -451,6 +451,24 @@ suite('Navigation', function() {
         "message0": "",
         "previousStatement": null,
         "nextStatement": null,
+      },
+      {
+        "type": "inline_block",
+        "message0": "%1 %2",
+        "args0": [
+          {
+            "type": "input_value",
+            "name": "NAME"
+          },
+          {
+            "type": "input_value",
+            "name": "NAME"
+          }
+        ],
+        "inputsInline": true,
+        "output": null,
+        "tooltip": "",
+        "helpUrl": ""
       }]);
 
       var toolbox = document.getElementById('toolbox-categories');
@@ -461,14 +479,23 @@ suite('Navigation', function() {
       var basicBlock3 = this.workspace.newBlock('basic_block');
       var basicBlock4 = this.workspace.newBlock('basic_block');
 
+      var inlineBlock1 = this.workspace.newBlock('inline_block');
+      var inlineBlock2 = this.workspace.newBlock('inline_block');
+
+
       this.basicBlock = basicBlock;
       this.basicBlock2 = basicBlock2;
       this.basicBlock3 = basicBlock3;
       this.basicBlock4 = basicBlock4;
 
+      this.inlineBlock1 = inlineBlock1;
+      this.inlineBlock2 = inlineBlock2;
+
       this.basicBlock.nextConnection.connect(this.basicBlock2.previousConnection);
 
       this.basicBlock3.nextConnection.connect(this.basicBlock4.previousConnection);
+
+      this.inlineBlock1.inputList[0].connection.connect(this.inlineBlock2.outputConnection);
     });
 
     teardown(function() {
@@ -517,6 +544,15 @@ suite('Navigation', function() {
       chai.assert.equal(this.basicBlock3.previousConnection.targetBlock(), this.basicBlock2);
     });
 
+    test('Try to connect input that is descendant of output', function() {
+      var markedLocation = this.inlineBlock2.inputList[0].connection;
+      var cursorLocation = this.inlineBlock1.outputConnection;
+
+      Blockly.navigation.connect(cursorLocation, markedLocation);
+
+      chai.assert.equal(this.inlineBlock2.outputConnection.targetBlock(), null);
+      chai.assert.equal(this.inlineBlock1.outputConnection.targetBlock(), this.inlineBlock2);
+    });
   });