From 62f7d0fe5c4052feeecabe311d04c28f223297b4 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 15 Nov 2019 16:25:32 +0000 Subject: [PATCH] fix(ivy): i18n - ensure that colons in i18n metadata are not rendered (#33820) The `:` char is used as a metadata marker in `$localize` messages. If this char appears in the metadata it must be escaped, as `\:`. Previously, although the `:` char was being escaped, the TS AST being generated was not correct and so it was being output double escaped, which meant that it appeared in the rendered message. As of TS 3.6.2 the "raw" string can be specified when creating tagged template AST nodes, so it is possible to correct this. PR Close #33820 --- .../src/app/app.component.html | 2 +- .../ivy-i18n/src/app/app.component.html | 2 +- .../src/ngtsc/translator/src/translator.ts | 4 +- .../compliance/r3_view_compiler_i18n_spec.ts | 2 +- .../compiler/src/output/abstract_emitter.ts | 2 +- .../compiler/src/render3/view/i18n/meta.ts | 15 ++++--- .../compiler/test/render3/view/i18n_spec.ts | 44 ++++++++++++------- 7 files changed, 44 insertions(+), 27 deletions(-) diff --git a/integration/cli-hello-world-ivy-i18n/src/app/app.component.html b/integration/cli-hello-world-ivy-i18n/src/app/app.component.html index 4e22afcce0..b181126918 100644 --- a/integration/cli-hello-world-ivy-i18n/src/app/app.component.html +++ b/integration/cli-hello-world-ivy-i18n/src/app/app.component.html @@ -1,4 +1,4 @@ -

Hello {{ title }}!

+

Hello {{ title }}!

{{ locale }}

{{ 1 | percent }} awesome

{{ jan | date : 'LLLL' }}

diff --git a/integration/ivy-i18n/src/app/app.component.html b/integration/ivy-i18n/src/app/app.component.html index 7bee9429e1..eb7616861d 100644 --- a/integration/ivy-i18n/src/app/app.component.html +++ b/integration/ivy-i18n/src/app/app.component.html @@ -1,6 +1,6 @@
-

+

Hello {{ title }}!

diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index c4462fdbbc..63f43730c3 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -549,9 +549,9 @@ function visitLocalizedString(ast: LocalizedString, context: Context, visitor: E let template: ts.TemplateLiteral; const metaBlock = serializeI18nHead(ast.metaBlock, ast.messageParts[0]); if (ast.messageParts.length === 1) { - template = ts.createNoSubstitutionTemplateLiteral(metaBlock); + template = ts.createNoSubstitutionTemplateLiteral(metaBlock.cooked, metaBlock.raw); } else { - const head = ts.createTemplateHead(metaBlock); + const head = ts.createTemplateHead(metaBlock.cooked, metaBlock.raw); const spans: ts.TemplateSpan[] = []; for (let i = 1; i < ast.messageParts.length; i++) { const resolvedExpression = ast.expressions[i - 1].visitExpression(visitor, context); 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 bcb1f76c97..9614315155 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 @@ -264,7 +264,7 @@ describe('i18n support in the template compiler', () => { $I18N_23$ = $MSG_EXTERNAL_idG$$APP_SPEC_TS_24$; } else { - $I18N_23$ = $localize \`:[BACKUP_MESSAGE_ID\\:idH]desc@@idG:Title G\`; + $I18N_23$ = $localize \`:[BACKUP_MESSAGE_ID\:idH]desc@@idG:Title G\`; } const $_c25$ = ["title", $I18N_23$]; … diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index b5671ffe81..9c505c9d18 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -364,7 +364,7 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex visitLocalizedString(ast: o.LocalizedString, ctx: EmitterVisitorContext): any { const head = serializeI18nHead(ast.metaBlock, ast.messageParts[0]); - ctx.print(ast, '$localize `' + escapeBackticks(head)); + ctx.print(ast, '$localize `' + escapeBackticks(head.raw)); for (let i = 1; i < ast.messageParts.length; i++) { ctx.print(ast, '${'); ast.expressions[i - 1].visitExpression(this, ctx); diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index 2e10fe02f4..0b78536955 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -235,13 +235,15 @@ export function parseI18nMeta(meta?: string): I18nMeta { } /** - * Serialize the given `meta` and `messagePart` a string that can be used in a `$localize` - * tagged string. The format of the metadata is the same as that parsed by `parseI18nMeta()`. + * Serialize the given `meta` and `messagePart` "cooked" and "raw" strings that can be used in a + * `$localize` tagged string. The format of the metadata is the same as that parsed by + * `parseI18nMeta()`. * * @param meta The metadata to serialize * @param messagePart The first part of the tagged string */ -export function serializeI18nHead(meta: I18nMeta, messagePart: string): string { +export function serializeI18nHead( + meta: I18nMeta, messagePart: string): {cooked: string, raw: string} { let metaBlock = meta.description || ''; if (meta.meaning) { metaBlock = `${meta.meaning}|${metaBlock}`; @@ -251,9 +253,12 @@ export function serializeI18nHead(meta: I18nMeta, messagePart: string): string { } if (metaBlock === '') { // There is no metaBlock, so we must ensure that any starting colon is escaped. - return escapeStartingColon(messagePart); + return {cooked: messagePart, raw: escapeStartingColon(messagePart)}; } else { - return `:${escapeColons(metaBlock)}:${messagePart}`; + return { + cooked: `:${metaBlock}:${messagePart}`, + raw: `:${escapeColons(metaBlock)}:${messagePart}` + }; } } diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index b9b77c1ca3..d446349e73 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -210,26 +210,38 @@ describe('Utils', () => { }); it('serializeI18nHead()', () => { - expect(serializeI18nHead(meta(), '')).toEqual(''); - expect(serializeI18nHead(meta('', '', 'desc'), '')).toEqual(':desc:'); - expect(serializeI18nHead(meta('id', '', 'desc'), '')).toEqual(':desc@@id:'); - expect(serializeI18nHead(meta('', 'meaning', 'desc'), '')).toEqual(':meaning|desc:'); - expect(serializeI18nHead(meta('id', 'meaning', 'desc'), '')).toEqual(':meaning|desc@@id:'); - expect(serializeI18nHead(meta('id', '', ''), '')).toEqual(':@@id:'); + expect(serializeI18nHead(meta(), '')).toEqual({cooked: '', raw: ''}); + expect(serializeI18nHead(meta('', '', 'desc'), '')) + .toEqual({cooked: ':desc:', raw: ':desc:'}); + expect(serializeI18nHead(meta('id', '', 'desc'), '')) + .toEqual({cooked: ':desc@@id:', raw: ':desc@@id:'}); + expect(serializeI18nHead(meta('', 'meaning', 'desc'), '')) + .toEqual({cooked: ':meaning|desc:', raw: ':meaning|desc:'}); + expect(serializeI18nHead(meta('id', 'meaning', 'desc'), '')) + .toEqual({cooked: ':meaning|desc@@id:', raw: ':meaning|desc@@id:'}); + expect(serializeI18nHead(meta('id', '', ''), '')).toEqual({cooked: ':@@id:', raw: ':@@id:'}); // Escaping colons (block markers) expect(serializeI18nHead(meta('id:sub_id', 'meaning', 'desc'), '')) - .toEqual(':meaning|desc@@id\\:sub_id:'); - expect(serializeI18nHead(meta('id', 'meaning:sub_meaning', 'desc'), '')) - .toEqual(':meaning\\:sub_meaning|desc@@id:'); + .toEqual({cooked: ':meaning|desc@@id:sub_id:', raw: ':meaning|desc@@id\\:sub_id:'}); + expect(serializeI18nHead(meta('id', 'meaning:sub_meaning', 'desc'), '')).toEqual({ + cooked: ':meaning:sub_meaning|desc@@id:', + raw: ':meaning\\:sub_meaning|desc@@id:' + }); expect(serializeI18nHead(meta('id', 'meaning', 'desc:sub_desc'), '')) - .toEqual(':meaning|desc\\:sub_desc@@id:'); - expect(serializeI18nHead(meta('id', 'meaning', 'desc'), 'message source')) - .toEqual(':meaning|desc@@id:message source'); - expect(serializeI18nHead(meta('id', 'meaning', 'desc'), ':message source')) - .toEqual(':meaning|desc@@id::message source'); - expect(serializeI18nHead(meta('', '', ''), 'message source')).toEqual('message source'); - expect(serializeI18nHead(meta('', '', ''), ':message source')).toEqual('\\:message source'); + .toEqual({cooked: ':meaning|desc:sub_desc@@id:', raw: ':meaning|desc\\:sub_desc@@id:'}); + expect(serializeI18nHead(meta('id', 'meaning', 'desc'), 'message source')).toEqual({ + cooked: ':meaning|desc@@id:message source', + raw: ':meaning|desc@@id:message source' + }); + expect(serializeI18nHead(meta('id', 'meaning', 'desc'), ':message source')).toEqual({ + cooked: ':meaning|desc@@id::message source', + raw: ':meaning|desc@@id::message source' + }); + expect(serializeI18nHead(meta('', '', ''), 'message source')) + .toEqual({cooked: 'message source', raw: 'message source'}); + expect(serializeI18nHead(meta('', '', ''), ':message source')) + .toEqual({cooked: ':message source', raw: '\\:message source'}); }); it('serializeI18nPlaceholderBlock()', () => {