From 24b7b2b8cb716adcb25905df9d64e5da43d41838 Mon Sep 17 00:00:00 2001
From: BADF00D <BADF00D@users.noreply.github.com>
Date: Wed, 12 Feb 2020 11:40:21 +0100
Subject: [PATCH] fix(directive): avoid recursive errors when using keys with
 whitespaces
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: David Störmer <David.Stoermer@copra-system.de>

Fixes #998 #1153 #1163
---
 .../core/src/lib/translate.directive.ts       |  19 ++-
 .../core/tests/translate.directive.spec.ts    | 109 ++++++++++++++----
 2 files changed, 100 insertions(+), 28 deletions(-)

diff --git a/projects/ngx-translate/core/src/lib/translate.directive.ts b/projects/ngx-translate/core/src/lib/translate.directive.ts
index 4e77b2cb..542b6940 100644
--- a/projects/ngx-translate/core/src/lib/translate.directive.ts
+++ b/projects/ngx-translate/core/src/lib/translate.directive.ts
@@ -69,17 +69,24 @@ export class TranslateDirective implements AfterViewChecked, OnDestroy {
       let node: any = nodes[i];
       if (node.nodeType === 3) { // node type 3 is a text node
         let key: string;
-        if (this.key) {
+        if (forceUpdate) {
+          node.lastKey = null;
+        }
+        if(isDefined(node.lookupKey)) {
+          key = node.lookupKey;
+        } else if (this.key) {
           key = this.key;
-          if (forceUpdate) {
-            node.lastKey = null;
-          }
         } else {
           let content = this.getContent(node);
           let trimmedContent = content.trim();
           if (trimmedContent.length) {
-            if (node.originalContent && forceUpdate) { // the content seems ok, but the lang has changed
-              node.lastKey = null;
+            node.lookupKey = trimmedContent;
+            // we want to use the content as a key, not the translation value
+            if (content !== node.currentValue) {
+              key = trimmedContent;
+              // the content was changed from the user, we'll use it as a reference if needed
+              node.originalContent = content || node.originalContent;
+            } else if (node.originalContent) { // the content seems ok, but the lang has changed
               // the current content is the translation, not the key, use the last real content as key
               key = node.originalContent.trim();
             } else if (content !== node.currentValue) {
diff --git a/projects/ngx-translate/core/tests/translate.directive.spec.ts b/projects/ngx-translate/core/tests/translate.directive.spec.ts
index 983e8495..c2216df3 100644
--- a/projects/ngx-translate/core/tests/translate.directive.spec.ts
+++ b/projects/ngx-translate/core/tests/translate.directive.spec.ts
@@ -7,24 +7,32 @@ import {TranslateModule, TranslateService} from '../src/public_api';
   selector: 'hmx-app',
   changeDetection: ChangeDetectionStrategy.OnPush,
   template: `
-      <div #noKey translate>
-        TEST
-      </div>
-      <div #withKey [translate]="'TEST'">Some init content</div>
-      <div #noContent [translate]="'TEST'"></div>
-      <div #withOtherElements translate>TEST1 <span>Hey</span> TEST2</div>
-      <div #withParams [translate]="'TEST'" [translateParams]="value">Some init content</div>
-      <div #withParamsNoKey translate [translateParams]="value">TEST</div>
+    <div #noKey translate>TEST</div>
+    <div #contentAsKey translate>TEST.VALUE</div>
+    <div #withKey [translate]="'TEST'">Some init content</div>
+    <div #noContent [translate]="'TEST'"></div>
+    <div #withOtherElements translate>TEST1 <span>Hey</span> TEST2</div>
+    <div #withParams [translate]="'TEST'" [translateParams]="value">Some init content</div>
+    <div #withParamsNoKey translate [translateParams]="value">TEST</div>
+    <div #leadingSpaceNoKeyNoParams translate> TEST</div>
+    <div #trailingSpaceNoKeyNoParams translate>TEST </div>
+    <div #withSpaceAndLineBreakNoKeyNoParams translate>
+      TEST
+    </div>
   `
 })
 class App {
   viewContainerRef: ViewContainerRef;
   @ViewChild('noKey', {static: true}) noKey: ElementRef;
+  @ViewChild('contentAsKey', {static: true}) contentAsKey: ElementRef;
   @ViewChild('withKey', {static: true}) withKey: ElementRef;
   @ViewChild('withOtherElements', {static: true}) withOtherElements: ElementRef;
   @ViewChild('withParams', {static: true}) withParams: ElementRef;
   @ViewChild('withParamsNoKey', {static: true}) withParamsNoKey: ElementRef;
   @ViewChild('noContent', {static: true}) noContent: ElementRef;
+  @ViewChild('leadingSpaceNoKeyNoParams') leadingSpaceNoKeyNoParams: ElementRef;
+  @ViewChild('trailingSpaceNoKeyNoParams') trailingSpaceNoKeyNoParams: ElementRef;
+  @ViewChild('withSpaceAndLineBreakNoKeyNoParams') withSpaceAndLineBreakNoKeyNoParams: ElementRef;
   value = {value: 'ok'};
 
   constructor(viewContainerRef: ViewContainerRef) {
@@ -43,7 +51,7 @@ describe('TranslateDirective', () => {
       ],
       declarations: [App]
     });
-    translate = TestBed.get(TranslateService);
+    translate = TestBed.inject(TranslateService);
 
     fixture = (<any>TestBed).createComponent(App);
     fixture.detectChanges();
@@ -55,12 +63,21 @@ describe('TranslateDirective', () => {
   });
 
   it('should translate a string using the container value', () => {
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' TEST ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('TEST');
 
     translate.setTranslation('en', {"TEST": "This is a test"});
     translate.use('en');
 
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' This is a test ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('This is a test');
+  });
+
+  it('should translate a string using the container value as a key', () => {
+    expect(fixture.componentInstance.contentAsKey.nativeElement.innerHTML).toEqual('TEST.VALUE');
+
+    translate.setTranslation('en', {"TEST": {"VALUE": "This is a test"}});
+    translate.use('en');
+
+    expect(fixture.componentInstance.contentAsKey.nativeElement.innerHTML).toEqual('This is a test');
   });
 
   it('should translate a string using the key value', () => {
@@ -130,8 +147,56 @@ describe('TranslateDirective', () => {
     expect(fixture.componentInstance.withParamsNoKey.nativeElement.innerHTML).toEqual('It is changed');
   });
 
+  it('should update the DOM when the lang changes and the translation key starts with space', () => {
+    expect(fixture.componentInstance.leadingSpaceNoKeyNoParams.nativeElement.innerHTML).toEqual(' TEST');
+
+    const en = "This is a test - with leading spaces in translation key";
+    const fr = "C'est un test - avec un espace de tête dans la clé de traduction";
+    const leadingSpaceFromKey = ' ';
+    translate.setTranslation('en', {"TEST": en});
+    translate.setTranslation('fr', {"TEST": fr});
+
+    translate.use('en');
+    expect(fixture.componentInstance.leadingSpaceNoKeyNoParams.nativeElement.innerHTML).toEqual(leadingSpaceFromKey + en);
+
+    translate.use('fr');
+    expect(fixture.componentInstance.leadingSpaceNoKeyNoParams.nativeElement.innerHTML).toEqual(leadingSpaceFromKey + fr);
+  });
+
+  it('should update the DOM when the lang changes and the translation key has line breaks and spaces', () => {
+    expect(fixture.componentInstance.withSpaceAndLineBreakNoKeyNoParams.nativeElement.innerHTML).toEqual(' TEST ');
+
+    const en = "This is a test - with trailing spaces in translation key";
+    const fr = "C'est un test - avec un espace de fuite dans la clé de traduction";
+    const whiteSpaceFromKey = ' ';
+    translate.setTranslation('en', {"TEST": en});
+    translate.setTranslation('fr', {"TEST": fr});
+
+    translate.use('en');
+    expect(fixture.componentInstance.withSpaceAndLineBreakNoKeyNoParams.nativeElement.innerHTML).toEqual(whiteSpaceFromKey + en + whiteSpaceFromKey);
+
+    translate.use('fr');
+    expect(fixture.componentInstance.withSpaceAndLineBreakNoKeyNoParams.nativeElement.innerHTML).toEqual(whiteSpaceFromKey + fr + whiteSpaceFromKey);
+  });
+
+  it('should update the DOM when the lang changes and the translation key ends with space', () => {
+    expect(fixture.componentInstance.trailingSpaceNoKeyNoParams.nativeElement.innerHTML).toEqual('TEST ');
+
+    const en = "This is a test - with spaces and line breaks in translation key";
+    const fr = "C'est un test - avec des espaces et sauts de lignes dans la clé de traduction";
+    const trailingSpaceFromKey = ' ';
+    translate.setTranslation('en', {"TEST": en});
+    translate.setTranslation('fr', {"TEST": fr});
+
+    translate.use('en');
+    expect(fixture.componentInstance.trailingSpaceNoKeyNoParams.nativeElement.innerHTML).toEqual(en + trailingSpaceFromKey);
+
+    translate.use('fr');
+    expect(fixture.componentInstance.trailingSpaceNoKeyNoParams.nativeElement.innerHTML).toEqual(fr + trailingSpaceFromKey);
+  });
+
   it('should update the DOM when the lang changes', () => {
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' TEST ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('TEST');
     expect(fixture.componentInstance.withParams.nativeElement.innerHTML).toEqual('TEST');
     expect(fixture.componentInstance.noContent.nativeElement.innerHTML).toEqual('TEST');
 
@@ -139,48 +204,48 @@ describe('TranslateDirective', () => {
     translate.setTranslation('fr', {"TEST": "C'est un test"});
 
     translate.use('en');
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' This is a test ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('This is a test');
     expect(fixture.componentInstance.withParams.nativeElement.innerHTML).toEqual('This is a test');
     expect(fixture.componentInstance.noContent.nativeElement.innerHTML).toEqual('This is a test');
 
     translate.use('fr');
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(" C'est un test ");
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual("C'est un test");
     expect(fixture.componentInstance.withParams.nativeElement.innerHTML).toEqual("C'est un test");
     expect(fixture.componentInstance.noContent.nativeElement.innerHTML).toEqual("C'est un test");
   });
 
   it('should update the DOM when the lang changes and the translation ends with space', () => {
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' TEST ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('TEST');
     expect(fixture.componentInstance.withParams.nativeElement.innerHTML).toEqual('TEST');
     expect(fixture.componentInstance.noContent.nativeElement.innerHTML).toEqual('TEST');
 
-    const en="  This is a test - with spaces ";
-    const fr="  C'est un test - pardon, je ne parle pas francais :) ";
+    const en = "  This is a test - with spaces ";
+    const fr = "  C'est un test - avec espaces ";
 
     translate.setTranslation('en', {"TEST": en});
     translate.setTranslation('fr', {"TEST": fr});
 
     translate.use('en');
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(` ${en} `);
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(`${en}`);
     expect(fixture.componentInstance.withParams.nativeElement.innerHTML).toEqual(en);
     expect(fixture.componentInstance.noContent.nativeElement.innerHTML).toEqual(en);
 
     translate.use('fr');
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(` ${fr} `);
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(`${fr}`);
     expect(fixture.componentInstance.withParams.nativeElement.innerHTML).toEqual(fr);
     expect(fixture.componentInstance.noContent.nativeElement.innerHTML).toEqual(fr);
   });
 
   it('should update the DOM when the default lang changes', () => {
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' TEST ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('TEST');
 
     translate.setTranslation('en', {"TEST": "This is a test"});
     translate.setTranslation('fr', {"TEST": "C'est un test"});
     translate.setDefaultLang('en');
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(' This is a test ');
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual('This is a test');
 
     translate.setDefaultLang('fr');
-    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual(" C'est un test ");
+    expect(fixture.componentInstance.noKey.nativeElement.innerHTML).toEqual("C'est un test");
   });
 
   it('should unsubscribe from lang change subscription on destroy', () => {