From 08c038ebd931957712be58daf627bc11aaba2141 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 28 Oct 2016 19:53:42 -0700 Subject: [PATCH] fix(core): xmb serializer uses decimal messaged IDs fixes #12511 --- .../integrationtest/src/messages.fi.xtb | 13 ---- .../integrationtest/test/i18n_spec.ts | 7 +- modules/@angular/compiler/src/i18n/digest.ts | 6 +- .../compiler/src/i18n/extractor_merger.ts | 21 +++--- .../@angular/compiler/src/i18n/i18n_ast.ts | 7 +- .../@angular/compiler/src/i18n/i18n_parser.ts | 10 +-- .../compiler/src/i18n/message_bundle.ts | 10 ++- .../src/i18n/serializers/placeholder.ts | 20 ++---- .../src/i18n/serializers/serializer.ts | 24 ++++--- .../compiler/src/i18n/serializers/xliff.ts | 58 ++++++++++------- .../compiler/src/i18n/serializers/xmb.ts | 14 ++-- .../compiler/src/i18n/serializers/xtb.ts | 63 +++++++++++------- .../compiler/src/i18n/translation_bundle.ts | 13 ++-- .../test/i18n/extractor_merger_spec.ts | 9 ++- .../compiler/test/i18n/i18n_parser_spec.ts | 7 +- .../compiler/test/i18n/integration_spec.ts | 65 ++++++++++--------- .../compiler/test/i18n/message_bundle_spec.ts | 18 ++--- .../test/i18n/serializers/xmb_spec.ts | 8 +-- .../test/i18n/serializers/xtb_spec.ts | 49 +++++++------- .../src/browser/browser_adapter.ts | 1 + 20 files changed, 217 insertions(+), 206 deletions(-) delete mode 100644 modules/@angular/compiler-cli/integrationtest/src/messages.fi.xtb diff --git a/modules/@angular/compiler-cli/integrationtest/src/messages.fi.xtb b/modules/@angular/compiler-cli/integrationtest/src/messages.fi.xtb deleted file mode 100644 index 533660d0bf..0000000000 --- a/modules/@angular/compiler-cli/integrationtest/src/messages.fi.xtb +++ /dev/null @@ -1,13 +0,0 @@ - - - - - - - -]> - - käännä teksti - tervetuloa - other-3rdP-component - diff --git a/modules/@angular/compiler-cli/integrationtest/test/i18n_spec.ts b/modules/@angular/compiler-cli/integrationtest/test/i18n_spec.ts index 5346d88d7f..fc3fd6e61a 100644 --- a/modules/@angular/compiler-cli/integrationtest/test/i18n_spec.ts +++ b/modules/@angular/compiler-cli/integrationtest/test/i18n_spec.ts @@ -34,9 +34,9 @@ const EXPECTED_XMB = ` ]> - other-3rdP-component - translate me - Welcome + other-3rdP-component + translate me + Welcome `; @@ -79,5 +79,4 @@ describe('template i18n extraction output', () => { const xlf = fs.readFileSync(xlfOutput, {encoding: 'utf-8'}); expect(xlf).toEqual(EXPECTED_XLIFF); }); - }); diff --git a/modules/@angular/compiler/src/i18n/digest.ts b/modules/@angular/compiler/src/i18n/digest.ts index d104fa74e1..2b35e63b8b 100644 --- a/modules/@angular/compiler/src/i18n/digest.ts +++ b/modules/@angular/compiler/src/i18n/digest.ts @@ -8,10 +8,14 @@ import * as i18n from './i18n_ast'; -export function digestMessage(message: i18n.Message): string { +export function digest(message: i18n.Message): string { return sha1(serializeNodes(message.nodes).join('') + `[${message.meaning}]`); } +export function decimalDigest(message: i18n.Message): string { + return fingerprint(serializeNodes(message.nodes).join('') + `[${message.meaning}]`); +} + /** * Serialize the i18n ast to something xml-like in order to generate an UID. * diff --git a/modules/@angular/compiler/src/i18n/extractor_merger.ts b/modules/@angular/compiler/src/i18n/extractor_merger.ts index 0e834c1ca2..703dd2de1b 100644 --- a/modules/@angular/compiler/src/i18n/extractor_merger.ts +++ b/modules/@angular/compiler/src/i18n/extractor_merger.ts @@ -10,7 +10,7 @@ import * as html from '../ml_parser/ast'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseTreeResult} from '../ml_parser/parser'; -import {digestMessage} from './digest'; +import {digest} from './digest'; import * as i18n from './i18n_ast'; import {createI18nMessageFactory} from './i18n_parser'; import {I18nError} from './parse_util'; @@ -214,8 +214,8 @@ class _Visitor implements html.Visitor { // Extract only top level nodes with the (implicit) "i18n" attribute if not in a block or an ICU // message const i18nAttr = _getI18nAttr(el); - const isImplicit = this._implicitTags.some((tag: string): boolean => el.name === tag) && - !this._inIcu && !this._isInTranslatableSection; + const isImplicit = this._implicitTags.some(tag => el.name === tag) && !this._inIcu && + !this._isInTranslatableSection; const isTopLevelImplicit = !wasInImplicitNode && isImplicit; this._inImplicitNode = this._inImplicitNode || isImplicit; @@ -348,14 +348,14 @@ class _Visitor implements html.Visitor { // no-op when called in extraction mode (returns []) private _translateMessage(el: html.Node, message: i18n.Message): html.Node[] { if (message && this._mode === _VisitorMode.Merge) { - const id = digestMessage(message); - const nodes = this._translations.get(id); + const nodes = this._translations.get(message); if (nodes) { return nodes; } - this._reportError(el, `Translation unavailable for message id="${id}"`); + this._reportError( + el, `Translation unavailable for message id="${this._translations.digest(message)}"`); } return []; @@ -384,19 +384,20 @@ class _Visitor implements html.Visitor { if (attr.value && attr.value != '' && i18nAttributeMeanings.hasOwnProperty(attr.name)) { const meaning = i18nAttributeMeanings[attr.name]; const message: i18n.Message = this._createI18nMessage([attr], meaning, ''); - const id = digestMessage(message); - const nodes = this._translations.get(id); + const nodes = this._translations.get(message); if (nodes) { if (nodes[0] instanceof html.Text) { const value = (nodes[0] as html.Text).value; translatedAttributes.push(new html.Attribute(attr.name, value, attr.sourceSpan)); } else { this._reportError( - el, `Unexpected translation for attribute "${attr.name}" (id="${id}")`); + el, + `Unexpected translation for attribute "${attr.name}" (id="${this._translations.digest(message)}")`); } } else { this._reportError( - el, `Translation unavailable for attribute "${attr.name}" (id="${id}")`); + el, + `Translation unavailable for attribute "${attr.name}" (id="${this._translations.digest(message)}")`); } } else { translatedAttributes.push(attr); diff --git a/modules/@angular/compiler/src/i18n/i18n_ast.ts b/modules/@angular/compiler/src/i18n/i18n_ast.ts index 7ed07b6b87..664feec264 100644 --- a/modules/@angular/compiler/src/i18n/i18n_ast.ts +++ b/modules/@angular/compiler/src/i18n/i18n_ast.ts @@ -12,14 +12,13 @@ export class Message { /** * @param nodes message AST * @param placeholders maps placeholder names to static content - * @param placeholderToMsgIds maps placeholder names to translatable message IDs (used for ICU - * messages) + * @param placeholderToMessage maps placeholder names to messages (used for nested ICU messages) * @param meaning * @param description */ constructor( - public nodes: Node[], public placeholders: {[name: string]: string}, - public placeholderToMsgIds: {[name: string]: string}, public meaning: string, + public nodes: Node[], public placeholders: {[phName: string]: string}, + public placeholderToMessage: {[phName: string]: Message}, public meaning: string, public description: string) {} } diff --git a/modules/@angular/compiler/src/i18n/i18n_parser.ts b/modules/@angular/compiler/src/i18n/i18n_parser.ts index 55b0412bb6..b2499e89ec 100644 --- a/modules/@angular/compiler/src/i18n/i18n_parser.ts +++ b/modules/@angular/compiler/src/i18n/i18n_parser.ts @@ -12,7 +12,7 @@ import * as html from '../ml_parser/ast'; import {getHtmlTagDefinition} from '../ml_parser/html_tags'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseSourceSpan} from '../parse_util'; -import {digestMessage} from './digest'; +import {digest} from './digest'; import * as i18n from './i18n_ast'; import {PlaceholderRegistry} from './serializers/placeholder'; @@ -35,7 +35,7 @@ class _I18nVisitor implements html.Visitor { private _icuDepth: number; private _placeholderRegistry: PlaceholderRegistry; private _placeholderToContent: {[name: string]: string}; - private _placeholderToIds: {[name: string]: string}; + private _placeholderToMessage: {[name: string]: i18n.Message}; constructor( private _expressionParser: ExpressionParser, @@ -46,12 +46,12 @@ class _I18nVisitor implements html.Visitor { this._icuDepth = 0; this._placeholderRegistry = new PlaceholderRegistry(); this._placeholderToContent = {}; - this._placeholderToIds = {}; + this._placeholderToMessage = {}; const i18nodes: i18n.Node[] = html.visitAll(this, nodes, {}); return new i18n.Message( - i18nodes, this._placeholderToContent, this._placeholderToIds, meaning, description); + i18nodes, this._placeholderToContent, this._placeholderToMessage, meaning, description); } visitElement(el: html.Element, context: any): i18n.Node { @@ -110,7 +110,7 @@ class _I18nVisitor implements html.Visitor { // TODO(vicb): add a html.Node -> i18n.Message cache to avoid having to re-create the msg const phName = this._placeholderRegistry.getPlaceholderName('ICU', icu.sourceSpan.toString()); const visitor = new _I18nVisitor(this._expressionParser, this._interpolationConfig); - this._placeholderToIds[phName] = digestMessage(visitor.toI18nMessage([icu], '', '')); + this._placeholderToMessage[phName] = visitor.toI18nMessage([icu], '', ''); return new i18n.IcuPlaceholder(i18nIcu, phName, icu.sourceSpan); } diff --git a/modules/@angular/compiler/src/i18n/message_bundle.ts b/modules/@angular/compiler/src/i18n/message_bundle.ts index ff37a19590..a82afebc79 100644 --- a/modules/@angular/compiler/src/i18n/message_bundle.ts +++ b/modules/@angular/compiler/src/i18n/message_bundle.ts @@ -10,7 +10,6 @@ import {HtmlParser} from '../ml_parser/html_parser'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {ParseError} from '../parse_util'; -import {digestMessage} from './digest'; import {extractMessages} from './extractor_merger'; import {Message} from './i18n_ast'; import {Serializer} from './serializers/serializer'; @@ -19,7 +18,7 @@ import {Serializer} from './serializers/serializer'; * A container for message extracted from the templates. */ export class MessageBundle { - private _messageMap: {[id: string]: Message} = {}; + private _messages: Message[] = []; constructor( private _htmlParser: HtmlParser, private _implicitTags: string[], @@ -40,11 +39,10 @@ export class MessageBundle { return i18nParserResult.errors; } - i18nParserResult.messages.forEach( - (message) => { this._messageMap[digestMessage(message)] = message; }); + this._messages.push(...i18nParserResult.messages); } - getMessageMap(): {[id: string]: Message} { return this._messageMap; } + getMessages(): Message[] { return this._messages; } - write(serializer: Serializer): string { return serializer.write(this._messageMap); } + write(serializer: Serializer): string { return serializer.write(this._messages); } } diff --git a/modules/@angular/compiler/src/i18n/serializers/placeholder.ts b/modules/@angular/compiler/src/i18n/serializers/placeholder.ts index 0a388deb86..4c418fbe52 100644 --- a/modules/@angular/compiler/src/i18n/serializers/placeholder.ts +++ b/modules/@angular/compiler/src/i18n/serializers/placeholder.ts @@ -40,7 +40,9 @@ const TAG_TO_PLACEHOLDER_NAMES: {[k: string]: string} = { }; /** - * Creates unique names for placeholder with different content + * Creates unique names for placeholder with different content. + * + * Returns the same placeholder name when the content is identical. * * @internal */ @@ -105,18 +107,8 @@ export class PlaceholderRegistry { private _hashClosingTag(tag: string): string { return this._hashTag(`/${tag}`, {}, false); } private _generateUniqueName(base: string): string { - let name = base; - let next = this._placeHolderNameCounts[name]; - - if (!next) { - next = 1; - } else { - name += `_${next}`; - next++; - } - - this._placeHolderNameCounts[base] = next; - - return name; + const next = this._placeHolderNameCounts[base]; + this._placeHolderNameCounts[base] = next ? next + 1 : 1; + return next ? `${base}_${next}` : base; } } diff --git a/modules/@angular/compiler/src/i18n/serializers/serializer.ts b/modules/@angular/compiler/src/i18n/serializers/serializer.ts index b1a15fa619..1e502c3837 100644 --- a/modules/@angular/compiler/src/i18n/serializers/serializer.ts +++ b/modules/@angular/compiler/src/i18n/serializers/serializer.ts @@ -11,31 +11,29 @@ import * as i18n from '../i18n_ast'; import {MessageBundle} from '../message_bundle'; export interface Serializer { - write(messageMap: {[id: string]: i18n.Message}): string; + write(messages: i18n.Message[]): string; load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: html.Node[]}; + + digest(message: i18n.Message): string; } // Generate a map of placeholder to content indexed by message ids -export function extractPlaceholders(messageBundle: MessageBundle) { - const messageMap = messageBundle.getMessageMap(); - const placeholders: {[id: string]: {[name: string]: string}} = {}; +export function extractPlaceholders(messageMap: {[msgKey: string]: i18n.Message}) { + const phByMsgId: {[msgId: string]: {[name: string]: string}} = {}; - Object.keys(messageMap).forEach(msgId => { - placeholders[msgId] = messageMap[msgId].placeholders; - }); + Object.keys(messageMap).forEach(msgId => { phByMsgId[msgId] = messageMap[msgId].placeholders; }); - return placeholders; + return phByMsgId; } // Generate a map of placeholder to message ids indexed by message ids -export function extractPlaceholderToIds(messageBundle: MessageBundle) { - const messageMap = messageBundle.getMessageMap(); - const placeholderToIds: {[id: string]: {[name: string]: string}} = {}; +export function extractPlaceholderToMessage(messageMap: {[msgKey: string]: i18n.Message}) { + const phToMsgByMsgId: {[msgId: string]: {[name: string]: i18n.Message}} = {}; Object.keys(messageMap).forEach(msgId => { - placeholderToIds[msgId] = messageMap[msgId].placeholderToMsgIds; + phToMsgByMsgId[msgId] = messageMap[msgId].placeholderToMessage; }); - return placeholderToIds; + return phToMsgByMsgId; } \ No newline at end of file diff --git a/modules/@angular/compiler/src/i18n/serializers/xliff.ts b/modules/@angular/compiler/src/i18n/serializers/xliff.ts index 7d70ab6c98..31962fc098 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xliff.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xliff.ts @@ -12,11 +12,12 @@ import {HtmlParser} from '../../ml_parser/html_parser'; import {InterpolationConfig} from '../../ml_parser/interpolation_config'; import {XmlParser} from '../../ml_parser/xml_parser'; import {ParseError} from '../../parse_util'; +import {digest} from '../digest'; import * as i18n from '../i18n_ast'; import {MessageBundle} from '../message_bundle'; import {I18nError} from '../parse_util'; -import {Serializer, extractPlaceholderToIds, extractPlaceholders} from './serializer'; +import {Serializer, extractPlaceholderToMessage, extractPlaceholders} from './serializer'; import * as xml from './xml_helper'; const _VERSION = '1.2'; @@ -33,15 +34,14 @@ const _UNIT_TAG = 'trans-unit'; export class Xliff implements Serializer { constructor(private _htmlParser: HtmlParser, private _interpolationConfig: InterpolationConfig) {} - write(messageMap: {[id: string]: i18n.Message}): string { + write(messages: i18n.Message[]): string { const visitor = new _WriteVisitor(); const transUnits: xml.Node[] = []; - Object.keys(messageMap).forEach((id) => { - const message = messageMap[id]; + messages.forEach(message => { - const transUnit = new xml.Tag(_UNIT_TAG, {id: id, datatype: 'html'}); + const transUnit = new xml.Tag(_UNIT_TAG, {id: this.digest(message), datatype: 'html'}); transUnit.children.push( new xml.CR(8), new xml.Tag(_SOURCE_TAG, {}, visitor.serialize(message.nodes)), new xml.CR(8), new xml.Tag(_TARGET_TAG)); @@ -85,7 +85,7 @@ export class Xliff implements Serializer { } // Replace the placeholders, messages are now string - const {messages, errors} = new _LoadVisitor().parse(result.rootNodes, messageBundle); + const {messages, errors} = new _LoadVisitor(this).parse(result.rootNodes, messageBundle); if (errors.length) { throw new Error(`xtb parse errors:\n${errors.join('\n')}`); @@ -108,6 +108,8 @@ export class Xliff implements Serializer { return messageMap; } + + digest(message: i18n.Message): string { return digest(message); } } class _WriteVisitor implements i18n.Visitor { @@ -174,8 +176,10 @@ class _LoadVisitor implements ml.Visitor { private _msgId: string; private _target: ml.Node[]; private _errors: I18nError[]; - private _placeholders: {[name: string]: string}; - private _placeholderToIds: {[name: string]: string}; + private _placeholders: {[phName: string]: string}; + private _placeholderToMessage: {[phName: string]: i18n.Message}; + + constructor(private _serializer: Serializer) {} parse(nodes: ml.Node[], messageBundle: MessageBundle): {messages: {[k: string]: string}, errors: I18nError[]} { @@ -188,9 +192,10 @@ class _LoadVisitor implements ml.Visitor { // Find all messages ml.visitAll(this, nodes, null); - const messageMap = messageBundle.getMessageMap(); - const placeholders = extractPlaceholders(messageBundle); - const placeholderToIds = extractPlaceholderToIds(messageBundle); + const messageMap: {[msgId: string]: i18n.Message} = {}; + messageBundle.getMessages().forEach(m => messageMap[this._serializer.digest(m)] = m); + const placeholdersByMsgId = extractPlaceholders(messageMap); + const placeholderToMessageByMsgId = extractPlaceholderToMessage(messageMap); this._messageNodes .filter(message => { @@ -198,26 +203,26 @@ class _LoadVisitor implements ml.Visitor { return messageMap.hasOwnProperty(message[0]); }) .sort((a, b) => { - // Because there could be no ICU placeholders inside an ICU message, + // Because there could be no ICU placeholdersByMsgId inside an ICU message, // we do not need to take into account the `placeholderToMsgIds` of the referenced // messages, those would always be empty // TODO(vicb): overkill - create 2 buckets and [...woDeps, ...wDeps].process() - if (Object.keys(messageMap[a[0]].placeholderToMsgIds).length == 0) { + if (Object.keys(messageMap[a[0]].placeholderToMessage).length == 0) { return -1; } - if (Object.keys(messageMap[b[0]].placeholderToMsgIds).length == 0) { + if (Object.keys(messageMap[b[0]].placeholderToMessage).length == 0) { return 1; } return 0; }) .forEach(message => { - const id = message[0]; - this._placeholders = placeholders[id] || {}; - this._placeholderToIds = placeholderToIds[id] || {}; + const msgId = message[0]; + this._placeholders = placeholdersByMsgId[msgId] || {}; + this._placeholderToMessage = placeholderToMessageByMsgId[msgId] || {}; // TODO(vicb): make sure there is no `_TRANSLATIONS_TAG` nor `_TRANSLATION_TAG` - this._translatedMessages[id] = ml.visitAll(this, message[1]).join(''); + this._translatedMessages[msgId] = ml.visitAll(this, message[1]).join(''); }); return {messages: this._translatedMessages, errors: this._errors}; @@ -252,17 +257,20 @@ class _LoadVisitor implements ml.Visitor { if (!idAttr) { this._addError(element, `<${_PLACEHOLDER_TAG}> misses the "id" attribute`); } else { - const id = idAttr.value; - if (this._placeholders.hasOwnProperty(id)) { - return this._placeholders[id]; + const phName = idAttr.value; + if (this._placeholders.hasOwnProperty(phName)) { + return this._placeholders[phName]; } - if (this._placeholderToIds.hasOwnProperty(id) && - this._translatedMessages.hasOwnProperty(this._placeholderToIds[id])) { - return this._translatedMessages[this._placeholderToIds[id]]; + if (this._placeholderToMessage.hasOwnProperty(phName)) { + const refMsgId = this._serializer.digest(this._placeholderToMessage[phName]); + if (this._translatedMessages.hasOwnProperty(refMsgId)) { + return this._translatedMessages[refMsgId]; + } } // TODO(vicb): better error message for when // !this._translatedMessages.hasOwnProperty(this._placeholderToIds[id]) - this._addError(element, `The placeholder "${id}" does not exists in the source message`); + this._addError( + element, `The placeholder "${phName}" does not exists in the source message`); } break; diff --git a/modules/@angular/compiler/src/i18n/serializers/xmb.ts b/modules/@angular/compiler/src/i18n/serializers/xmb.ts index 1860071c3d..20cea88ec1 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xmb.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xmb.ts @@ -8,6 +8,7 @@ import {ListWrapper} from '../../facade/collection'; import * as html from '../../ml_parser/ast'; +import {decimalDigest} from '../digest'; import * as i18n from '../i18n_ast'; import {MessageBundle} from '../message_bundle'; @@ -40,13 +41,12 @@ const _DOCTYPE = ` `; export class Xmb implements Serializer { - write(messageMap: {[k: string]: i18n.Message}): string { + write(messages: i18n.Message[]): string { const visitor = new _Visitor(); const rootNode = new xml.Tag(_MESSAGES_TAG); - Object.keys(messageMap).forEach((id) => { - const message = messageMap[id]; - const attrs: {[k: string]: string} = {id}; + messages.forEach(message => { + const attrs: {[k: string]: string} = {id: this.digest(message)}; if (message.description) { attrs['desc'] = message.description; @@ -75,6 +75,8 @@ export class Xmb implements Serializer { load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: html.Node[]} { throw new Error('Unsupported'); } + + digest(message: i18n.Message): string { return digest(message); } } class _Visitor implements i18n.Visitor { @@ -124,3 +126,7 @@ class _Visitor implements i18n.Visitor { return ListWrapper.flatten(nodes.map(node => node.visit(this))); } } + +export function digest(message: i18n.Message): string { + return decimalDigest(message); +} \ No newline at end of file diff --git a/modules/@angular/compiler/src/i18n/serializers/xtb.ts b/modules/@angular/compiler/src/i18n/serializers/xtb.ts index 201b89cf7c..92f7ac78b3 100644 --- a/modules/@angular/compiler/src/i18n/serializers/xtb.ts +++ b/modules/@angular/compiler/src/i18n/serializers/xtb.ts @@ -15,7 +15,8 @@ import * as i18n from '../i18n_ast'; import {MessageBundle} from '../message_bundle'; import {I18nError} from '../parse_util'; -import {Serializer, extractPlaceholderToIds, extractPlaceholders} from './serializer'; +import {Serializer, extractPlaceholderToMessage, extractPlaceholders} from './serializer'; +import {digest} from './xmb'; const _TRANSLATIONS_TAG = 'translationbundle'; const _TRANSLATION_TAG = 'translation'; @@ -24,7 +25,7 @@ const _PLACEHOLDER_TAG = 'ph'; export class Xtb implements Serializer { constructor(private _htmlParser: HtmlParser, private _interpolationConfig: InterpolationConfig) {} - write(messageMap: {[id: string]: i18n.Message}): string { throw new Error('Unsupported'); } + write(messages: i18n.Message[]): string { throw new Error('Unsupported'); } load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: ml.Node[]} { // Parse the xtb file into xml nodes @@ -35,7 +36,7 @@ export class Xtb implements Serializer { } // Replace the placeholders, messages are now string - const {messages, errors} = new _Visitor().parse(result.rootNodes, messageBundle); + const {messages, errors} = new _Visitor(this).parse(result.rootNodes, messageBundle); if (errors.length) { throw new Error(`xtb parse errors:\n${errors.join('\n')}`); @@ -46,10 +47,10 @@ export class Xtb implements Serializer { const messageMap: {[id: string]: ml.Node[]} = {}; const parseErrors: ParseError[] = []; - Object.keys(messages).forEach((id) => { - const res = this._htmlParser.parse(messages[id], url, true, this._interpolationConfig); + Object.keys(messages).forEach((msgId) => { + const res = this._htmlParser.parse(messages[msgId], url, true, this._interpolationConfig); parseErrors.push(...res.errors); - messageMap[id] = res.rootNodes; + messageMap[msgId] = res.rootNodes; }); if (parseErrors.length) { @@ -58,6 +59,11 @@ export class Xtb implements Serializer { return messageMap; } + + digest(message: i18n.Message): string { + // we must use the same digest as xmb + return digest(message); + } } class _Visitor implements ml.Visitor { @@ -66,11 +72,14 @@ class _Visitor implements ml.Visitor { private _bundleDepth: number; private _translationDepth: number; private _errors: I18nError[]; - private _placeholders: {[name: string]: string}; - private _placeholderToIds: {[name: string]: string}; + private _placeholders: {[phName: string]: string}; + private _placeholderToMessage: {[phName: string]: i18n.Message}; + + constructor(private _serializer: Serializer) {} parse(nodes: ml.Node[], messageBundle: MessageBundle): {messages: {[k: string]: string}, errors: I18nError[]} { + // Tuple [, [ml nodes]] this._messageNodes = []; this._translatedMessages = {}; this._bundleDepth = 0; @@ -80,9 +89,10 @@ class _Visitor implements ml.Visitor { // Find all messages ml.visitAll(this, nodes, null); - const messageMap = messageBundle.getMessageMap(); - const placeholders = extractPlaceholders(messageBundle); - const placeholderToIds = extractPlaceholderToIds(messageBundle); + const messageMap: {[msgId: string]: i18n.Message} = {}; + messageBundle.getMessages().forEach(m => messageMap[this._serializer.digest(m)] = m); + const placeholdersByMsgId = extractPlaceholders(messageMap); + const placeholderToMessageByMsgId = extractPlaceholderToMessage(messageMap); this._messageNodes .filter(message => { @@ -94,22 +104,23 @@ class _Visitor implements ml.Visitor { // we do not need to take into account the `placeholderToMsgIds` of the referenced // messages, those would always be empty // TODO(vicb): overkill - create 2 buckets and [...woDeps, ...wDeps].process() - if (Object.keys(messageMap[a[0]].placeholderToMsgIds).length == 0) { + if (Object.keys(messageMap[a[0]].placeholderToMessage).length == 0) { return -1; } - if (Object.keys(messageMap[b[0]].placeholderToMsgIds).length == 0) { + if (Object.keys(messageMap[b[0]].placeholderToMessage).length == 0) { return 1; } return 0; }) .forEach(message => { - const id = message[0]; - this._placeholders = placeholders[id] || {}; - this._placeholderToIds = placeholderToIds[id] || {}; + const msgId = message[0]; + this._placeholders = placeholdersByMsgId[msgId] || {}; + this._placeholderToMessage = placeholderToMessageByMsgId[msgId] || {}; + // TODO(vicb): make sure there is no `_TRANSLATIONS_TAG` nor `_TRANSLATION_TAG` - this._translatedMessages[id] = ml.visitAll(this, message[1]).join(''); + this._translatedMessages[msgId] = ml.visitAll(this, message[1]).join(''); }); return {messages: this._translatedMessages, errors: this._errors}; @@ -149,18 +160,20 @@ class _Visitor implements ml.Visitor { if (!nameAttr) { this._addError(element, `<${_PLACEHOLDER_TAG}> misses the "name" attribute`); } else { - const name = nameAttr.value; - if (this._placeholders.hasOwnProperty(name)) { - return this._placeholders[name]; + const phName = nameAttr.value; + if (this._placeholders.hasOwnProperty(phName)) { + return this._placeholders[phName]; } - if (this._placeholderToIds.hasOwnProperty(name) && - this._translatedMessages.hasOwnProperty(this._placeholderToIds[name])) { - return this._translatedMessages[this._placeholderToIds[name]]; + if (this._placeholderToMessage.hasOwnProperty(phName)) { + const refMessageId = this._serializer.digest(this._placeholderToMessage[phName]); + if (this._translatedMessages.hasOwnProperty(refMessageId)) { + return this._translatedMessages[refMessageId]; + } } // TODO(vicb): better error message for when - // !this._translatedMessages.hasOwnProperty(this._placeholderToIds[name]) + // !this._translatedMessages.hasOwnProperty(refMessageId) this._addError( - element, `The placeholder "${name}" does not exists in the source message`); + element, `The placeholder "${phName}" does not exists in the source message`); } break; diff --git a/modules/@angular/compiler/src/i18n/translation_bundle.ts b/modules/@angular/compiler/src/i18n/translation_bundle.ts index 28cbe0587f..b76934f166 100644 --- a/modules/@angular/compiler/src/i18n/translation_bundle.ts +++ b/modules/@angular/compiler/src/i18n/translation_bundle.ts @@ -8,21 +8,26 @@ import * as html from '../ml_parser/ast'; +import {Message} from './i18n_ast'; import {MessageBundle} from './message_bundle'; import {Serializer} from './serializers/serializer'; + /** * A container for translated messages */ export class TranslationBundle { - constructor(private _messageMap: {[id: string]: html.Node[]} = {}) {} + constructor( + private _messageMap: {[id: string]: html.Node[]} = {}, + public digest: (m: Message) => string) {} static load(content: string, url: string, messageBundle: MessageBundle, serializer: Serializer): TranslationBundle { - return new TranslationBundle(serializer.load(content, url, messageBundle)); + return new TranslationBundle( + serializer.load(content, url, messageBundle), (m: Message) => serializer.digest(m)); } - get(id: string): html.Node[] { return this._messageMap[id]; } + get(message: Message): html.Node[] { return this._messageMap[this.digest(message)]; } - has(id: string): boolean { return id in this._messageMap; } + has(message: Message): boolean { return this.digest(message) in this._messageMap; } } \ No newline at end of file diff --git a/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts b/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts index 88605c39b5..acda260f59 100644 --- a/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts +++ b/modules/@angular/compiler/test/i18n/extractor_merger_spec.ts @@ -6,15 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ +import {DEFAULT_INTERPOLATION_CONFIG, HtmlParser} from '@angular/compiler'; import {describe, expect, it} from '@angular/core/testing/testing_internal'; -import {digestMessage, serializeNodes as serializeI18nNodes} from '../../src/i18n/digest'; +import {digest, serializeNodes as serializeI18nNodes} from '../../src/i18n/digest'; import {extractMessages, mergeTranslations} from '../../src/i18n/extractor_merger'; import * as i18n from '../../src/i18n/i18n_ast'; import {TranslationBundle} from '../../src/i18n/translation_bundle'; import * as html from '../../src/ml_parser/ast'; -import {HtmlParser} from '../../src/ml_parser/html_parser'; -import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/interpolation_config'; import {serializeNodes as serializeHtmlNodes} from '../ml_parser/ast_serializer_spec'; export function main() { @@ -403,12 +402,12 @@ function fakeTranslate( const i18nMsgMap: {[id: string]: html.Node[]} = {}; messages.forEach(message => { - const id = digestMessage(message); + const id = digest(message); const text = serializeI18nNodes(message.nodes).join(''); i18nMsgMap[id] = [new html.Text(`**${text}**`, null)]; }); - const translations = new TranslationBundle(i18nMsgMap); + const translations = new TranslationBundle(i18nMsgMap, digest); const translatedNodes = mergeTranslations( diff --git a/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts b/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts index 451f1770aa..245b34cf98 100644 --- a/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts +++ b/modules/@angular/compiler/test/i18n/i18n_parser_spec.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {digest} from '@angular/compiler/src/i18n/digest'; import {extractMessages} from '@angular/compiler/src/i18n/extractor_merger'; import {Message} from '@angular/compiler/src/i18n/i18n_ast'; import {describe, expect, it} from '@angular/core/testing/testing_internal'; @@ -276,7 +277,7 @@ export function main() { // As such they have no static content but refs to message ids. expect(_humanizePlaceholders(html)).toEqual(['', '', '', '']); - expect(_humanizePlaceholdersToIds(html)).toEqual([ + expect(_humanizePlaceholdersToMessage(html)).toEqual([ 'ICU=f0f76923009914f1b05f41042a5c7231b9496504, ICU_1=73693d1f78d0fc882f0bcbce4cb31a0aa1995cfe', '', '', @@ -308,13 +309,13 @@ function _humanizePlaceholders( // clang-format on } -function _humanizePlaceholdersToIds( +function _humanizePlaceholdersToMessage( html: string, implicitTags: string[] = [], implicitAttrs: {[k: string]: string[]} = {}): string[] { // clang-format off // https://github.com/angular/clang-format/issues/35 return _extractMessages(html, implicitTags, implicitAttrs).map( - msg => Object.keys(msg.placeholderToMsgIds).map(k => `${k}=${msg.placeholderToMsgIds[k]}`).join(', ')); + msg => Object.keys(msg.placeholderToMessage).map(k => `${k}=${digest(msg.placeholderToMessage[k])}`).join(', ')); // clang-format on } diff --git a/modules/@angular/compiler/test/i18n/integration_spec.ts b/modules/@angular/compiler/test/i18n/integration_spec.ts index b8000491c8..3ecb9b0ff0 100644 --- a/modules/@angular/compiler/test/i18n/integration_spec.ts +++ b/modules/@angular/compiler/test/i18n/integration_spec.ts @@ -153,51 +153,52 @@ class FrLocalization extends NgLocalization { const XTB = ` - attributs i18n sur les balises - imbriqué - imbriqué - avec des espaces réservés - sur des balises non traductibles - sur des balises traductibles - {count, plural, =0 {zero} =1 {un} =2 {deux} other {beaucoup}} - - {sex, sex, m {homme} f {femme}} - - sexe = - - dans une section traductible - + attributs i18n sur les balises + imbriqué + imbriqué + avec des espaces réservés + sur des balises non traductibles + sur des balises traductibles + {count, plural, =0 {zero} =1 {un} =2 {deux} other {beaucoup}} + + {sex, sex, m {homme} f {femme}} + + sexe = + + dans une section traductible + Balises dans les commentaires html - ca devrait marcher + ca devrait marcher `; // unused, for reference only // can be generated from xmb_spec as follow: -// `iit('extract xmb', () => { console.log(toXmb(HTML)); });` +// `fit('extract xmb', () => { console.log(toXmb(HTML)); });` const XMB = ` - i18n attribute on tags - nested - nested - <i>with placeholders</i> - on not translatable node - on translatable node - {count, plural, =0 {zero}=1 {one}=2 {two}other {<b>many</b>}} - + i18n attribute on tags + nested + nested + <i>with placeholders</i> + on not translatable node + on translatable node + {count, plural, =0 {zero} =1 {one} =2 {two} other {<b>many</b>} } + - {sex, sex, m {male}f {female}} - - sex = - - in a translatable section - + {sex, sex, m {male} f {female} } + + sex = + + in a translatable section + <h1>Markers in html comments</h1> <div></div> <div></div> - it <b>should</b> work -`; + it <b>should</b> work + +`; diff --git a/modules/@angular/compiler/test/i18n/message_bundle_spec.ts b/modules/@angular/compiler/test/i18n/message_bundle_spec.ts index c47c4e0bc4..ed1692094f 100644 --- a/modules/@angular/compiler/test/i18n/message_bundle_spec.ts +++ b/modules/@angular/compiler/test/i18n/message_bundle_spec.ts @@ -26,17 +26,18 @@ export function main(): void { messages.updateFromTemplate( '

Translate Me

', 'url', DEFAULT_INTERPOLATION_CONFIG); expect(humanizeMessages(messages)).toEqual([ - '2e791a68a3324ecdd29e252198638dafacec46e9=Translate Me', + 'Translate Me (m|d)', ]); }); - it('should extract the same message with different meaning in different entries', () => { + it('should extract the all messages and duplicates', () => { messages.updateFromTemplate( - '

Translate Me

Translate Me

', 'url', + '

Translate Me

Translate Me

Translate Me

', 'url', DEFAULT_INTERPOLATION_CONFIG); expect(humanizeMessages(messages)).toEqual([ - '2e791a68a3324ecdd29e252198638dafacec46e9=Translate Me', - '8ca133f957845af1b1868da1b339180d1f519644=Translate Me', + 'Translate Me (m|d)', + 'Translate Me (|)', + 'Translate Me (|)', ]); }); }); @@ -44,13 +45,14 @@ export function main(): void { } class _TestSerializer implements Serializer { - write(messageMap: {[id: string]: i18n.Message}): string { - return Object.keys(messageMap) - .map(id => `${id}=${serializeNodes(messageMap[id].nodes)}`) + write(messages: i18n.Message[]): string { + return messages.map(msg => `${serializeNodes(msg.nodes)} (${msg.meaning}|${msg.description})`) .join('//'); } load(content: string, url: string, placeholders: {}): {} { return null; } + + digest(msg: i18n.Message): string { return 'unused'; } } function humanizeMessages(catalog: MessageBundle): string[] { diff --git a/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts b/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts index 658582fcd3..315a451c05 100644 --- a/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/xmb_spec.ts @@ -43,10 +43,10 @@ export function main(): void { ]> - translatable element <b>with placeholders</b> - { count, plural, =0 {<p>test</p>} } - foo - { count, plural, =0 {{ sex, select, other {<p>deeply nested</p>} } } } + translatable element <b>with placeholders</b> + { count, plural, =0 {<p>test</p>} } + foo + { count, plural, =0 {{ sex, gender, other {<p>deeply nested</p>} } } } `; diff --git a/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts b/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts index 8c85305062..2b8d0828d4 100644 --- a/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts +++ b/modules/@angular/compiler/test/i18n/serializers/xtb_spec.ts @@ -50,10 +50,10 @@ export function main(): void { ]> - rab + rab `; - expect(loadAsText(HTML, XTB)).toEqual({'28a86c8a00ae573b2bac698d6609316dc7b4a226': 'rab'}); + expect(loadAsText(HTML, XTB)).toEqual({'8841459487341224498': 'rab'}); }); it('should load XTB files without placeholders', () => { @@ -61,23 +61,22 @@ export function main(): void { const XTB = ` - rab + rab `; - expect(loadAsText(HTML, XTB)).toEqual({'28a86c8a00ae573b2bac698d6609316dc7b4a226': 'rab'}); + expect(loadAsText(HTML, XTB)).toEqual({'8841459487341224498': 'rab'}); }); + it('should load XTB files with placeholders', () => { const HTML = `

bar

`; const XTB = ` - rab + rab `; - expect(loadAsText(HTML, XTB)).toEqual({ - '7de4d8ff1e42b7b31da6204074818236a9a5317f': '

rab

' - }); + expect(loadAsText(HTML, XTB)).toEqual({'8877975308926375834': '

rab

'}); }); it('should replace ICU placeholders with their translations', () => { @@ -85,13 +84,13 @@ export function main(): void { const XTB = ` - ** - { count, plural, =1 {rab}} + ** + { count, plural, =1 {rab}} `; expect(loadAsText(HTML, XTB)).toEqual({ - 'eb404e202fed4846e25e7d9ac1fcb719fe4da257': `*{ count, plural, =1 {

rab

}}*`, - 'fc92b9b781194a02ab773129c8c5a7fc0735efd7': `{ count, plural, =1 {

rab

}}`, + '1430521728694081603': `*{ count, plural, =1 {

rab

}}*`, + '4004755025589356097': `{ count, plural, =1 {

rab

}}`, }); }); @@ -104,21 +103,19 @@ export function main(): void { const XTB = ` - rab oof - { count, plural, =1 {rab}} - oof - { count, plural, =1 {{ sex, select, other {rab}} }} + rab oof + { count, plural, =1 {rab}} + oof + { count, plural, =1 {{ sex, gender, male {rab}} }} `; expect(loadAsText(HTML, XTB)).toEqual({ - '7103b4b13b616270a0044efade97d8b4f96f2ca6': `{{ a + b }}rab oof`, - 'fc92b9b781194a02ab773129c8c5a7fc0735efd7': `{ count, plural, =1 {

rab

}}`, - 'db3e0a6a5a96481f60aec61d98c3eecddef5ac23': `oof`, - '8fb569d3dd83e92eff2551b24f5290d3035ce61b': - `{ count, plural, =1 {{ sex, select, other {

rab

}} }}`, + '8281795707202401639': `{{ a + b }}rab oof`, + '4004755025589356097': `{ count, plural, =1 {

rab

}}`, + '130772889486467622': `oof`, + '4244993204427636474': `{ count, plural, =1 {{ sex, gender, male {

rab

}} }}`, }); }); - }); describe('errors', () => { @@ -145,7 +142,7 @@ export function main(): void { const HTML = '
give me a message
'; const XTB = ` - + `; expect(() => { loadAsText(HTML, XTB); }).toThrowError(/ misses the "name" attribute/); @@ -156,7 +153,7 @@ export function main(): void { const XTB = ` - + `; expect(() => { @@ -170,7 +167,7 @@ export function main(): void { const XTB = ` - rab + rab `; expect(() => { @@ -188,6 +185,6 @@ export function main(): void { }); it('should throw when trying to save an xtb file', - () => { expect(() => { serializer.write({}); }).toThrowError(/Unsupported/); }); + () => { expect(() => { serializer.write([]); }).toThrowError(/Unsupported/); }); }); } \ No newline at end of file diff --git a/modules/@angular/platform-browser/src/browser/browser_adapter.ts b/modules/@angular/platform-browser/src/browser/browser_adapter.ts index eace0bf914..86e0e965d3 100644 --- a/modules/@angular/platform-browser/src/browser/browser_adapter.ts +++ b/modules/@angular/platform-browser/src/browser/browser_adapter.ts @@ -303,6 +303,7 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { importIntoDoc(node: Node): any { return document.importNode(this.templateAwareRoot(node), true); } adoptNode(node: Node): any { return document.adoptNode(node); } getHref(el: Element): string { return (el).href; } + getEventKey(event: any): string { let key = event.key; if (isBlank(key)) {