WebUI: Fix $i18nPolymer{} strings with special characters in Polymer3.

Need to double escape backslashes, since the JS parser parses the string
before the Polymer bindings parser kicks in.

Fixed: 1063530
Change-Id: I3a61824703cd5826920e347c82468d1ddf764cac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2117162Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756424}

chrome/test/data/webui/settings/sync_account_control_test.js

@@ -12,8 +12,6 @@
 // #import {isChildVisible, isVisible} from 'chrome://test/test_util.m.js';
 // clang-format on
 
-// TODO(https://crbug.com/1063530): Remove all ignores in this file, once
-// i18nPolymer works correctly in Polymer 3.
 cr.define('settings_sync_account_control', function() {
 
   suite('SyncAccountControl', function() {
@@ -172,10 +170,10 @@ cr.define('settings_sync_account_control', function() {
       // Avatar row shows the right account.
       assertTrue(test_util.isChildVisible(testElement, '#promo-header'));
       assertTrue(test_util.isChildVisible(testElement, '#avatar-row'));
-      /* #ignore */ assertTrue(userInfo.textContent.includes('fooName'));
-      /* #ignore */ assertTrue(userInfo.textContent.includes('foo@foo.com'));
-      /* #ignore */ assertFalse(userInfo.textContent.includes('barName'));
-      /* #ignore */ assertFalse(userInfo.textContent.includes('bar@bar.com'));
+      assertTrue(userInfo.textContent.includes('fooName'));
+      assertTrue(userInfo.textContent.includes('foo@foo.com'));
+      assertFalse(userInfo.textContent.includes('barName'));
+      assertFalse(userInfo.textContent.includes('bar@bar.com'));
 
       // Menu contains the right items.
       assertTrue(!!testElement.$$('#menu'));
@@ -213,13 +211,10 @@ cr.define('settings_sync_account_control', function() {
             // email.
             items[1].click();
             Polymer.dom.flush();
-            /* #ignore */ assertFalse(
-                /* #ignore */ userInfo.textContent.includes('fooName'));
-            /* #ignore */ assertFalse(
-                /* #ignore */ userInfo.textContent.includes('foo@foo.com'));
-            /* #ignore */ assertTrue(userInfo.textContent.includes('barName'));
-            /* #ignore */ assertTrue(
-                /* #ignore */ userInfo.textContent.includes('bar@bar.com'));
+            assertFalse(userInfo.textContent.includes('fooName'));
+            assertFalse(userInfo.textContent.includes('foo@foo.com'));
+            assertTrue(userInfo.textContent.includes('barName'));
+            assertTrue(userInfo.textContent.includes('bar@bar.com'));
             assertTrue(test_util.isVisible(syncButton));
 
             browserProxy.resetResolver('startSyncingWithEmail');
@@ -263,10 +258,10 @@ cr.define('settings_sync_account_control', function() {
       }
 
       const userInfo = testElement.$$('#user-info');
-      /* #ignore */ assertTrue(userInfo.textContent.includes('barName'));
-      /* #ignore */ assertTrue(userInfo.textContent.includes('bar@bar.com'));
-      /* #ignore */ assertFalse(userInfo.textContent.includes('fooName'));
-      /* #ignore */ assertFalse(userInfo.textContent.includes('foo@foo.com'));
+      assertTrue(userInfo.textContent.includes('barName'));
+      assertTrue(userInfo.textContent.includes('bar@bar.com'));
+      assertFalse(userInfo.textContent.includes('fooName'));
+      assertFalse(userInfo.textContent.includes('foo@foo.com'));
 
       assertFalse(test_util.isChildVisible(testElement, '#sync-button'));
       assertTrue(test_util.isChildVisible(testElement, '#turn-off'));
@@ -298,9 +293,9 @@ cr.define('settings_sync_account_control', function() {
       assertTrue(!!testElement.$$('[icon="settings:sync-problem"]'));
       let displayedText =
           userInfo.querySelector('span:not([hidden])').textContent;
-      /* #ignore */ assertFalse(displayedText.includes('barName'));
-      /* #ignore */ assertFalse(displayedText.includes('fooName'));
-      /* #ignore */ assertTrue(displayedText.includes('Sync isn\'t working'));
+      assertFalse(displayedText.includes('barName'));
+      assertFalse(displayedText.includes('fooName'));
+      assertTrue(displayedText.includes('Sync isn\'t working'));
       // The sync error button is shown to resolve the error.
       assertTrue(test_util.isChildVisible(testElement, '#sync-error-button'));
 
@@ -317,9 +312,9 @@ cr.define('settings_sync_account_control', function() {
                      .classList.contains('sync-paused'));
       assertTrue(!!testElement.$$('[icon=\'settings:sync-disabled\']'));
       displayedText = userInfo.querySelector('span:not([hidden])').textContent;
-      /* #ignore */ assertFalse(displayedText.includes('barName'));
-      /* #ignore */ assertFalse(displayedText.includes('fooName'));
-      /* #ignore */ assertTrue(displayedText.includes('Sync is paused'));
+      assertFalse(displayedText.includes('barName'));
+      assertFalse(displayedText.includes('fooName'));
+      assertTrue(displayedText.includes('Sync is paused'));
       // The sync error button is shown to resolve the error.
       assertTrue(test_util.isChildVisible(testElement, '#sync-error-button'));
 
@@ -337,9 +332,9 @@ cr.define('settings_sync_account_control', function() {
                      .classList.contains('sync-disabled'));
       assertTrue(!!testElement.$$('[icon=\'cr:sync\']'));
       displayedText = userInfo.querySelector('span:not([hidden])').textContent;
-      /* #ignore */ assertFalse(displayedText.includes('barName'));
-      /* #ignore */ assertFalse(displayedText.includes('fooName'));
-      /* #ignore */ assertTrue(displayedText.includes('Sync disabled'));
+      assertFalse(displayedText.includes('barName'));
+      assertFalse(displayedText.includes('fooName'));
+      assertTrue(displayedText.includes('Sync disabled'));
       assertFalse(test_util.isChildVisible(testElement, '#sync-error-button'));
 
       testElement.syncStatus = {
@@ -355,9 +350,9 @@ cr.define('settings_sync_account_control', function() {
                      .classList.contains('sync-problem'));
       assertTrue(!!testElement.$$('[icon="settings:sync-problem"]'));
       displayedText = userInfo.querySelector('span:not([hidden])').textContent;
-      /* #ignore */ assertFalse(displayedText.includes('barName'));
-      /* #ignore */ assertFalse(displayedText.includes('fooName'));
-      /* #ignore */ assertTrue(displayedText.includes('Sync isn\'t working'));
+      assertFalse(displayedText.includes('barName'));
+      assertFalse(displayedText.includes('fooName'));
+      assertTrue(displayedText.includes('Sync isn\'t working'));
 
       testElement.syncStatus = {
         firstSetupInProgress: false,
@@ -373,11 +368,10 @@ cr.define('settings_sync_account_control', function() {
                      .classList.contains('sync-problem'));
       assertTrue(!!testElement.$$('[icon="settings:sync-problem"]'));
       displayedText = userInfo.querySelector('span:not([hidden])').textContent;
-      /* #ignore */ assertFalse(displayedText.includes('barName'));
-      /* #ignore */ assertFalse(displayedText.includes('fooName'));
-      /* #ignore */ assertFalse(displayedText.includes('Sync isn\'t working'));
-      /* #ignore */ assertTrue(
-          /* #ignore */ displayedText.includes('Error syncing passwords'));
+      assertFalse(displayedText.includes('barName'));
+      assertFalse(displayedText.includes('fooName'));
+      assertFalse(displayedText.includes('Sync isn\'t working'));
+      assertTrue(displayedText.includes('Error syncing passwords'));
       // The sync error button is shown to resolve the error.
       assertTrue(test_util.isChildVisible(testElement, '#sync-error-button'));
       assertTrue(test_util.isChildVisible(testElement, '#turn-off'));
@@ -398,9 +392,8 @@ cr.define('settings_sync_account_control', function() {
       const userInfo = testElement.$$('#user-info');
       const setupButtons = testElement.$$('#setup-buttons');
 
-      /* #ignore */ assertTrue(userInfo.textContent.includes('barName'));
-      /* #ignore */ assertTrue(
-          /* #ignore */ userInfo.textContent.includes('Setup in progress...'));
+      assertTrue(userInfo.textContent.includes('barName'));
+      assertTrue(userInfo.textContent.includes('Setup in progress...'));
       assertTrue(test_util.isVisible(setupButtons));
     });
 

ui/base/template_expressions.cc

@@ -65,22 +65,23 @@ HtmlTemplate FindHtmlTemplate(const base::StringPiece& source) {
 }
 
 // Escape quotes and backslashes ('"\).
-std::string PolymerParameterEscape(const std::string& in_string) {
+std::string PolymerParameterEscape(const std::string& in_string,
+                                   bool is_javascript) {
   std::string out;
   out.reserve(in_string.size() * 2);
   for (const char c : in_string) {
     switch (c) {
       case '\\':
-        out.append("\\\\");
+        out.append(is_javascript ? R"(\\\\)" : R"(\\)");
         break;
       case '\'':
-        out.append("\\'");
+        out.append(is_javascript ? R"(\\')" : R"(\')");
         break;
       case '"':
         out.append(""");
         break;
       case ',':
-        out.append("\\\\,");
+        out.append(is_javascript ? R"(\\,)" : R"(\,)");
         break;
       default:
         out += c;
@@ -182,7 +183,7 @@ bool ReplaceTemplateExpressionsInternal(
       // Pass the replacement through unchanged.
     } else if (context == "Polymer") {
       // Escape quotes and backslash for '$i18nPolymer{}' use (i.e. quoted).
-      replacement = PolymerParameterEscape(replacement);
+      replacement = PolymerParameterEscape(replacement, is_javascript);
     } else {
       CHECK(false) << "Unknown context " << context;
     }

ui/base/template_expressions_unittest.cc

@@ -76,14 +76,14 @@ TEST(TemplateExpressionsTest, ReplaceTemplateExpressionsPolymerQuoting) {
 
 TEST(TemplateExpressionsTest, ReplaceTemplateExpressionsPolymerMixed) {
   static TemplateReplacements substitutions;
-  substitutions["punctuationSample"] = "a\"b'c<d>e&f,g";
+  substitutions["punctuationSample"] = R"(a"b'c<d>e&f,g)";
   substitutions["htmlSample"] = "<div>hello</div>";
-  EXPECT_EQ("a&quot;b\\'c<d>e&f\\\\,g",
+  EXPECT_EQ(R"(a&quot;b\'c<d>e&f\,g)",
             ReplaceTemplateExpressions("$i18nPolymer{punctuationSample}",
                                        substitutions));
   EXPECT_EQ("<div>hello</div>", ReplaceTemplateExpressions(
                                     "$i18nPolymer{htmlSample}", substitutions));
-  EXPECT_EQ("multiple: <div>hello</div>, a&quot;b\\'c<d>e&f\\\\,g.",
+  EXPECT_EQ(R"(multiple: <div>hello</div>, a&quot;b\'c<d>e&f\,g.)",
             ReplaceTemplateExpressions("multiple: $i18nPolymer{htmlSample}, "
                                        "$i18nPolymer{punctuationSample}.",
                                        substitutions));
@@ -561,4 +561,33 @@ TEST(TemplateExpressionsTest, JSMultipleTemplates) {
   }
 }
 
+TEST(TemplateExpressionsTest, ReplaceTemplateExpressionsPolymerQuotingJS) {
+  static TemplateReplacements substitutions;
+  substitutions["quotesAndCommas"] = R"(don't "moo", they said)";
+  substitutions["backslashes"] = R"(\ \\ \n \r \t \b \f \0)";
+  const TestCase kTestCases[] = {
+      // Case: quotesAndCommas
+      {R"(<!--_html_template_start_-->
+       <div>[[Call('$i18nPolymer{quotesAndCommas}')]]</div>
+       <!--_html_template_end_-->)",
+       R"(<!--_html_template_start_-->
+       <div>[[Call('don\\'t &quot;moo&quot;\\, they said')]]</div>
+       <!--_html_template_end_-->)"},
+      // Case: backslashes
+      {R"(<!--_html_template_start_-->
+       <div>[[Call('$i18nPolymer{backslashes}')]]</div>
+       <!--_html_template_end_-->)",
+       R"(<!--_html_template_start_-->
+       <div>[[Call('\\\\ \\\\\\\\ \\\\n \\\\r \\\\t \\\\b \\\\f \\\\0')]]</div>
+       <!--_html_template_end_-->)"},
+  };
+  std::string formatted;
+  for (const TestCase test_case : kTestCases) {
+    ASSERT_TRUE(ReplaceTemplateExpressionsInJS(test_case.js_in, substitutions,
+                                               &formatted));
+    EXPECT_EQ(test_case.expected_out, formatted);
+    formatted.clear();
+  }
+}
+
 }  // namespace ui