From bc28ca7b0cb7ad5227f5d6f8d9b1977b4d0aebcd 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 4e22afcce00..b181126918e 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 7bee9429e16..eb7616861d9 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 c4462fdbbcc..63f43730c37 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 bcb1f76c970..96143151553 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 b5671ffe812..9c505c9d182 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 2e10fe02f41..0b785369552 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 b9b77c1ca36..d446349e735 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()', () => {