Skip to content

Commit 6efa366

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
fix(compiler): compute correct offsets when interpolations have HTML entities (#44811)
When parsing interpolations, the input string is _decoded_ from what was in the orginal template. This means that we cannot soley rely on the input string to compute source spans because it does not necessarily reflect the exact content of the original template. Specifically, when there is an HTML entity (i.e. `&nbsp;`), this will show up in its decoded form when processing the interpolation (' '). We need to compute offsets using the original _encoded_ string. Note that this problem only surfaces in the splitting of interpolations. The spans to this point have already been tracked accurately. For example, given the template `&nbsp;<div></div>`, the source span for the `div` is already correctly determined to be 6. Only when we encounter interpolations with many parts do we run into situations where we need to compute new spans for the individual parts of the interpolation. PR Close #44811
1 parent ad9d981 commit 6efa366

File tree

7 files changed

+131
-19
lines changed

7 files changed

+131
-19
lines changed

‎packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,5 +889,26 @@ runInEachFileSystem(() => {
889889
}
890890
]));
891891
});
892+
893+
it('should handle interpolations in attributes, preceded by HTML entity', () => {
894+
const template = `<img src="&nbsp;{{foo}}" />`;
895+
const refs = getTemplateIdentifiers(bind(template));
896+
897+
expect(Array.from(refs)).toEqual([
898+
{
899+
kind: IdentifierKind.Element,
900+
name: 'img',
901+
span: new AbsoluteSourceSpan(1, 4),
902+
usedDirectives: new Set(),
903+
attributes: new Set(),
904+
},
905+
{
906+
kind: IdentifierKind.Property,
907+
name: 'foo',
908+
span: new AbsoluteSourceSpan(18, 21),
909+
target: null,
910+
}
911+
]);
912+
});
892913
});
893914
});

‎packages/compiler/src/expression_parser/parser.ts‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import * as chars from '../chars';
1010
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config';
11+
import {InterpolatedAttributeToken, InterpolatedTextToken, TokenType as MlParserTokenType} from '../ml_parser/tokens';
1112

1213
import {AbsoluteSourceSpan, AST, ASTWithSource, Binary, BindingPipe, Call, Chain, Conditional, EmptyExpr, ExpressionBinding, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast';
1314
import {EOF, isIdentifier, Lexer, Token, TokenType} from './lexer';
@@ -167,9 +168,10 @@ export class Parser {
167168

168169
parseInterpolation(
169170
input: string, location: string, absoluteOffset: number,
171+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null,
170172
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource|null {
171173
const {strings, expressions, offsets} =
172-
this.splitInterpolation(input, location, interpolationConfig);
174+
this.splitInterpolation(input, location, interpolatedTokens, interpolationConfig);
173175
if (expressions.length === 0) return null;
174176

175177
const expressionNodes: AST[] = [];
@@ -223,10 +225,13 @@ export class Parser {
223225
*/
224226
splitInterpolation(
225227
input: string, location: string,
228+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null,
226229
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): SplitInterpolation {
227230
const strings: InterpolationPiece[] = [];
228231
const expressions: InterpolationPiece[] = [];
229232
const offsets: number[] = [];
233+
const inputToTemplateIndexMap =
234+
interpolatedTokens ? getIndexMapForOriginalTemplate(interpolatedTokens) : null;
230235
let i = 0;
231236
let atInterpolation = false;
232237
let extendLastString = false;
@@ -264,7 +269,9 @@ export class Parser {
264269
`at column ${i} in`, location);
265270
}
266271
expressions.push({text, start: fullStart, end: fullEnd});
267-
offsets.push(exprStart);
272+
const startInOriginalTemplate = inputToTemplateIndexMap?.get(fullStart) ?? fullStart;
273+
const offset = startInOriginalTemplate + interpStart.length;
274+
offsets.push(offset);
268275

269276
i = fullEnd;
270277
atInterpolation = false;
@@ -1334,3 +1341,38 @@ class SimpleExpressionChecker extends RecursiveAstVisitor {
13341341
this.errors.push('pipes');
13351342
}
13361343
}
1344+
/**
1345+
* Computes the real offset in the original template for indexes in an interpolation.
1346+
*
1347+
* Because templates can have encoded HTML entities and the input passed to the parser at this stage
1348+
* of the compiler is the _decoded_ value, we need to compute the real offset using the original
1349+
* encoded values in the interpolated tokens. Note that this is only a special case handling for
1350+
* `MlParserTokenType.ENCODED_ENTITY` token types. All other interpolated tokens are expected to
1351+
* have parts which exactly match the input string for parsing the interpolation.
1352+
*
1353+
* @param interpolatedTokens The tokens for the interpolated value.
1354+
*
1355+
* @returns A map of index locations in the decoded template to indexes in the original template
1356+
*/
1357+
function getIndexMapForOriginalTemplate(interpolatedTokens: InterpolatedAttributeToken[]|
1358+
InterpolatedTextToken[]): Map<number, number> {
1359+
let offsetMap = new Map<number, number>();
1360+
let consumedInOriginalTemplate = 0;
1361+
let consumedInInput = 0;
1362+
let tokenIndex = 0;
1363+
while (tokenIndex < interpolatedTokens.length) {
1364+
const currentToken = interpolatedTokens[tokenIndex];
1365+
if (currentToken.type === MlParserTokenType.ENCODED_ENTITY) {
1366+
const [decoded, encoded] = currentToken.parts;
1367+
consumedInOriginalTemplate += encoded.length;
1368+
consumedInInput += decoded.length;
1369+
} else {
1370+
const lengthOfParts = currentToken.parts.reduce((sum, current) => sum + current.length, 0);
1371+
consumedInInput += lengthOfParts;
1372+
consumedInOriginalTemplate += lengthOfParts;
1373+
}
1374+
offsetMap.set(consumedInInput, consumedInOriginalTemplate);
1375+
tokenIndex++;
1376+
}
1377+
return offsetMap;
1378+
}

‎packages/compiler/src/render3/r3_template_transform.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as i18n from '../i18n/i18n_ast';
1111
import * as html from '../ml_parser/ast';
1212
import {replaceNgsp} from '../ml_parser/html_whitespaces';
1313
import {isNgTemplate} from '../ml_parser/tags';
14+
import {InterpolatedAttributeToken, InterpolatedTextToken} from '../ml_parser/tokens';
1415
import {ParseError, ParseErrorLevel, ParseSourceSpan} from '../parse_util';
1516
import {isStyleUrlResolvable} from '../style_url_resolver';
1617
import {BindingParser} from '../template_parser/binding_parser';
@@ -255,7 +256,7 @@ class HtmlAstToIvyAst implements html.Visitor {
255256
}
256257

257258
visitText(text: html.Text): t.Node {
258-
return this._visitTextWithInterpolation(text.value, text.sourceSpan, text.i18n);
259+
return this._visitTextWithInterpolation(text.value, text.sourceSpan, text.tokens, text.i18n);
259260
}
260261

261262
visitExpansion(expansion: html.Expansion): t.Icu|null {
@@ -288,7 +289,7 @@ class HtmlAstToIvyAst implements html.Visitor {
288289

289290
vars[formattedKey] = new t.BoundText(ast, value.sourceSpan);
290291
} else {
291-
placeholders[key] = this._visitTextWithInterpolation(value.text, value.sourceSpan);
292+
placeholders[key] = this._visitTextWithInterpolation(value.text, value.sourceSpan, null);
292293
}
293294
});
294295
return new t.Icu(vars, placeholders, expansion.sourceSpan, message);
@@ -443,14 +444,17 @@ class HtmlAstToIvyAst implements html.Visitor {
443444
// No explicit binding found.
444445
const keySpan = createKeySpan(srcSpan, '' /* prefix */, name);
445446
const hasBinding = this.bindingParser.parsePropertyInterpolation(
446-
name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan);
447+
name, value, srcSpan, attribute.valueSpan, matchableAttributes, parsedProperties, keySpan,
448+
attribute.valueTokens ?? null);
447449
return hasBinding;
448450
}
449451

450452
private _visitTextWithInterpolation(
451-
value: string, sourceSpan: ParseSourceSpan, i18n?: i18n.I18nMeta): t.Text|t.BoundText {
453+
value: string, sourceSpan: ParseSourceSpan,
454+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null,
455+
i18n?: i18n.I18nMeta): t.Text|t.BoundText {
452456
const valueNoNgsp = replaceNgsp(value);
453-
const expr = this.bindingParser.parseInterpolation(valueNoNgsp, sourceSpan);
457+
const expr = this.bindingParser.parseInterpolation(valueNoNgsp, sourceSpan, interpolatedTokens);
454458
return expr ? new t.BoundText(expr, sourceSpan, i18n) : new t.Text(valueNoNgsp, sourceSpan);
455459
}
456460

‎packages/compiler/src/template_parser/binding_parser.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, BindingType, BoundElemen
1111
import {Parser} from '../expression_parser/parser';
1212
import {InterpolationConfig} from '../ml_parser/interpolation_config';
1313
import {mergeNsAndName} from '../ml_parser/tags';
14+
import {InterpolatedAttributeToken, InterpolatedTextToken} from '../ml_parser/tokens';
1415
import {ParseError, ParseErrorLevel, ParseLocation, ParseSourceSpan} from '../parse_util';
1516
import {ElementSchemaRegistry} from '../schema/element_schema_registry';
1617
import {CssSelector} from '../selector';
@@ -95,13 +96,16 @@ export class BindingParser {
9596
return targetEvents;
9697
}
9798

98-
parseInterpolation(value: string, sourceSpan: ParseSourceSpan): ASTWithSource {
99+
parseInterpolation(
100+
value: string, sourceSpan: ParseSourceSpan,
101+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|
102+
null): ASTWithSource {
99103
const sourceInfo = sourceSpan.start.toString();
100104
const absoluteOffset = sourceSpan.fullStart.offset;
101105

102106
try {
103107
const ast = this._exprParser.parseInterpolation(
104-
value, sourceInfo, absoluteOffset, this._interpolationConfig)!;
108+
value, sourceInfo, absoluteOffset, interpolatedTokens, this._interpolationConfig)!;
105109
if (ast) this._reportExpressionParserErrors(ast.errors, sourceSpan);
106110
return ast;
107111
} catch (e) {
@@ -275,8 +279,9 @@ export class BindingParser {
275279
parsePropertyInterpolation(
276280
name: string, value: string, sourceSpan: ParseSourceSpan,
277281
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
278-
targetProps: ParsedProperty[], keySpan: ParseSourceSpan): boolean {
279-
const expr = this.parseInterpolation(value, valueSpan || sourceSpan);
282+
targetProps: ParsedProperty[], keySpan: ParseSourceSpan,
283+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null): boolean {
284+
const expr = this.parseInterpolation(value, valueSpan || sourceSpan, interpolatedTokens);
280285
if (expr) {
281286
this._parsePropertyAst(
282287
name, expr, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);

‎packages/compiler/test/expression_parser/parser_spec.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,7 +1012,8 @@ describe('parser', () => {
10121012

10131013
it('should support custom interpolation', () => {
10141014
const parser = new Parser(new Lexer());
1015-
const ast = parser.parseInterpolation('{% a %}', '', 0, {start: '{%', end: '%}'})!.ast as any;
1015+
const ast =
1016+
parser.parseInterpolation('{% a %}', '', 0, null, {start: '{%', end: '%}'})!.ast as any;
10161017
expect(ast.strings).toEqual(['', '']);
10171018
expect(ast.expressions.length).toEqual(1);
10181019
expect(ast.expressions[0].name).toEqual('a');
@@ -1211,11 +1212,11 @@ function _parseTemplateBindings(attribute: string, templateUrl: string) {
12111212

12121213
function parseInterpolation(text: string, location: any = null, offset: number = 0): ASTWithSource|
12131214
null {
1214-
return createParser().parseInterpolation(text, location, offset);
1215+
return createParser().parseInterpolation(text, location, offset, null);
12151216
}
12161217

12171218
function splitInterpolation(text: string, location: any = null): SplitInterpolation|null {
1218-
return createParser().splitInterpolation(text, location);
1219+
return createParser().splitInterpolation(text, location, null);
12191220
}
12201221

12211222
function parseSimpleBinding(text: string, location: any = null, offset: number = 0): ASTWithSource {

‎packages/compiler/test/render3/r3_ast_absolute_span_spec.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,45 @@ describe('expression AST absolute source spans', () => {
157157
['2', new AbsoluteSourceSpan(11, 12)],
158158
]));
159159
});
160+
161+
it('should handle HTML entity before interpolation', () => {
162+
expect(humanizeExpressionSource(parse('&nbsp;{{abc}}').nodes))
163+
.toEqual(jasmine.arrayContaining([
164+
['abc', new AbsoluteSourceSpan(8, 11)],
165+
]));
166+
});
167+
168+
it('should handle many HTML entities and many interpolations', () => {
169+
expect(humanizeExpressionSource(parse('&quot;{{abc}}&quot;{{def}}&nbsp;{{ghi}}').nodes))
170+
.toEqual(jasmine.arrayContaining([
171+
['abc', new AbsoluteSourceSpan(8, 11)],
172+
['def', new AbsoluteSourceSpan(21, 24)],
173+
['ghi', new AbsoluteSourceSpan(34, 37)],
174+
]));
175+
});
176+
177+
it('should handle interpolation in attribute', () => {
178+
expect(humanizeExpressionSource(parse('<div class="{{abc}}"><div>').nodes))
179+
.toEqual(jasmine.arrayContaining([
180+
['abc', new AbsoluteSourceSpan(14, 17)],
181+
]));
182+
});
183+
184+
it('should handle interpolation preceded by HTML entity in attribute', () => {
185+
expect(humanizeExpressionSource(parse('<div class="&nbsp;{{abc}}"><div>').nodes))
186+
.toEqual(jasmine.arrayContaining([
187+
['abc', new AbsoluteSourceSpan(20, 23)],
188+
]));
189+
});
190+
191+
it('should handle many interpolation with HTML entities in attribute', () => {
192+
expect(humanizeExpressionSource(
193+
parse('<div class="&quot;{{abc}}&quot;&nbsp;{{def}}"><div>').nodes))
194+
.toEqual(jasmine.arrayContaining([
195+
['abc', new AbsoluteSourceSpan(20, 23)],
196+
['def', new AbsoluteSourceSpan(39, 42)],
197+
]));
198+
});
160199
});
161200

162201
describe('keyed read', () => {

‎packages/compiler/test/render3/view/i18n_spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ describe('I18nContext', () => {
5252

5353
// binding collection checks
5454
expect(ctx.bindings.size).toBe(0);
55-
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0) as AST);
56-
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0) as AST);
55+
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0, null) as AST);
56+
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0, null) as AST);
5757
expect(ctx.bindings.size).toBe(2);
5858
});
5959

@@ -80,7 +80,7 @@ describe('I18nContext', () => {
8080

8181
// set data for root ctx
8282
ctx.appendBoundText(i18nOf(boundTextA));
83-
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0) as AST);
83+
ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0, null) as AST);
8484
ctx.appendElement(i18nOf(elementA), 0);
8585
ctx.appendTemplate(i18nOf(templateA), 1);
8686
ctx.appendElement(i18nOf(elementA), 0, true);
@@ -96,11 +96,11 @@ describe('I18nContext', () => {
9696
// set data for child context
9797
childCtx.appendElement(i18nOf(elementB), 0);
9898
childCtx.appendBoundText(i18nOf(boundTextB));
99-
childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0) as AST);
99+
childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0, null) as AST);
100100
childCtx.appendElement(i18nOf(elementC), 1);
101101
childCtx.appendElement(i18nOf(elementC), 1, true);
102102
childCtx.appendBoundText(i18nOf(boundTextC));
103-
childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '', 0) as AST);
103+
childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '', 0, null) as AST);
104104
childCtx.appendElement(i18nOf(elementB), 0, true);
105105

106106
expect(childCtx.bindings.size).toBe(2);

0 commit comments

Comments
 (0)