From f992ea0f9cc3639ccee96fa8a92056603a662201 Mon Sep 17 00:00:00 2001 From: Andrew n marshall Date: Tue, 24 Oct 2017 13:02:14 -0700 Subject: [PATCH] Fix #1369: resource loading order constraints, interpolation tokens in message expansion (PR #1383) This fixes race condition in #1369 by using message references instead of explicit string lookups. This required fixing a bug the token interpolation parser that was breaking on the inner '"%1"' for these tooltips. --- blocks/loops.js | 4 ++-- blocks/math.js | 2 +- core/utils.js | 24 +++++++++++++++-------- tests/jsunit/utils_test.js | 40 ++++++++++++++++++++++++++------------ 4 files changed, 47 insertions(+), 23 deletions(-) diff --git a/blocks/loops.js b/blocks/loops.js index 48d155ac5..b93147c4c 100644 --- a/blocks/loops.js +++ b/blocks/loops.js @@ -281,11 +281,11 @@ Blockly.Extensions.registerMixin('contextMenu_newGetVariableBlock', Blockly.Extensions.register('controls_for_tooltip', Blockly.Extensions.buildTooltipWithFieldValue( - Blockly.Msg.CONTROLS_FOR_TOOLTIP, 'VAR')); + '%{BKY_CONTROLS_FOR_TOOLTIP}', 'VAR')); Blockly.Extensions.register('controls_forEach_tooltip', Blockly.Extensions.buildTooltipWithFieldValue( - Blockly.Msg.CONTROLS_FOREACH_TOOLTIP, 'VAR')); + '%{BKY_CONTROLS_FOREACH_TOOLTIP}', 'VAR')); /** * This mixin adds a check to make sure the 'controls_flow_statements' block diff --git a/blocks/math.js b/blocks/math.js index 8244618f9..bc745dcb7 100644 --- a/blocks/math.js +++ b/blocks/math.js @@ -506,7 +506,7 @@ Blockly.Constants.Math.CHANGE_TOOLTIP_EXTENSION = function() { Blockly.Extensions.register('math_change_tooltip', Blockly.Extensions.buildTooltipWithFieldValue( - Blockly.Msg.MATH_CHANGE_TOOLTIP, 'VAR')); + '%{BKY_MATH_CHANGE_TOOLTIP}', 'VAR')); /** * Mixin with mutator methods to support alternate output based if the diff --git a/core/utils.js b/core/utils.js index a8ac9117a..6d1d0c482 100644 --- a/core/utils.js +++ b/core/utils.js @@ -457,22 +457,29 @@ Blockly.utils.replaceMessageReferences = function(message) { }; /** - * Validates that any %{BKY_...} references in the message refer to keys of + * Validates that any %{MSG_KEY} references in the message refer to keys of * the Blockly.Msg string table. * @param {string} message Text which might contain string table references. * @return {boolean} True if all message references have matching values. * Otherwise, false. */ Blockly.utils.checkMessageReferences = function(message) { - var isValid = true; // True until a bad reference is found. + var validSoFar = true; - var regex = /%{BKY_([a-zA-Z][a-zA-Z0-9_]*)}/g; + var msgTable = Blockly.utils.getMessageArray_(); + + // TODO(#1169): Implement support for other string tables, prefixes other than BKY_. + var regex = /%{(BKY_[A-Z][A-Z0-9_]*)}/gi; var match = regex.exec(message); while (match) { var msgKey = match[1]; - if (Blockly.utils.getMessageArray_()[msgKey] == undefined) { - console.log('WARNING: No message string for %{BKY_' + msgKey + '}.'); - isValid = false; + msgKey = msgKey.toUpperCase(); + if (msgKey.substr(0, 4) != 'BKY_') { + console.log('WARNING: Unsupported message table prefix in %{' + match[1] + '}.'); + validSoFar = false; // Continue to report other errors. + } else if (msgTable[msgKey.substr(4)] == undefined) { + console.log('WARNING: No message string for %{' + match[1] + '}.'); + validSoFar = false; // Continue to report other errors. } // Re-run on remainder of string. @@ -480,7 +487,7 @@ Blockly.utils.checkMessageReferences = function(message) { match = regex.exec(message); } - return isValid; + return validSoFar; }; /** @@ -569,7 +576,8 @@ Blockly.utils.tokenizeInterpolation_ = function(message, if (goog.isString(rawValue)) { // Attempt to dereference substrings, too, appending to the end. Array.prototype.push.apply(tokens, - Blockly.utils.tokenizeInterpolation(rawValue)); + Blockly.utils.tokenizeInterpolation_( + rawValue, parseInterpolationTokens)); } else if (parseInterpolationTokens) { // When parsing interpolation tokens, numbers are special // placeholders (%1, %2, etc). Make sure all other values are diff --git a/tests/jsunit/utils_test.js b/tests/jsunit/utils_test.js index 4edb83fe2..98ffabd7f 100644 --- a/tests/jsunit/utils_test.js +++ b/tests/jsunit/utils_test.js @@ -192,22 +192,13 @@ function test_tokenizeInterpolation() { function test_replaceMessageReferences() { Blockly.Msg = Blockly.Msg || {}; Blockly.Msg.STRING_REF = 'test string'; + Blockly.Msg.SUBREF = 'subref'; + Blockly.Msg.STRING_REF_WITH_ARG = 'test %1 string'; + Blockly.Msg.STRING_REF_WITH_SUBREF = 'test %{bky_subref} string'; var resultString = Blockly.utils.replaceMessageReferences(''); assertEquals('Empty string produces empty string', '', resultString); - resultString = Blockly.utils.replaceMessageReferences('%{bky_string_ref}'); - assertEquals('Message ref dereferenced.', 'test string', resultString); - resultString = Blockly.utils.replaceMessageReferences('before %{bky_string_ref} after'); - assertEquals('Message ref dereferenced.', 'before test string after', resultString); - - resultString = Blockly.utils.replaceMessageReferences('%1'); - assertEquals('Interpolation tokens ignored.', '%1', resultString); - resultString = Blockly.utils.replaceMessageReferences('%1 %2'); - assertEquals('Interpolation tokens ignored.', '%1 %2', resultString); - resultString = Blockly.utils.replaceMessageReferences('before %1 after'); - assertEquals('Interpolation tokens ignored.', 'before %1 after', resultString); - resultString = Blockly.utils.replaceMessageReferences('%%'); assertEquals('Escaped %', '%', resultString); resultString = Blockly.utils.replaceMessageReferences('%%{bky_string_ref}'); @@ -215,4 +206,29 @@ function test_replaceMessageReferences() { resultString = Blockly.utils.replaceMessageReferences('%a'); assertEquals('Unrecognized % escape code treated as literal', '%a', resultString); + + resultString = Blockly.utils.replaceMessageReferences('%1'); + assertEquals('Interpolation tokens ignored.', '%1', resultString); + resultString = Blockly.utils.replaceMessageReferences('%1 %2'); + assertEquals('Interpolation tokens ignored.', '%1 %2', resultString); + resultString = Blockly.utils.replaceMessageReferences('before %1 after'); + assertEquals('Interpolation tokens ignored.', 'before %1 after', resultString); + + // Blockly.Msg.STRING_REF cases: + resultString = Blockly.utils.replaceMessageReferences('%{bky_string_ref}'); + assertEquals('Message ref dereferenced.', 'test string', resultString); + resultString = Blockly.utils.replaceMessageReferences('before %{bky_string_ref} after'); + assertEquals('Message ref dereferenced.', 'before test string after', resultString); + + // Blockly.Msg.STRING_REF_WITH_ARG cases: + resultString = Blockly.utils.replaceMessageReferences('%{bky_string_ref_with_arg}'); + assertEquals('Message ref dereferenced with argument preserved.', 'test %1 string', resultString); + resultString = Blockly.utils.replaceMessageReferences('before %{bky_string_ref_with_arg} after'); + assertEquals('Message ref dereferenced with argument preserved.', 'before test %1 string after', resultString); + + // Blockly.Msg.STRING_REF_WITH_SUBREF cases: + resultString = Blockly.utils.replaceMessageReferences('%{bky_string_ref_with_subref}'); + assertEquals('Message ref and subref dereferenced.', 'test subref string', resultString); + resultString = Blockly.utils.replaceMessageReferences('before %{bky_string_ref_with_subref} after'); + assertEquals('Message ref and subref dereferenced.', 'before test subref string after', resultString); }