From 63bb89108b09c43e73f5d0e82ae0023ed0f35b99 Mon Sep 17 00:00:00 2001
From: Sean Lip <sll@google.com>
Date: Tue, 28 Jun 2016 18:28:47 -0700
Subject: [PATCH] Do not allow a block to be moved to a marked connection that
 it contains. Fix an error where block ids did not update when a new block is
 inserted in a list.

---
 accessible/clipboard.service.js        | 68 +++++++++++++++-----------
 accessible/toolbox-tree.component.js   |  7 ++-
 accessible/workspace-tree.component.js | 15 ++++--
 3 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/accessible/clipboard.service.js b/accessible/clipboard.service.js
index de9b382a3..eb8c0896d 100644
--- a/accessible/clipboard.service.js
+++ b/accessible/clipboard.service.js
@@ -30,6 +30,43 @@ blocklyApp.ClipboardService = ng.core
       this.clipboardBlockNextConnection_ = null;
       this.markedConnection_ = null;
     },
+    areConnectionsCompatible_: function(blockConnection, connection) {
+      // Check that both connections exist, that it's the right kind of
+      // connection, and that the types match.
+      return Boolean(
+          connection && blockConnection &&
+          Blockly.OPPOSITE_TYPE[blockConnection.type] == connection.type &&
+          connection.checkType_(blockConnection));
+    },
+    isCompatibleWithClipboard: function(connection) {
+      var superiorConnection = this.clipboardBlockSuperiorConnection_;
+      var nextConnection = this.clipboardBlockNextConnection_;
+      return Boolean(
+          this.areConnectionsCompatible_(connection, superiorConnection) ||
+          this.areConnectionsCompatible_(connection, nextConnection));
+    },
+    canBeMovedToMarkedConnection: function(block) {
+      // It should not be possible to move a block to one of its own
+      // connections.
+      if (this.markedConnection_ &&
+          this.markedConnection_.sourceBlock_.id == block.id) {
+        return false;
+      }
+
+      return this.canBeCopiedToMarkedConnection(block);
+    },
+    canBeCopiedToMarkedConnection: function(block) {
+      var blockConnection = block.outputConnection || block.previousConnection;
+      return Boolean(
+          this.markedConnection_ &&
+          this.markedConnection_.sourceBlock_.workspace &&
+          this.areConnectionsCompatible_(
+              blockConnection, this.markedConnection_));
+    },
+    markConnection: function(connection) {
+      this.markedConnection_ = connection;
+      alert(Blockly.Msg.MARKED_SPOT_MSG);
+    },
     cut: function(block) {
       var blockSummary = block.toString();
       this.copy(block, false);
@@ -64,8 +101,8 @@ blocklyApp.ClipboardService = ng.core
     },
     pasteToMarkedConnection: function(block, announce) {
       var xml = Blockly.Xml.blockToDom(block);
-      var reconstitutedBlock =
-          Blockly.Xml.domToBlock(blocklyApp.workspace, xml);
+      var reconstitutedBlock = Blockly.Xml.domToBlock(
+          blocklyApp.workspace, xml);
       this.markedConnection_.connect(
           reconstitutedBlock.outputConnection ||
           reconstitutedBlock.previousConnection);
@@ -74,32 +111,5 @@ blocklyApp.ClipboardService = ng.core
             Blockly.Msg.PASTED_BLOCK_TO_MARKED_SPOT_MSG +
             reconstitutedBlock.toString());
       }
-    },
-    markConnection: function(connection) {
-      this.markedConnection_ = connection;
-      alert(Blockly.Msg.MARKED_SPOT_MSG);
-    },
-    isCompatibleWithConnection_: function(blockConnection, connection) {
-      // Check that both connections exist, that the types match, and that it's
-      // the right kind of connection.
-      return Boolean(
-          connection && blockConnection &&
-          Blockly.OPPOSITE_TYPE[blockConnection.type] == connection.type &&
-          connection.checkType_(blockConnection));
-    },
-    isBlockCompatibleWithMarkedConnection: function(block) {
-      var blockConnection = block.outputConnection || block.previousConnection;
-      return Boolean(
-          this.markedConnection_ &&
-          this.markedConnection_.sourceBlock_.workspace &&
-          this.isCompatibleWithConnection_(
-              blockConnection, this.markedConnection_));
-    },
-    isClipboardCompatibleWithConnection: function(connection) {
-      var superiorConnection = this.clipboardBlockSuperiorConnection_;
-      var nextConnection = this.clipboardBlockNextConnection_;
-      return Boolean(
-          this.isCompatibleWithConnection_(connection, superiorConnection) ||
-          this.isCompatibleWithConnection_(connection, nextConnection));
     }
   });
diff --git a/accessible/toolbox-tree.component.js b/accessible/toolbox-tree.component.js
index cc701fc35..64320458d 100644
--- a/accessible/toolbox-tree.component.js
+++ b/accessible/toolbox-tree.component.js
@@ -59,12 +59,12 @@ blocklyApp.ToolboxTreeComponent = ng.core
               </button>
             </li>
             <li #sendToSelected [id]="idMap['sendToSelected']" role="treeitem"
-                [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['sendToSelectedButton'], 'blockly-button', !clipboardService.isBlockCompatibleWithMarkedConnection(block))"
+                [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['sendToSelectedButton'], 'blockly-button', !canBeCopiedToMarkedConnection(block))"
                 [attr.aria-level]="level+2" aria-selected=false>
               <button #sendToSelectedButton
                       [id]="idMap['sendToSelectedButton']"
                       (click)="copyToMarked(block)"
-                      [disabled]="!clipboardService.isBlockCompatibleWithMarkedConnection(block)">
+                      [disabled]="!canBeCopiedToMarkedConnection(block)">
                 {{'COPY_TO_MARKED_SPOT'|translate}}
               </button>
             </li>
@@ -138,6 +138,9 @@ blocklyApp.ToolboxTreeComponent = ng.core
       return this.utilsService.generateAriaLabelledByAttr(
           mainLabel, secondLabel, isDisabled);
     },
+    canBeCopiedToMarkedConnection: function(block) {
+      return this.clipboardService.canBeCopiedToMarkedConnection(block);
+    },
     copyToWorkspace: function(block) {
       var xml = Blockly.Xml.blockToDom(block);
       Blockly.Xml.domToBlock(blocklyApp.workspace, xml);
diff --git a/accessible/workspace-tree.component.js b/accessible/workspace-tree.component.js
index cf867ab45..48ecd2c14 100644
--- a/accessible/workspace-tree.component.js
+++ b/accessible/workspace-tree.component.js
@@ -79,10 +79,10 @@ blocklyApp.WorkspaceTreeComponent = ng.core
                       [disabled]="!hasPreviousConnection(block)">{{'MARK_SPOT_ABOVE'|translate}}</button>
             </li>
             <li [id]="idMap['sendToSelectedListItem']" role="treeitem"
-                [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['sendToSelectedButton'], 'blockly-button', !clipboardService.isBlockCompatibleWithMarkedConnection(block))"
+                [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['sendToSelectedButton'], 'blockly-button', !canBeMovedToMarkedConnection(block))"
                 [attr.aria-level]="level+2" aria-selected=false>
               <button [id]="idMap['sendToSelectedButton']" (click)="sendToMarkedSpot(block)"
-                      [disabled]="!clipboardService.isBlockCompatibleWithMarkedConnection(block)">{{'MOVE_TO_MARKED_SPOT'|translate}}</button>
+                      [disabled]="!canBeMovedToMarkedConnection(block)">{{'MOVE_TO_MARKED_SPOT'|translate}}</button>
             </li>
             <li [id]="idMap['delete']" role="treeitem"
                 [attr.aria-labelledBy]="generateAriaLabelledByAttr(idMap['deleteButton'], 'blockly-button')"
@@ -161,7 +161,10 @@ blocklyApp.WorkspaceTreeComponent = ng.core
 
       return elementsNeedingIds;
     },
-    ngOnInit: function() {
+    ngOnChanges: function() {
+      // The ids needs to be generated on every change (as opposed to only on
+      // init), so that they will update if, e.g., a new workspace-tree
+      // component is added to the middle of an array.
       var elementsNeedingIds = this.getElementsNeedingIds_();
 
       this.idMap = {}
@@ -183,6 +186,9 @@ blocklyApp.WorkspaceTreeComponent = ng.core
         this.treeService.setActiveDesc(this.idMap['parentList'], this.tree.id);
       }
     },
+    canBeMovedToMarkedConnection: function(block) {
+      return this.clipboardService.canBeMovedToMarkedConnection(block);
+    },
     hasPreviousConnection: function(block) {
       return Boolean(block.previousConnection);
     },
@@ -190,8 +196,7 @@ blocklyApp.WorkspaceTreeComponent = ng.core
       return Boolean(block.nextConnection);
     },
     isCompatibleWithClipboard: function(connection) {
-      return this.clipboardService.isClipboardCompatibleWithConnection(
-          connection);
+      return this.clipboardService.isCompatibleWithClipboard(connection);
     },
     isTopLevelBlock: function(block) {
       return blocklyApp.workspace.topBlocks_.some(function(topBlock) {