Skip to content

Commit 2b7553d

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 9fa6f5a commit 2b7553d

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, RecursiveAstVisitor, SafeCall, SafeKeyedRead, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast';
1314
import {EOF, Lexer, Token, TokenType} from './lexer';
@@ -147,9 +148,10 @@ export class Parser {
147148

148149
parseInterpolation(
149150
input: string, location: string, absoluteOffset: number,
151+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null,
150152
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource|null {
151153
const {strings, expressions, offsets} =
152-
this.splitInterpolation(input, location, interpolationConfig);
154+
this.splitInterpolation(input, location, interpolatedTokens, interpolationConfig);
153155
if (expressions.length === 0) return null;
154156

155157
const expressionNodes: AST[] = [];
@@ -203,10 +205,13 @@ export class Parser {
203205
*/
204206
splitInterpolation(
205207
input: string, location: string,
208+
interpolatedTokens: InterpolatedAttributeToken[]|InterpolatedTextToken[]|null,
206209
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): SplitInterpolation {
207210
const strings: InterpolationPiece[] = [];
208211
const expressions: InterpolationPiece[] = [];
209212
const offsets: number[] = [];
213+
const inputToTemplateIndexMap =
214+
interpolatedTokens ? getIndexMapForOriginalTemplate(interpolatedTokens) : null;
210215
let i = 0;
211216
let atInterpolation = false;
212217
let extendLastString = false;
@@ -244,7 +249,9 @@ export class Parser {
244249
`at column ${i} in`, location);
245250
}
246251
expressions.push({text, start: fullStart, end: fullEnd});
247-
offsets.push(exprStart);
252+
const startInOriginalTemplate = inputToTemplateIndexMap?.get(fullStart) ?? fullStart;
253+
const offset = startInOriginalTemplate + interpStart.length;
254+
offsets.push(offset);
248255

249256
i = fullEnd;
250257
atInterpolation = false;
@@ -1314,3 +1321,38 @@ class SimpleExpressionChecker extends RecursiveAstVisitor {
13141321
this.errors.push('pipes');
13151322
}
13161323
}
1324+
/**
1325+
* Computes the real offset in the original template for indexes in an interpolation.
1326+
*
1327+
* Because templates can have encoded HTML entities and the input passed to the parser at this stage
1328+
* of the compiler is the _decoded_ value, we need to compute the real offset using the original
1329+
* encoded values in the interpolated tokens. Note that this is only a special case handling for
1330+
* `MlParserTokenType.ENCODED_ENTITY` token types. All other interpolated tokens are expected to
1331+
* have parts which exactly match the input string for parsing the interpolation.
1332+
*
1333+
* @param interpolatedTokens The tokens for the interpolated value.
1334+
*
1335+
* @returns A map of index locations in the decoded template to indexes in the original template
1336+
*/
1337+
function getIndexMapForOriginalTemplate(interpolatedTokens: InterpolatedAttributeToken[]|
1338+
InterpolatedTextToken[]): Map<number, number> {
1339+
let offsetMap = new Map<number, number>();
1340+
let consumedInOriginalTemplate = 0;
1341+
let consumedInInput = 0;
1342+
let tokenIndex = 0;
1343+
while (tokenIndex < interpolatedTokens.length) {
1344+
const currentToken = interpolatedTokens[tokenIndex];
1345+
if (currentToken.type === MlParserTokenType.ENCODED_ENTITY) {
1346+
const [decoded, encoded] = currentToken.parts;
1347+
consumedInOriginalTemplate += encoded.length;
1348+
consumedInInput += decoded.length;
1349+
} else {
1350+
const lengthOfParts = currentToken.parts.reduce((sum, current) => sum + current.length, 0);
1351+
consumedInInput += lengthOfParts;
1352+
consumedInOriginalTemplate += lengthOfParts;
1353+
}
1354+
offsetMap.set(consumedInInput, consumedInOriginalTemplate);
1355+
tokenIndex++;
1356+
}
1357+
return offsetMap;
1358+
}

‎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
@@ -995,7 +995,8 @@ describe('parser', () => {
995995

996996
it('should support custom interpolation', () => {
997997
const parser = new Parser(new Lexer());
998-
const ast = parser.parseInterpolation('{% a %}', '', 0, {start: '{%', end: '%}'})!.ast as any;
998+
const ast =
999+
parser.parseInterpolation('{% a %}', '', 0, null, {start: '{%', end: '%}'})!.ast as any;
9991000
expect(ast.strings).toEqual(['', '']);
10001001
expect(ast.expressions.length).toEqual(1);
10011002
expect(ast.expressions[0].name).toEqual('a');
@@ -1194,11 +1195,11 @@ function _parseTemplateBindings(attribute: string, templateUrl: string) {
11941195

11951196
function parseInterpolation(text: string, location: any = null, offset: number = 0): ASTWithSource|
11961197
null {
1197-
return createParser().parseInterpolation(text, location, offset);
1198+
return createParser().parseInterpolation(text, location, offset, null);
11981199
}
11991200

12001201
function splitInterpolation(text: string, location: any = null): SplitInterpolation|null {
1201-
return createParser().splitInterpolation(text, location);
1202+
return createParser().splitInterpolation(text, location, null);
12021203
}
12031204

12041205
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)