From aed6b131bbc36eb438e2c6e29b657a36101baba2 Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Mon, 29 Jun 2020 17:50:21 -0700
Subject: [PATCH] fix(core): handle spaces after `select` and `plural` ICU
 keywords (#37866)

Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g. `{count, select , ...}`), these spaces are also included into the key names in ICU vars (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be converted into `_` symbols while normalizing placeholder names, thus causing mismatches at runtime (i.e. placeholder will not be replaced with the correct value). This commit updates the code to trim these spaces while generating an object with placeholders, to make sure the runtime logic can replace these placeholders with the right values.

PR Close #37866
---
 .../compliance/r3_view_compiler_i18n_spec.ts  | 33 +++++++++++++++++++
 .../src/render3/r3_template_transform.ts      | 12 ++++++-
 packages/core/test/acceptance/i18n_spec.ts    | 18 ++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
index 05dad886e2..2755e0b57a 100644
--- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
+++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
@@ -3661,6 +3661,39 @@ describe('i18n support in the template compiler', () => {
 
       verify(input, output);
     });
+
+    it('should produce proper messages when `select` or `plural` keywords have spaces after them',
+       () => {
+         const input = `
+            <div i18n>
+              {count, select , 1 {one} other {more than one}}
+              {count, plural , =1 {one} other {more than one}}
+            </div>
+          `;
+
+         const output = String.raw`
+            var $I18N_1$;
+            if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
+                const $MSG_EXTERNAL_199763560911211963$$APP_SPEC_TS_2$ = goog.getMsg("{VAR_SELECT , select , 1 {one} other {more than one}}");
+                $I18N_1$ = $MSG_EXTERNAL_199763560911211963$$APP_SPEC_TS_2$;
+            }
+            else {
+                $I18N_1$ = $localize \`{VAR_SELECT , select , 1 {one} other {more than one}}\`;
+            }
+            $I18N_1$ = i0.ɵɵi18nPostprocess($I18N_1$, { "VAR_SELECT": "\uFFFD0\uFFFD" });
+            var $I18N_3$;
+            if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
+                const $MSG_EXTERNAL_3383986062053865025$$APP_SPEC_TS_4$ = goog.getMsg("{VAR_PLURAL , plural , =1 {one} other {more than one}}");
+                $I18N_3$ = $MSG_EXTERNAL_3383986062053865025$$APP_SPEC_TS_4$;
+            }
+            else {
+                $I18N_3$ = $localize \`{VAR_PLURAL , plural , =1 {one} other {more than one}}\`;
+            }
+            $I18N_3$ = i0.ɵɵi18nPostprocess($I18N_3$, { "VAR_PLURAL": "\uFFFD1\uFFFD" });
+          `;
+
+         verify(input, output);
+       });
   });
 
   describe('$localize legacy message ids', () => {
diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts
index e91502af63..7d1ffeba28 100644
--- a/packages/compiler/src/render3/r3_template_transform.ts
+++ b/packages/compiler/src/render3/r3_template_transform.ts
@@ -273,10 +273,20 @@ class HtmlAstToIvyAst implements html.Visitor {
       const value = message.placeholders[key];
       if (key.startsWith(I18N_ICU_VAR_PREFIX)) {
         const config = this.bindingParser.interpolationConfig;
+
         // ICU expression is a plain string, not wrapped into start
         // and end tags, so we wrap it before passing to binding parser
         const wrapped = `${config.start}${value}${config.end}`;
-        vars[key] = this._visitTextWithInterpolation(wrapped, expansion.sourceSpan) as t.BoundText;
+
+        // Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g.
+        // `{count, select , ...}`), these spaces are also included into the key names in ICU vars
+        // (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be
+        // converted into `_` symbols while normalizing placeholder names, which might lead to
+        // mismatches at runtime (i.e. placeholder will not be replaced with the correct value).
+        const formattedKey = key.trim();
+
+        vars[formattedKey] =
+            this._visitTextWithInterpolation(wrapped, expansion.sourceSpan) as t.BoundText;
       } else {
         placeholders[key] = this._visitTextWithInterpolation(value, expansion.sourceSpan);
       }
diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts
index 796f55756e..cbca1e1246 100644
--- a/packages/core/test/acceptance/i18n_spec.ts
+++ b/packages/core/test/acceptance/i18n_spec.ts
@@ -627,6 +627,24 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
       expect(element.textContent).toContain('ICU start --> Autre <-- ICU end');
     });
 
+    it('when `select` or `plural` keywords have spaces after them', () => {
+      loadTranslations({
+        [computeMsgId('{VAR_SELECT , select , 10 {ten} 20 {twenty} other {other}}')]:
+            '{VAR_SELECT , select , 10 {dix} 20 {vingt} other {autre}}',
+        [computeMsgId('{VAR_PLURAL , plural , =0 {zero} =1 {one} other {other}}')]:
+            '{VAR_PLURAL , plural , =0 {zéro} =1 {une} other {autre}}'
+      });
+      const fixture = initWithTemplate(AppComp, `
+        <div i18n>
+          {count, select , 10 {ten} 20 {twenty} other {other}} -
+          {count, plural , =0 {zero} =1 {one} other {other}}
+        </div>
+      `);
+
+      const element = fixture.nativeElement;
+      expect(element.textContent).toContain('autre - zéro');
+    });
+
     it('with no root node and text and DOM nodes surrounding ICU', () => {
       loadTranslations({
         [computeMsgId('{VAR_SELECT, select, 10 {Ten} 20 {Twenty} other {Other}}')]: