Cleaning up undo and redo of events some more (particularly undo deletion). Setting the size of a comment is separate from setting the visual comment bubble. E.g. minimizing a comment changes the bubble size but not the comment size. Separate this functionaltiy into two separate functions. Creating a comment should also take in stater text. Creating a comment for the first time does not also need to trigger an event for setting the comment text.

core/block_svg.js

@@ -989,11 +989,12 @@ Blockly.BlockSvg.prototype.setCommentText = function(text, commentId,
   var changedState = false;
   if (goog.isString(text)) {
     if (!this.comment) {
-      this.comment = new Blockly.ScratchBlockComment(this, commentId,
+      this.comment = new Blockly.ScratchBlockComment(this, text, commentId,
           commentX, commentY, minimized);
       changedState = true;
+    } else {
+      this.comment.setText(/** @type {string} */ (text));
     }
-    this.comment.setText(/** @type {string} */ (text));
   } else {
     if (this.comment) {
       this.comment.dispose();

core/comment_events.js

@@ -117,12 +117,7 @@ Blockly.Events.CommentBase.prototype.fromJson = function(json) {
  */
 Blockly.Events.CommentBase.prototype.getComment_ = function() {
   var workspace = this.getEventWorkspace_();
-  if (this.blockId) {
-    var block = workspace.getBlockById(this.blockId);
-    if (block) return block.comment;
-  } else {
-    return workspace.getCommentById(this.commentId);
-  }
+  return workspace.getCommentById(this.commentId);
 };
 
 /**
@@ -202,11 +197,7 @@ Blockly.Events.CommentChange.prototype.run = function(forward) {
       }
       return;
     } else if (contents.hasOwnProperty('width') && contents.hasOwnProperty('height')) {
-      if (comment instanceof Blockly.ScratchBlockComment) {
-        comment.setBubbleSize(contents.width, contents.height);
-      } else {
-        comment.setSize(contents.width, contents.height);
-      }
+      comment.setSize(contents.width, contents.height);
       return;
     }
   }
@@ -260,7 +251,10 @@ Blockly.Events.CommentCreate = function(comment) {
    * Whether or not this comment is minimized.
    * @type {boolean}
    */
-  this.minimized = comment.minimized || false;
+  // TODO remove the instanceof check after adding minimize state to
+  // workspace comments
+  this.minimized = (comment instanceof Blockly.ScratchBlockComment &&
+      comment.isMinimized()) || false;
 
   this.xml = comment.toXmlWithXY();
 };
@@ -334,7 +328,12 @@ Blockly.Events.CommentDelete = function(comment) {
   }
   Blockly.Events.CommentDelete.superClass_.constructor.call(this, comment);
   this.xy = comment.getXY();
-  this.minimized = comment.minimized || false;
+  this.minimized = (comment instanceof Blockly.ScratchBlockComment &&
+      comment.isMinimized()) || false;
+  this.text = comment.getText();
+  var hw = comment.getHeightWidth();
+  this.height = hw.height;
+  this.width = hw.width;
 
   this.xml = comment.toXmlWithXY();
 };
@@ -382,7 +381,8 @@ Blockly.Events.CommentDelete.prototype.run = function(forward) {
     var workspace = this.getEventWorkspace_();
     if (this.blockId) {
       var block = workspace.getBlockById(this.blockId);
-      block.setCommentText('', this.commentId, this.xy.x, this.xy.y, this.minimized);
+      block.setCommentText(this.text, this.commentId, this.xy.x, this.xy.y, this.minimized);
+      block.comment.setSize(this.width, this.height);
     } else {
       var xml = goog.dom.createDom('xml');
       xml.appendChild(this.xml);

core/scratch_block_comment.js

@@ -39,6 +39,7 @@ goog.require('goog.userAgent');
 /**
  * Class for a comment.
  * @param {!Blockly.Block} block The block associated with this comment.
+ * @param {string} text The text content of this comment.
  * @param {string=} id Optional uid for comment; a new one will be generated if
  *     not provided.
  * @param {number=} x Initial x position for comment, in workspace coordinates.
@@ -48,8 +49,9 @@ goog.require('goog.userAgent');
  * @extends {Blockly.Comment}
  * @constructor
  */
-Blockly.ScratchBlockComment = function(block, id, x, y, minimized) {
+Blockly.ScratchBlockComment = function(block, text, id, x, y, minimized) {
   Blockly.ScratchBlockComment.superClass_.constructor.call(this, block);
+  this.text_ = text;
   this.x_ = x;
   this.y_ = y;
   this.isMinimized_ = minimized || false;
@@ -348,21 +350,33 @@ Blockly.ScratchBlockComment.prototype.setBubbleSize = function(width, height) {
       this.bubble_.setBubbleSize(Blockly.ScratchBlockComment.MINIMIZE_WIDTH,
           Blockly.ScratchBubble.TOP_BAR_HEIGHT);
     } else {
-      var oldWidth = this.width_;
-      var oldHeight = this.height_;
       this.bubble_.setBubbleSize(width, height);
-      this.width_ = width;
-      this.height_ = height;
-      if (oldWidth != this.width_ || oldHeight != this.height_) {
-        Blockly.Events.fire(new Blockly.Events.CommentChange(
-            this,
-            {width: oldWidth, height: oldHeight},
-            {width: this.width_, height: this.height_}));
-      }
     }
-  } else {
-    this.width_ = width;
-    this.height_ = height;
+  }
+};
+
+/**
+ * Set the un-minimized size of this comment. If the comment has an un-minimized
+ * bubble, also set the bubble's size.
+ * @param {number} width Width of the unminimized comment.
+ * @param {number} height Height of the unminimized comment.
+ */
+Blockly.ScratchBlockComment.prototype.setSize = function(width, height) {
+  var oldWidth = this.width_;
+  var oldHeight = this.height_;
+
+  if (!this.isMinimized_) {
+    this.setBubbleSize(width, height);
+  }
+
+  this.height_ = height;
+  this.width_ = width;
+
+  if (oldWidth != this.width_ || oldHeight != this.height_) {
+    Blockly.Events.fire(new Blockly.Events.CommentChange(
+        this,
+        {width: oldWidth, height: oldHeight},
+        {width: this.width_, height: this.height_}));
   }
 };
 
@@ -538,5 +552,7 @@ Blockly.ScratchBlockComment.prototype.dispose = function() {
     // information (for undoing this event)
     Blockly.Events.fire(new Blockly.Events.CommentDelete(this));
   }
-  Blockly.ScratchBlockComment.superClass_.dispose.call(this);
+  this.block_.comment = null;
+  this.workspace.removeTopComment(this);
+  Blockly.Icon.prototype.dispose.call(this);
 };

core/xml.js

@@ -705,7 +705,11 @@ Blockly.Xml.domToBlockHeadless_ = function(xmlBlock, workspace) {
         var bubbleH = parseInt(xmlChild.getAttribute('h'), 10);
         if (!isNaN(bubbleW) && !isNaN(bubbleH) &&
             block.comment && block.comment.setVisible) {
-          block.comment.setBubbleSize(bubbleW, bubbleH);
+          if (block.comment instanceof Blockly.ScratchBlockComment) {
+            block.commet.setSize(bubbleW, bubbleH);
+          } else {
+            block.comment.setBubbleSize(bubbleW, bubbleH);
+          }
         }
         break;
       case 'data':