From 640fb9c733c1a1a5c53dec028b2289b85c4a3b06 Mon Sep 17 00:00:00 2001 From: Andrew n marshall Date: Mon, 13 Feb 2017 15:12:57 -0800 Subject: [PATCH 1/4] Dereference string table references when loading variable fields from JSON. --- core/block.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/block.js b/core/block.js index 18ff384cf..ddbe1b847 100644 --- a/core/block.js +++ b/core/block.js @@ -1169,7 +1169,9 @@ Blockly.Block.prototype.interpolate_ = function(message, args, lastDummyAlign) { field = new Blockly.FieldColour(element['colour']); break; case 'field_variable': - field = new Blockly.FieldVariable(element['variable']); + var varname = + Blockly.utils.replaceMessageReferences(element['variable']); + field = new Blockly.FieldVariable(varname); break; case 'field_dropdown': field = new Blockly.FieldDropdown(element['options']); From dc0d3beba36e33c0ee0922a6bbbebcc68799636a Mon Sep 17 00:00:00 2001 From: Andrew n marshall Date: Mon, 13 Feb 2017 16:01:14 -0800 Subject: [PATCH 2/4] Moving FieldImage string dereferencing back into Block.interpolate_() (part of jsonInit()). This sets a clear boundary of where dereferencing should happen. Towards this, I've added message dereferencing for other field types here, as well. I've used a pattern of field-type specific helper functions. --- core/block.js | 78 +++++++++++++++++++++++++++++++++++++++------ core/field_image.js | 16 ++-------- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/core/block.js b/core/block.js index ddbe1b847..468a32a3a 100644 --- a/core/block.js +++ b/core/block.js @@ -1150,13 +1150,10 @@ Blockly.Block.prototype.interpolate_ = function(message, args, lastDummyAlign) { input = this.appendDummyInput(element['name']); break; case 'field_label': - field = new Blockly.FieldLabel(element['text'], element['class']); + field = Blockly.Block.newFieldLabelFromJson_(element); break; case 'field_input': - field = new Blockly.FieldTextInput(element['text']); - if (typeof element['spellcheck'] == 'boolean') { - field.setSpellcheck(element['spellcheck']); - } + field = Blockly.Block.newFieldTextInputFromJson_(element); break; case 'field_angle': field = new Blockly.FieldAngle(element['angle']); @@ -1169,16 +1166,13 @@ Blockly.Block.prototype.interpolate_ = function(message, args, lastDummyAlign) { field = new Blockly.FieldColour(element['colour']); break; case 'field_variable': - var varname = - Blockly.utils.replaceMessageReferences(element['variable']); - field = new Blockly.FieldVariable(varname); + field = Blockly.Block.newFieldVariableFromJson_(element); break; case 'field_dropdown': field = new Blockly.FieldDropdown(element['options']); break; case 'field_image': - field = new Blockly.FieldImage(element['src'], - element['width'], element['height'], element['alt']); + field = Blockly.Block.newFieldImageFromJson_(element); break; case 'field_number': field = new Blockly.FieldNumber(element['value'], @@ -1217,6 +1211,70 @@ Blockly.Block.prototype.interpolate_ = function(message, args, lastDummyAlign) { } }; +/** + * Helper function to construct a FieldImage from a JSON arg object, + * dereferncing any string table references. + * @param {!Object} options A set of options (src, width, height, and alt). + * @returns {!Blockly.FieldImage} The new image. + */ +Blockly.Block.newFieldImageFromJson_ = function(options) { + var src = Blockly.utils.replaceMessageReferences(options['src']); + var width = options['width']; + if (goog.isString(width)) { + width = Number(Blockly.utils.replaceMessageReferences(width)); + } + var height = options['height']; + if (goog.isString(height)) { + height = Number(Blockly.utils.replaceMessageReferences(height)); + } + var alt = options['alt']; + if (goog.isString(alt)) { + alt = Blockly.utils.replaceMessageReferences(alt); + } + return new Blockly.FieldImage(src, width, height, alt); +}; + +/** + * Helper function to construct a FieldLabel from a JSON arg object, + * dereferncing any string table references. + * @param {!Object} options A set of options (text, and class). + * @returns {!Blockly.FieldImage} The new image. + */ +Blockly.Block.newFieldLabelFromJson_ = function(options) { + var text = Blockly.utils.replaceMessageReferences(options['text']); + return new Blockly.FieldLabel(text, options['class']); +}; + +/** + * Helper function to construct a FieldTextInput from a JSON arg object, + * dereferncing any string table references. + * @param {!Object} options A set of options (text, class, and spellcheck). + * @returns {!Blockly.FieldImage} The new image. + */ +Blockly.Block.newFieldTextInputFromJson_ = function(options) { + var text = Blockly.utils.replaceMessageReferences(options['text']); + var field = new Blockly.FieldTextInput(text, options['class']); + if (typeof options['spellcheck'] == 'boolean') { + field.setSpellcheck(options['spellcheck']); + } + return field; +}; + +/** + * Helper function to construct a FieldVariable from a JSON arg object, + * dereferncing any string table references. + * @param {!Object} options A set of options (variable). + * @returns {!Blockly.FieldImage} The new image. + */ +Blockly.Block.newFieldVariableFromJson_ = function(options) { + var varname = options['variable']; + if (goog.isString(varname)) { + varname = Blockly.utils.replaceMessageReferences(varname); + } + return new Blockly.FieldVariable(varname);; +}; + + /** * Add a value input, statement input or local variable to this block. * @param {number} type Either Blockly.INPUT_VALUE or Blockly.NEXT_STATEMENT or diff --git a/core/field_image.js b/core/field_image.js index e91f00070..028905922 100644 --- a/core/field_image.js +++ b/core/field_image.js @@ -35,8 +35,8 @@ goog.require('goog.userAgent'); /** * Class for an image on a block. * @param {string} src The URL of the image. - * @param {number|string} width Width of the image, possibly via reference. - * @param {number|string} height Height of the image, possibly via reference. + * @param {number} width Width of the image. + * @param {number} height Height of the image. * @param {string=} opt_alt Optional alt text for when block is collapsed. * @extends {Blockly.Field} * @constructor @@ -44,18 +44,6 @@ goog.require('goog.userAgent'); Blockly.FieldImage = function(src, width, height, opt_alt) { this.sourceBlock_ = null; - // Replace any message references in the arguments. - src = Blockly.utils.replaceMessageReferences(src); - if (goog.isString(height)) { - height = Blockly.utils.replaceMessageReferences(height); - } - if (goog.isString(width)) { - width = Blockly.utils.replaceMessageReferences(width); - } - if (goog.isString(opt_alt)) { - opt_alt = Blockly.utils.replaceMessageReferences(opt_alt); - } - // Ensure height and width are numbers. Strings are bad at math. this.height_ = Number(height); this.width_ = Number(width); From 750c0300ec8a1cefb09c6823005051e0d1a649a3 Mon Sep 17 00:00:00 2001 From: Andrew n marshall Date: Tue, 14 Feb 2017 09:47:18 -0800 Subject: [PATCH 3/4] Addressing comments. --- core/block.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/core/block.js b/core/block.js index 468a32a3a..e68f5db5c 100644 --- a/core/block.js +++ b/core/block.js @@ -1213,9 +1213,10 @@ Blockly.Block.prototype.interpolate_ = function(message, args, lastDummyAlign) { /** * Helper function to construct a FieldImage from a JSON arg object, - * dereferncing any string table references. - * @param {!Object} options A set of options (src, width, height, and alt). + * dereferencing any string table references. + * @param {!Object} options A JSON object with options (src, width, height, and alt). * @returns {!Blockly.FieldImage} The new image. + * @private */ Blockly.Block.newFieldImageFromJson_ = function(options) { var src = Blockly.utils.replaceMessageReferences(options['src']); @@ -1236,9 +1237,10 @@ Blockly.Block.newFieldImageFromJson_ = function(options) { /** * Helper function to construct a FieldLabel from a JSON arg object, - * dereferncing any string table references. - * @param {!Object} options A set of options (text, and class). + * dereferencing any string table references. + * @param {!Object} options A JSON object with options (text, and class). * @returns {!Blockly.FieldImage} The new image. + * @private */ Blockly.Block.newFieldLabelFromJson_ = function(options) { var text = Blockly.utils.replaceMessageReferences(options['text']); @@ -1247,9 +1249,11 @@ Blockly.Block.newFieldLabelFromJson_ = function(options) { /** * Helper function to construct a FieldTextInput from a JSON arg object, - * dereferncing any string table references. - * @param {!Object} options A set of options (text, class, and spellcheck). + * dereferencing any string table references. + * @param {!Object} options A JSON object with options (text, class, and + * spellcheck). * @returns {!Blockly.FieldImage} The new image. + * @private */ Blockly.Block.newFieldTextInputFromJson_ = function(options) { var text = Blockly.utils.replaceMessageReferences(options['text']); @@ -1262,16 +1266,17 @@ Blockly.Block.newFieldTextInputFromJson_ = function(options) { /** * Helper function to construct a FieldVariable from a JSON arg object, - * dereferncing any string table references. - * @param {!Object} options A set of options (variable). + * dereferencing any string table references. + * @param {!Object} options A JSON object with options (variable). * @returns {!Blockly.FieldImage} The new image. + * @private */ Blockly.Block.newFieldVariableFromJson_ = function(options) { var varname = options['variable']; if (goog.isString(varname)) { varname = Blockly.utils.replaceMessageReferences(varname); } - return new Blockly.FieldVariable(varname);; + return new Blockly.FieldVariable(varname); }; From afd1fdeb1537e60c5ceefbfe8cc9c1877fb0414d Mon Sep 17 00:00:00 2001 From: Andrew n marshall Date: Tue, 14 Feb 2017 10:59:31 -0800 Subject: [PATCH 4/4] .utils.replaceMessageReferences(..) now gracefully returns non-string arguments. --- core/block.js | 21 +++++---------------- core/utils.js | 14 +++++++++----- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/core/block.js b/core/block.js index e68f5db5c..4ad3117b0 100644 --- a/core/block.js +++ b/core/block.js @@ -1220,18 +1220,10 @@ Blockly.Block.prototype.interpolate_ = function(message, args, lastDummyAlign) { */ Blockly.Block.newFieldImageFromJson_ = function(options) { var src = Blockly.utils.replaceMessageReferences(options['src']); - var width = options['width']; - if (goog.isString(width)) { - width = Number(Blockly.utils.replaceMessageReferences(width)); - } - var height = options['height']; - if (goog.isString(height)) { - height = Number(Blockly.utils.replaceMessageReferences(height)); - } - var alt = options['alt']; - if (goog.isString(alt)) { - alt = Blockly.utils.replaceMessageReferences(alt); - } + var width = Number(Blockly.utils.replaceMessageReferences(options['width'])); + var height = + Number(Blockly.utils.replaceMessageReferences(options['height'])); + var alt = Blockly.utils.replaceMessageReferences(options['alt']); return new Blockly.FieldImage(src, width, height, alt); }; @@ -1272,10 +1264,7 @@ Blockly.Block.newFieldTextInputFromJson_ = function(options) { * @private */ Blockly.Block.newFieldVariableFromJson_ = function(options) { - var varname = options['variable']; - if (goog.isString(varname)) { - varname = Blockly.utils.replaceMessageReferences(varname); - } + var varname = Blockly.utils.replaceMessageReferences(options['variable']); return new Blockly.FieldVariable(varname); }; diff --git a/core/utils.js b/core/utils.js index 5066c34f4..b4928044e 100644 --- a/core/utils.js +++ b/core/utils.js @@ -259,7 +259,7 @@ Blockly.utils.getRelativeXY.XY_2D_REGEX_ = * context (scale...). * @return {!SVGElement} Newly created SVG element. */ -Blockly.utils.createSvgElement = function(name, attrs, parent, opt_workspace) { +Blockly.utils.createSvgElement = function(name, attrs, parent /*, opt_workspace */) { var e = /** @type {!SVGElement} */ ( document.createElementNS(Blockly.SVG_NS, name)); for (var key in attrs) { @@ -409,13 +409,17 @@ Blockly.utils.tokenizeInterpolation = function(message) { }; /** - * Replaces string table references in a message string. For example, - * %{bky_my_msg} and %{BKY_MY_MSG} will both be replaced with the value in - * Blockly.Msg['MY_MSG']. - * @param {string} message Text which might contain string table references. + * Replaces string table references in a message, if the message is a string. + * For example, "%{bky_my_msg}" and "%{BKY_MY_MSG}" will both be replaced with + * the value in Blockly.Msg['MY_MSG']. + * @param {string|?} message Message, which may be a string that contains + * string table references. * @return {!string} String with message references replaced. */ Blockly.utils.replaceMessageReferences = function(message) { + if (!goog.isString(message)) { + return message; + } var interpolatedResult = Blockly.utils.tokenizeInterpolation_(message, false); // When parseInterpolationTokens == false, interpolatedResult should be at // most length 1.