fix(core): better handing of ICUs outside of i18n blocks (#35347)

Currently the logic that handles ICUs located outside of i18n blocks may throw exceptions at runtime. The problem is caused by the fact that we store incorrect TNode index for previous TNode (index that includes HEADER_OFFSET) and do not store a flag whether this TNode is a parent or a sibling node. As a result, the logic that assembles the final output uses incorrect TNodes and in some cases throws exceptions (when incompatible structure is extracted from tView.data due to the incorrect index). This commit adjusts the index and captures whether TNode is a parent to make sure underlying logic manipulates correct TNode.

PR Close #35347
This commit is contained in:
Andrew Kushnir 2020-02-11 14:44:13 -08:00 committed by Alex Rickabaugh
parent 7b56daffe6
commit c013dd4ca6
4 changed files with 125 additions and 12 deletions

View File

@ -390,11 +390,19 @@ function i18nStartFirstPass(
parentIndexStack[parentIndexPointer] = parentIndex;
const createOpCodes: I18nMutateOpCodes = [];
// If the previous node wasn't the direct parent then we have a translation without top level
// element and we need to keep a reference of the previous element if there is one
// element and we need to keep a reference of the previous element if there is one. We should also
// keep track whether an element was a parent node or not, so that the logic that consumes
// the generated `I18nMutateOpCode`s can leverage this information to properly set TNode state
// (whether it's a parent or sibling).
if (index > 0 && previousOrParentTNode !== parentTNode) {
let previousTNodeIndex = previousOrParentTNode.index - HEADER_OFFSET;
// If current TNode is a sibling node, encode it using a negative index. This information is
// required when the `Select` action is processed (see the `readCreateOpCodes` function).
if (!getIsParent()) {
previousTNodeIndex = ~previousTNodeIndex;
}
// Create an OpCode to select the previous TNode
createOpCodes.push(
previousOrParentTNode.index << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select);
createOpCodes.push(previousTNodeIndex << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select);
}
const updateOpCodes: I18nUpdateOpCodes = [];
const icuExpressions: TIcu[] = [];
@ -414,12 +422,16 @@ function i18nStartFirstPass(
}
} else {
const phIndex = parseInt(value.substr(1), 10);
// The value represents a placeholder that we move to the designated index
const isElement = value.charAt(0) === TagType.ELEMENT;
// The value represents a placeholder that we move to the designated index.
// Note: positive indicies indicate that a TNode with a given index should also be marked as
// parent while executing `Select` instruction.
createOpCodes.push(
phIndex << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
(isElement ? phIndex : ~phIndex) << I18nMutateOpCode.SHIFT_REF |
I18nMutateOpCode.Select,
parentIndex << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild);
if (value.charAt(0) === TagType.ELEMENT) {
if (isElement) {
parentIndexStack[++parentIndexPointer] = parentIndex = phIndex;
}
}
@ -757,12 +769,15 @@ function readCreateOpCodes(
appendI18nNode(tView, currentTNode !, destinationTNode, previousTNode, lView);
break;
case I18nMutateOpCode.Select:
const nodeIndex = opCode >>> I18nMutateOpCode.SHIFT_REF;
// Negative indicies indicate that a given TNode is a sibling node, not a parent node
// (see `i18nStartFirstPass` for additional information).
const isParent = opCode >= 0;
const nodeIndex = (isParent ? opCode : ~opCode) >>> I18nMutateOpCode.SHIFT_REF;
visitedNodes.push(nodeIndex);
previousTNode = currentTNode;
currentTNode = getTNode(tView, nodeIndex);
if (currentTNode) {
setPreviousOrParentTNode(currentTNode, currentTNode.type === TNodeType.Element);
setPreviousOrParentTNode(currentTNode, isParent);
}
break;
case I18nMutateOpCode.ElementEnd:

View File

@ -270,9 +270,9 @@ export function getPreviousOrParentTNode(): TNode {
return instructionState.lFrame.previousOrParentTNode;
}
export function setPreviousOrParentTNode(tNode: TNode, _isParent: boolean) {
export function setPreviousOrParentTNode(tNode: TNode, isParent: boolean) {
instructionState.lFrame.previousOrParentTNode = tNode;
instructionState.lFrame.isParent = _isParent;
instructionState.lFrame.isParent = isParent;
}
export function getIsParent(): boolean {

View File

@ -530,6 +530,36 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
expect(element).toHaveText('autre');
});
it('with no root node and text surrounding ICU', () => {
loadTranslations({
[computeMsgId('{VAR_SELECT, select, 10 {Ten} 20 {Twenty} other {Other}}')]:
'{VAR_SELECT, select, 10 {Dix} 20 {Vingt} other {Autre}}'
});
const fixture = initWithTemplate(AppComp, `
ICU start -->
{count, select, 10 {Ten} 20 {Twenty} other {Other}}
<-- ICU end
`);
const element = fixture.nativeElement;
expect(element.textContent).toContain('ICU start --> Autre <-- ICU end');
});
it('with no root node and text and DOM nodes surrounding ICU', () => {
loadTranslations({
[computeMsgId('{VAR_SELECT, select, 10 {Ten} 20 {Twenty} other {Other}}')]:
'{VAR_SELECT, select, 10 {Dix} 20 {Vingt} other {Autre}}'
});
const fixture = initWithTemplate(AppComp, `
<span>ICU start --> </span>
{count, select, 10 {Ten} 20 {Twenty} other {Other}}
<-- ICU end
`);
const element = fixture.nativeElement;
expect(element.textContent).toContain('ICU start --> Autre <-- ICU end');
});
it('with no i18n tag', () => {
loadTranslations({
[computeMsgId('{VAR_SELECT, select, 10 {ten} 20 {twenty} other {other}}')]:
@ -2100,6 +2130,73 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
"ng-reflect-ng-if": "false"
}--></div><button title="Close dialog">Button label</button>`);
});
describe('ngTemplateOutlet', () => {
it('should work with i18n content that includes elements', () => {
loadTranslations({
[computeMsgId('{$START_TAG_SPAN}A{$CLOSE_TAG_SPAN} B ')]:
'{$START_TAG_SPAN}a{$CLOSE_TAG_SPAN} b',
});
const fixture = initWithTemplate(AppComp, `
<ng-container *ngTemplateOutlet="tmpl"></ng-container>
<ng-template #tmpl i18n>
<span>A</span> B
</ng-template>
`);
expect(fixture.nativeElement.textContent).toContain('a b');
});
it('should work with i18n content that includes other templates (*ngIf)', () => {
loadTranslations({
[computeMsgId('{$START_TAG_SPAN}A{$CLOSE_TAG_SPAN} B ')]:
'{$START_TAG_SPAN}a{$CLOSE_TAG_SPAN} b',
});
const fixture = initWithTemplate(AppComp, `
<ng-container *ngTemplateOutlet="tmpl"></ng-container>
<ng-template #tmpl i18n>
<span *ngIf="visible">A</span> B
</ng-template>
`);
expect(fixture.nativeElement.textContent).toContain('a b');
});
it('should work with i18n content that includes projection', () => {
loadTranslations({
[computeMsgId('{$START_TAG_NG_CONTENT}{$CLOSE_TAG_NG_CONTENT} B ')]:
'{$START_TAG_NG_CONTENT}{$CLOSE_TAG_NG_CONTENT} b',
});
@Component({
selector: 'projector',
template: `
<ng-container *ngTemplateOutlet="tmpl"></ng-container>
<ng-template #tmpl i18n>
<ng-content></ng-content> B
</ng-template>
`
})
class Projector {
}
@Component({
selector: 'app',
template: `
<projector>a</projector>
`
})
class AppComponent {
}
TestBed.configureTestingModule({declarations: [AppComponent, Projector]});
const fixture = TestBed.createComponent(AppComponent);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toContain('a b');
});
});
});
function initWithTemplate(compType: Type<any>, template: string) {

View File

@ -194,6 +194,7 @@ describe('Runtime i18n', () => {
let nbConsts = 3;
let index = 1;
const firstTextNode = 3;
const rootTemplate = 2;
let opCodes = getOpCodes(() => { ɵɵi18nStart(index, MSG_DIV); }, null, nbConsts, index);
expect(opCodes).toEqual({
@ -202,7 +203,7 @@ describe('Runtime i18n', () => {
'',
nbConsts,
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild,
2 << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
~rootTemplate << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild,
'!',
nbConsts + 1,
@ -234,7 +235,7 @@ describe('Runtime i18n', () => {
'before',
nbConsts,
spanElement << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild,
bElementSubTemplate << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
~bElementSubTemplate << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
spanElement << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild,
'after',
nbConsts + 1,