fix(core): handle multiple i18n attributes with expression bindings (#41882)

When there are multiple attributes that are marked for i18n translation,
which contain expression bindings, we were generating i18n update op-codes
that did not accurately map to the correct value to be bound in the lView.
Each attribute's bindings were relative to the start of the lView first
attributes values rather than to their own.

This fix passes the current binding index to `generateBindingUpdateOpCodes()`
when processing i18n attributes to account for this.

Fixes #41869

PR Close #41882
This commit is contained in:
Pete Bacon Darwin 2021-04-29 16:29:14 +01:00 committed by Misko Hevery
parent 72384a3357
commit 9b4b281c52
3 changed files with 57 additions and 22 deletions

View File

@ -219,7 +219,7 @@ function i18nStartFirstCreatePassProcessTextNode(
const tNode = createTNodeAndAddOpCode( const tNode = createTNodeAndAddOpCode(
tView, rootTNode, existingTNodes, lView, createOpCodes, hasBinding ? null : text, false); tView, rootTNode, existingTNodes, lView, createOpCodes, hasBinding ? null : text, false);
if (hasBinding) { if (hasBinding) {
generateBindingUpdateOpCodes(updateOpCodes, text, tNode.index); generateBindingUpdateOpCodes(updateOpCodes, text, tNode.index, null, 0, null);
} }
} }
@ -251,7 +251,11 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str
// i18n attributes that hit this code path are guaranteed to have bindings, because // i18n attributes that hit this code path are guaranteed to have bindings, because
// the compiler treats static i18n attributes as regular attribute bindings. // the compiler treats static i18n attributes as regular attribute bindings.
generateBindingUpdateOpCodes(updateOpCodes, message, previousElementIndex, attrName); // Since this may not be the first i18n attribute on this element we need to pass in how
// many previous bindings there have already been.
generateBindingUpdateOpCodes(
updateOpCodes, message, previousElementIndex, attrName, countBindings(updateOpCodes),
null);
} }
} }
tView.data[index] = updateOpCodes; tView.data[index] = updateOpCodes;
@ -267,10 +271,12 @@ export function i18nAttributesFirstPass(tView: TView, index: number, values: str
* @param destinationNode Index of the destination node which will receive the binding. * @param destinationNode Index of the destination node which will receive the binding.
* @param attrName Name of the attribute, if the string belongs to an attribute. * @param attrName Name of the attribute, if the string belongs to an attribute.
* @param sanitizeFn Sanitization function used to sanitize the string after update, if necessary. * @param sanitizeFn Sanitization function used to sanitize the string after update, if necessary.
* @param bindingStart The lView index of the next expression that can be bound via an opCode.
* @returns The mask value for these bindings
*/ */
export function generateBindingUpdateOpCodes( function generateBindingUpdateOpCodes(
updateOpCodes: I18nUpdateOpCodes, str: string, destinationNode: number, attrName?: string, updateOpCodes: I18nUpdateOpCodes, str: string, destinationNode: number, attrName: string|null,
sanitizeFn: SanitizerFn|null = null): number { bindingStart: number, sanitizeFn: SanitizerFn|null): number {
ngDevMode && ngDevMode &&
assertGreaterThanOrEqual( assertGreaterThanOrEqual(
destinationNode, HEADER_OFFSET, 'Index must be in absolute LView offset'); destinationNode, HEADER_OFFSET, 'Index must be in absolute LView offset');
@ -289,7 +295,7 @@ export function generateBindingUpdateOpCodes(
if (j & 1) { if (j & 1) {
// Odd indexes are bindings // Odd indexes are bindings
const bindingIndex = parseInt(textValue, 10); const bindingIndex = bindingStart + parseInt(textValue, 10);
updateOpCodes.push(-1 - bindingIndex); updateOpCodes.push(-1 - bindingIndex);
mask = mask | toMaskBit(bindingIndex); mask = mask | toMaskBit(bindingIndex);
} else if (textValue !== '') { } else if (textValue !== '') {
@ -309,6 +315,28 @@ export function generateBindingUpdateOpCodes(
return mask; return mask;
} }
/**
* Count the number of bindings in the given `opCodes`.
*
* It could be possible to speed this up, by passing the number of bindings found back from
* `generateBindingUpdateOpCodes()` to `i18nAttributesFirstPass()` but this would then require more
* complexity in the code and/or transient objects to be created.
*
* Since this function is only called once when the template is instantiated, is trivial in the
* first instance (since `opCodes` will be an empty array), and it is not common for elements to
* contain multiple i18n bound attributes, it seems like this is a reasonable compromise.
*/
function countBindings(opCodes: I18nUpdateOpCodes): number {
let count = 0;
for (let i = 0; i < opCodes.length; i++) {
const opCode = opCodes[i];
// Bindings are negative numbers.
if (typeof opCode === 'number' && opCode < 0) {
count++;
}
}
return count;
}
/** /**
* Convert binding index to mask bit. * Convert binding index to mask bit.
@ -595,12 +623,12 @@ function walkIcuTree(
if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) { if (VALID_ATTRS.hasOwnProperty(lowerAttrName)) {
if (URI_ATTRS[lowerAttrName]) { if (URI_ATTRS[lowerAttrName]) {
generateBindingUpdateOpCodes( generateBindingUpdateOpCodes(
update, attr.value, newIndex, attr.name, _sanitizeUrl); update, attr.value, newIndex, attr.name, 0, _sanitizeUrl);
} else if (SRCSET_ATTRS[lowerAttrName]) { } else if (SRCSET_ATTRS[lowerAttrName]) {
generateBindingUpdateOpCodes( generateBindingUpdateOpCodes(
update, attr.value, newIndex, attr.name, sanitizeSrcset); update, attr.value, newIndex, attr.name, 0, sanitizeSrcset);
} else { } else {
generateBindingUpdateOpCodes(update, attr.value, newIndex, attr.name); generateBindingUpdateOpCodes(update, attr.value, newIndex, attr.name, 0, null);
} }
} else { } else {
ngDevMode && ngDevMode &&
@ -627,7 +655,8 @@ function walkIcuTree(
addCreateNodeAndAppend(create, null, hasBinding ? '' : value, parentIdx, newIndex); addCreateNodeAndAppend(create, null, hasBinding ? '' : value, parentIdx, newIndex);
addRemoveNode(remove, newIndex, depth); addRemoveNode(remove, newIndex, depth);
if (hasBinding) { if (hasBinding) {
bindingMask = generateBindingUpdateOpCodes(update, value, newIndex) | bindingMask; bindingMask =
generateBindingUpdateOpCodes(update, value, newIndex, null, 0, null) | bindingMask;
} }
break; break;
case Node.COMMENT_NODE: case Node.COMMENT_NODE:

View File

@ -1624,17 +1624,23 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
}); });
it('multiple attributes', () => { it('multiple attributes', () => {
loadTranslations({[computeMsgId('hello {$INTERPOLATION}')]: 'bonjour {$INTERPOLATION}'}); loadTranslations({
[computeMsgId('hello {$INTERPOLATION} - {$INTERPOLATION_1}')]:
'bonjour {$INTERPOLATION} - {$INTERPOLATION_1}',
[computeMsgId('bye {$INTERPOLATION} - {$INTERPOLATION_1}')]:
'au revoir {$INTERPOLATION} - {$INTERPOLATION_1}',
});
const fixture = initWithTemplate( const fixture = initWithTemplate(
AppComp, AppComp,
`<input i18n i18n-title title="hello {{name}}" i18n-placeholder placeholder="hello {{name}}">`); `<input i18n i18n-title title="hello {{name}} - {{count}}" i18n-placeholder placeholder="bye {{count}} - {{name}}">`);
expect(fixture.nativeElement.innerHTML) expect(fixture.nativeElement.innerHTML)
.toEqual(`<input title="bonjour Angular" placeholder="bonjour Angular">`); .toEqual(`<input title="bonjour Angular - 0" placeholder="au revoir 0 - Angular">`);
fixture.componentRef.instance.name = 'John'; fixture.componentRef.instance.name = 'John';
fixture.componentRef.instance.count = 5;
fixture.detectChanges(); fixture.detectChanges();
expect(fixture.nativeElement.innerHTML) expect(fixture.nativeElement.innerHTML)
.toEqual(`<input title="bonjour John" placeholder="bonjour John">`); .toEqual(`<input title="bonjour John - 5" placeholder="au revoir 5 - John">`);
}); });
it('on removed elements', () => { it('on removed elements', () => {

View File

@ -458,9 +458,10 @@ describe('Runtime i18n', () => {
}); });
it('for multiple attributes', () => { it('for multiple attributes', () => {
const message = `Hello <20>0<EFBFBD>!`; const message1 = `Hello <20>0<EFBFBD> - <20>1<EFBFBD>!`;
const attrs = ['title', message, 'aria-label', message]; const message2 = `Bye <20>0<EFBFBD> - <20>1<EFBFBD>!`;
const nbConsts = 2; const attrs = ['title', message1, 'aria-label', message2];
const nbConsts = 4;
const index = 1; const index = 1;
const opCodes = getOpCodes(attrs, () => { const opCodes = getOpCodes(attrs, () => {
ɵɵelementStart(0, 'div'); ɵɵelementStart(0, 'div');
@ -469,11 +470,10 @@ describe('Runtime i18n', () => {
}, undefined, nbConsts, HEADER_OFFSET + index); }, undefined, nbConsts, HEADER_OFFSET + index);
expect(opCodes).toEqual(matchDebug([ expect(opCodes).toEqual(matchDebug([
`if (mask & 0b1) { (lView[${ `if (mask & 0b11) { (lView[${HEADER_OFFSET + 0}] as Element).` +
HEADER_OFFSET + 0}] as Element).setAttribute('title', \`Hello \$\{lView[i-1]}!\`); }`, 'setAttribute(\'title\', `Hello ${lView[i-1]} - ${lView[i-2]}!`); }',
`if (mask & 0b1) { (lView[${ `if (mask & 0b1100) { (lView[${HEADER_OFFSET + 0}] as Element).` +
HEADER_OFFSET + 'setAttribute(\'aria-label\', `Bye ${lView[i-3]} - ${lView[i-4]}!`); }',
0}] as Element).setAttribute('aria-label', \`Hello \$\{lView[i-1]}!\`); }`,
])); ]));
}); });
}); });