fix(i18n): ICU placeholders are replaced by their translations (#10586)

They were replaced by the original message.

modules/@angular/compiler/src/i18n/digest.ts

@@ -97,7 +97,7 @@ export function sha1(str: string): string {
     hex += (b >>> 4 & 0x0f).toString(16) + (b & 0x0f).toString(16);
   }
 
-  return hex;
+  return hex.toLowerCase();
 }
 
 function utf8Encode(str: string): string {

modules/@angular/compiler/src/i18n/i18n_ast.ts

@@ -9,8 +9,17 @@
 import {ParseSourceSpan} from '../parse_util';
 
 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 meaning
+   * @param description
+   */
   constructor(
-      public nodes: Node[], public placeholders: {[name: string]: string}, public meaning: string,
+      public nodes: Node[], public placeholders: {[name: string]: string},
+      public placeholderToMsgIds: {[name: string]: string}, public meaning: string,
       public description: string) {}
 }
 

modules/@angular/compiler/src/i18n/i18n_parser.ts

@@ -12,6 +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 * as i18n from './i18n_ast';
 import {PlaceholderRegistry} from './serializers/placeholder';
@@ -19,7 +20,7 @@ import {PlaceholderRegistry} from './serializers/placeholder';
 const _expParser = new ExpressionParser(new ExpressionLexer());
 
 /**
- * Returns a function converting html Messages to i18n Messages given an interpolationConfig
+ * Returns a function converting html nodes to an i18n Message given an interpolationConfig
  */
 export function createI18nMessageFactory(interpolationConfig: InterpolationConfig): (
     nodes: html.Node[], meaning: string, description: string) => i18n.Message {
@@ -34,6 +35,7 @@ class _I18nVisitor implements html.Visitor {
   private _icuDepth: number;
   private _placeholderRegistry: PlaceholderRegistry;
   private _placeholderToContent: {[name: string]: string};
+  private _placeholderToIds: {[name: string]: string};
 
   constructor(
       private _expressionParser: ExpressionParser,
@@ -44,10 +46,12 @@ class _I18nVisitor implements html.Visitor {
     this._icuDepth = 0;
     this._placeholderRegistry = new PlaceholderRegistry();
     this._placeholderToContent = {};
+    this._placeholderToIds = {};
 
     const i18nodes: i18n.Node[] = html.visitAll(this, nodes, {});
 
-    return new i18n.Message(i18nodes, this._placeholderToContent, meaning, description);
+    return new i18n.Message(
+        i18nodes, this._placeholderToContent, this._placeholderToIds, meaning, description);
   }
 
   visitElement(el: html.Element, context: any): i18n.Node {
@@ -99,9 +103,14 @@ class _I18nVisitor implements html.Visitor {
       return i18nIcu;
     }
 
-    // else returns a placeholder
+    // Else returns a placeholder
+    // ICU placeholders should not be replaced with their original content but with the their
+    // translations. We need to create a new visitor (they are not re-entrant) to compute the
+    // message id.
+    // 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());
-    this._placeholderToContent[phName] = icu.sourceSpan.toString();
+    const visitor = new _I18nVisitor(this._expressionParser, this._interpolationConfig);
+    this._placeholderToIds[phName] = digestMessage(visitor.toI18nMessage([icu], '', ''));
     return new i18n.IcuPlaceholder(i18nIcu, phName, icu.sourceSpan);
   }
 

modules/@angular/compiler/src/i18n/message_bundle.ts

@@ -45,5 +45,7 @@ export class MessageBundle {
         (message) => { this._messageMap[digestMessage(message)] = message; });
   }
 
+  getMessageMap(): {[id: string]: Message} { return this._messageMap; }
+
   write(serializer: Serializer): string { return serializer.write(this._messageMap); }
 }

modules/@angular/compiler/src/i18n/serializers/serializer.ts

@@ -8,10 +8,34 @@
 
 import * as html from '../../ml_parser/ast';
 import * as i18n from '../i18n_ast';
+import {MessageBundle} from '../message_bundle';
 
 export interface Serializer {
   write(messageMap: {[id: string]: i18n.Message}): string;
 
-  load(content: string, url: string, placeholders: {[id: string]: {[name: string]: string}}):
-      {[id: string]: html.Node[]};
+  load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: html.Node[]};
+}
+
+// Generate a map of placeholder to content indexed by message ids
+export function extractPlaceholders(messageBundle: MessageBundle) {
+  const messageMap = messageBundle.getMessageMap();
+  let placeholders: {[id: string]: {[name: string]: string}} = {};
+
+  Object.keys(messageMap).forEach(msgId => {
+    placeholders[msgId] = messageMap[msgId].placeholders;
+  });
+
+  return placeholders;
+}
+
+// Generate a map of placeholder to message ids indexed by message ids
+export function extractPlaceholderToIds(messageBundle: MessageBundle) {
+  const messageMap = messageBundle.getMessageMap();
+  let placeholderToIds: {[id: string]: {[name: string]: string}} = {};
+
+  Object.keys(messageMap).forEach(msgId => {
+    placeholderToIds[msgId] = messageMap[msgId].placeholderToMsgIds;
+  });
+
+  return placeholderToIds;
 }

modules/@angular/compiler/src/i18n/serializers/xmb.ts

@@ -9,6 +9,7 @@
 import {ListWrapper} from '../../facade/collection';
 import * as html from '../../ml_parser/ast';
 import * as i18n from '../i18n_ast';
+import {MessageBundle} from '../message_bundle';
 
 import {Serializer} from './serializer';
 import * as xml from './xml_helper';
@@ -70,8 +71,7 @@ export class Xmb implements Serializer {
     ]);
   }
 
-  load(content: string, url: string, placeholders: {[id: string]: {[name: string]: string}}):
-      {[id: string]: html.Node[]} {
+  load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: html.Node[]} {
     throw new Error('Unsupported');
   }
 }

modules/@angular/compiler/src/i18n/serializers/xtb.ts

@@ -12,9 +12,10 @@ import {InterpolationConfig} from '../../ml_parser/interpolation_config';
 import {XmlParser} from '../../ml_parser/xml_parser';
 import {ParseError} from '../../parse_util';
 import * as i18n from '../i18n_ast';
+import {MessageBundle} from '../message_bundle';
 import {I18nError} from '../parse_util';
 
-import {Serializer} from './serializer';
+import {Serializer, extractPlaceholderToIds, extractPlaceholders} from './serializer';
 
 const _TRANSLATIONS_TAG = 'translationbundle';
 const _TRANSLATION_TAG = 'translation';
@@ -25,8 +26,7 @@ export class Xtb implements Serializer {
 
   write(messageMap: {[id: string]: i18n.Message}): string { throw new Error('Unsupported'); }
 
-  load(content: string, url: string, placeholders: {[id: string]: {[name: string]: string}}):
-      {[id: string]: ml.Node[]} {
+  load(content: string, url: string, messageBundle: MessageBundle): {[id: string]: ml.Node[]} {
     // Parse the xtb file into xml nodes
     const result = new XmlParser().parse(content, url);
 
@@ -35,7 +35,7 @@ export class Xtb implements Serializer {
     }
 
     // Replace the placeholders, messages are now string
-    const {messages, errors} = new _Serializer().parse(result.rootNodes, placeholders);
+    const {messages, errors} = new _Serializer().parse(result.rootNodes, messageBundle);
 
     if (errors.length) {
       throw new Error(`xtb parse errors:\n${errors.join('\n')}`);
@@ -44,7 +44,7 @@ export class Xtb implements Serializer {
     // Convert the string messages to html ast
     // TODO(vicb): map error message back to the original message in xtb
     let messageMap: {[id: string]: ml.Node[]} = {};
-    let parseErrors: ParseError[] = [];
+    const parseErrors: ParseError[] = [];
 
     Object.keys(messages).forEach((id) => {
       const res = this._htmlParser.parse(messages[id], url, true, this._interpolationConfig);
@@ -61,24 +61,58 @@ export class Xtb implements Serializer {
 }
 
 class _Serializer implements ml.Visitor {
-  private _messages: {[id: string]: string};
+  private _messageNodes: [string, ml.Node[]][];
+  private _translatedMessages: {[id: string]: string};
   private _bundleDepth: number;
   private _translationDepth: number;
   private _errors: I18nError[];
-  private _placeholders: {[id: string]: {[name: string]: string}};
-  private _currentPlaceholders: {[name: string]: string};
+  private _placeholders: {[name: string]: string};
+  private _placeholderToIds: {[name: string]: string};
 
-  parse(nodes: ml.Node[], _placeholders: {[id: string]: {[name: string]: string}}):
+  parse(nodes: ml.Node[], messageBundle: MessageBundle):
       {messages: {[k: string]: string}, errors: I18nError[]} {
-    this._messages = {};
+    this._messageNodes = [];
+    this._translatedMessages = {};
     this._bundleDepth = 0;
     this._translationDepth = 0;
     this._errors = [];
-    this._placeholders = _placeholders;
 
+    // Find all messages
     ml.visitAll(this, nodes, null);
 
-    return {messages: this._messages, errors: this._errors};
+    const messageMap = messageBundle.getMessageMap();
+    const placeholders = extractPlaceholders(messageBundle);
+    const placeholderToIds = extractPlaceholderToIds(messageBundle);
+
+    this._messageNodes
+        .filter(message => {
+          // Remove any messages that is not present in the source message bundle.
+          return messageMap.hasOwnProperty(message[0]);
+        })
+        .sort((a, b) => {
+          // Because there could be no ICU placeholders 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) {
+            return -1;
+          }
+
+          if (Object.keys(messageMap[b[0]].placeholderToMsgIds).length == 0) {
+            return 1;
+          }
+
+          return 0;
+        })
+        .forEach(message => {
+          const id = message[0];
+          this._placeholders = placeholders[id] || {};
+          this._placeholderToIds = placeholderToIds[id] || {};
+          // TODO(vicb): make sure there is no `_TRANSLATIONS_TAG` nor `_TRANSLATION_TAG`
+          this._translatedMessages[id] = ml.visitAll(this, message[1]).join('');
+        });
+
+    return {messages: this._translatedMessages, errors: this._errors};
   }
 
   visitElement(element: ml.Element, context: any): any {
@@ -101,8 +135,11 @@ class _Serializer implements ml.Visitor {
         if (!idAttr) {
           this._addError(element, `<${_TRANSLATION_TAG}> misses the "id" attribute`);
         } else {
-          this._currentPlaceholders = this._placeholders[idAttr.value] || {};
-          this._messages[idAttr.value] = ml.visitAll(this, element.children).join('');
+          // ICU placeholders are reference to other messages.
+          // The referenced message might not have been decoded yet.
+          // We need to have all messages available to make sure deps are decoded first.
+          // TODO(vicb): report an error on duplicate id
+          this._messageNodes.push([idAttr.value, element.children]);
         }
         this._translationDepth--;
         break;
@@ -112,11 +149,18 @@ class _Serializer implements ml.Visitor {
         if (!nameAttr) {
           this._addError(element, `<${_PLACEHOLDER_TAG}> misses the "name" attribute`);
         } else {
-          if (this._currentPlaceholders.hasOwnProperty(nameAttr.value)) {
-            return this._currentPlaceholders[nameAttr.value];
+          const name = nameAttr.value;
+          if (this._placeholders.hasOwnProperty(name)) {
+            return this._placeholders[name];
           }
+          if (this._placeholderToIds.hasOwnProperty(name) &&
+              this._translatedMessages.hasOwnProperty(this._placeholderToIds[name])) {
+            return this._translatedMessages[this._placeholderToIds[name]];
+          }
+          // TODO(vicb): better error message for when
+          // !this._translatedMessages.hasOwnProperty(this._placeholderToIds[name])
           this._addError(
-              element, `The placeholder "${nameAttr.value}" does not exists in the source message`);
+              element, `The placeholder "${name}" does not exists in the source message`);
         }
         break;
 

modules/@angular/compiler/src/i18n/translation_bundle.ts

@@ -7,8 +7,9 @@
  */
 
 import * as html from '../ml_parser/ast';
-import {Serializer} from './serializers/serializer';
 
+import {MessageBundle} from './message_bundle';
+import {Serializer} from './serializers/serializer';
 
 /**
  * A container for translated messages
@@ -16,10 +17,9 @@ import {Serializer} from './serializers/serializer';
 export class TranslationBundle {
   constructor(private _messageMap: {[id: string]: html.Node[]} = {}) {}
 
-  static load(
-      content: string, url: string, placeholders: {[id: string]: {[name: string]: string}},
-      serializer: Serializer): TranslationBundle {
-    return new TranslationBundle(serializer.load(content, url, placeholders));
+  static load(content: string, url: string, messageBundle: MessageBundle, serializer: Serializer):
+      TranslationBundle {
+    return new TranslationBundle(serializer.load(content, url, messageBundle));
   }
 
   get(id: string): html.Node[] { return this._messageMap[id]; }