From 66c27ffdfc0e4f0a37a375a453d368c8ef13e326 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 25 Dec 2020 10:56:51 +0200 Subject: [PATCH] fix(compiler): incorrectly inferring content type of SVG-specific title tag (#40259) The parser has a list of tag definitions that it uses when parsing the template. Each tag has a `contentType` which tells the parser what kind of content the tag should contain. The problem is that the browser has two separate `title` tags (`HTMLTitleElement` and `SVGTitleElement`) and each of them has to have a different `contentType`, otherwise the parser will throw an error further down the pipeline. These changes update the tag definitions so that each tag name can have multiple content types associated with it and the correct one can be returned based on the element's prefix. Fixes #31503. PR Close #40259 --- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 23 +++++++++++++++++ packages/compiler/src/ml_parser/html_tags.ts | 19 +++++++++++--- packages/compiler/src/ml_parser/lexer.ts | 4 +-- packages/compiler/src/ml_parser/tags.ts | 2 +- packages/compiler/src/ml_parser/xml_tags.ts | 5 +++- .../compiler/test/ml_parser/lexer_spec.ts | 25 +++++++++++++++++++ packages/language-service/src/completions.ts | 2 +- 7 files changed, 72 insertions(+), 8 deletions(-) diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index cb2a939196..ed827dcb3d 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -4312,6 +4312,29 @@ runInEachFileSystem(os => { expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.Other')); }); + it('should not throw when using an SVG-specific `title` tag', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + template: \` + + + I'm a title tag + + + \`, + }) + export class SvgCmp {} + @NgModule({ + declarations: [SvgCmp], + }) + export class SvgModule {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + describe('namespace support', () => { it('should generate correct imports for type references to namespaced symbols using a namespace import', () => { diff --git a/packages/compiler/src/ml_parser/html_tags.ts b/packages/compiler/src/ml_parser/html_tags.ts index 9fbffd45e2..ffde2b7679 100644 --- a/packages/compiler/src/ml_parser/html_tags.ts +++ b/packages/compiler/src/ml_parser/html_tags.ts @@ -10,10 +10,11 @@ import {TagContentType, TagDefinition} from './tags'; export class HtmlTagDefinition implements TagDefinition { private closedByChildren: {[key: string]: boolean} = {}; + private contentType: TagContentType| + {default: TagContentType, [namespace: string]: TagContentType}; closedByParent: boolean = false; implicitNamespacePrefix: string|null; - contentType: TagContentType; isVoid: boolean; ignoreFirstLf: boolean; canSelfClose: boolean = false; @@ -31,7 +32,7 @@ export class HtmlTagDefinition implements TagDefinition { closedByChildren?: string[], closedByParent?: boolean, implicitNamespacePrefix?: string, - contentType?: TagContentType, + contentType?: TagContentType|{default: TagContentType, [namespace: string]: TagContentType}, isVoid?: boolean, ignoreFirstLf?: boolean, preventNamespaceInheritance?: boolean @@ -50,6 +51,14 @@ export class HtmlTagDefinition implements TagDefinition { isClosedByChild(name: string): boolean { return this.isVoid || name.toLowerCase() in this.closedByChildren; } + + getContentType(prefix?: string): TagContentType { + if (typeof this.contentType === 'object') { + const overrideType = prefix == null ? undefined : this.contentType[prefix]; + return overrideType ?? this.contentType.default; + } + return this.contentType; + } } let _DEFAULT_TAG_DEFINITION!: HtmlTagDefinition; @@ -121,7 +130,11 @@ export function getHtmlTagDefinition(tagName: string): HtmlTagDefinition { 'listing': new HtmlTagDefinition({ignoreFirstLf: true}), 'style': new HtmlTagDefinition({contentType: TagContentType.RAW_TEXT}), 'script': new HtmlTagDefinition({contentType: TagContentType.RAW_TEXT}), - 'title': new HtmlTagDefinition({contentType: TagContentType.ESCAPABLE_RAW_TEXT}), + 'title': new HtmlTagDefinition({ + // The browser supports two separate `title` tags which have to use + // a different content type: `HTMLTitleElement` and `SVGTitleElement` + contentType: {default: TagContentType.ESCAPABLE_RAW_TEXT, svg: TagContentType.PARSABLE_DATA} + }), 'textarea': new HtmlTagDefinition( {contentType: TagContentType.ESCAPABLE_RAW_TEXT, ignoreFirstLf: true}), }; diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts index 1b4ce13929..09d976a7b5 100644 --- a/packages/compiler/src/ml_parser/lexer.ts +++ b/packages/compiler/src/ml_parser/lexer.ts @@ -550,7 +550,7 @@ class _Tokenizer { throw e; } - const contentTokenType = this._getTagDefinition(tagName).contentType; + const contentTokenType = this._getTagDefinition(tagName).getContentType(prefix); if (contentTokenType === TagContentType.RAW_TEXT) { this._consumeRawTextWithTagClose(prefix, tagName, false); @@ -560,7 +560,7 @@ class _Tokenizer { } private _consumeRawTextWithTagClose(prefix: string, tagName: string, decodeEntities: boolean) { - const textToken = this._consumeRawText(decodeEntities, () => { + this._consumeRawText(decodeEntities, () => { if (!this._attemptCharCode(chars.$LT)) return false; if (!this._attemptCharCode(chars.$SLASH)) return false; this._attemptCharCodeUntilFn(isNotWhitespace); diff --git a/packages/compiler/src/ml_parser/tags.ts b/packages/compiler/src/ml_parser/tags.ts index 4d061e9f68..8c2f4a65fc 100644 --- a/packages/compiler/src/ml_parser/tags.ts +++ b/packages/compiler/src/ml_parser/tags.ts @@ -15,13 +15,13 @@ export enum TagContentType { export interface TagDefinition { closedByParent: boolean; implicitNamespacePrefix: string|null; - contentType: TagContentType; isVoid: boolean; ignoreFirstLf: boolean; canSelfClose: boolean; preventNamespaceInheritance: boolean; isClosedByChild(name: string): boolean; + getContentType(prefix?: string): TagContentType; } export function splitNsName(elementName: string): [string|null, string] { diff --git a/packages/compiler/src/ml_parser/xml_tags.ts b/packages/compiler/src/ml_parser/xml_tags.ts index e4a76ed245..d36bf684d6 100644 --- a/packages/compiler/src/ml_parser/xml_tags.ts +++ b/packages/compiler/src/ml_parser/xml_tags.ts @@ -16,7 +16,6 @@ export class XmlTagDefinition implements TagDefinition { parentToAdd!: string; // TODO(issue/24571): remove '!'. implicitNamespacePrefix!: string; - contentType: TagContentType = TagContentType.PARSABLE_DATA; isVoid: boolean = false; ignoreFirstLf: boolean = false; canSelfClose: boolean = true; @@ -29,6 +28,10 @@ export class XmlTagDefinition implements TagDefinition { isClosedByChild(name: string): boolean { return false; } + + getContentType(): TagContentType { + return TagContentType.PARSABLE_DATA; + } } const _TAG_DEFINITION = new XmlTagDefinition(); diff --git a/packages/compiler/test/ml_parser/lexer_spec.ts b/packages/compiler/test/ml_parser/lexer_spec.ts index 886e1e30a4..179b49f01d 100644 --- a/packages/compiler/test/ml_parser/lexer_spec.ts +++ b/packages/compiler/test/ml_parser/lexer_spec.ts @@ -789,6 +789,31 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u }); }); + describe('parsable data', () => { + it('should parse an SVG tag', () => { + expect(tokenizeAndHumanizeParts(`<svg:title>test</svg:title>`)).toEqual([ + [lex.TokenType.TAG_OPEN_START, 'svg', 'title'], + [lex.TokenType.TAG_OPEN_END], + [lex.TokenType.TEXT, 'test'], + [lex.TokenType.TAG_CLOSE, 'svg', 'title'], + [lex.TokenType.EOF], + ]); + }); + + it('should parse an SVG <title> tag with children', () => { + expect(tokenizeAndHumanizeParts(`<svg:title><f>test</f></svg:title>`)).toEqual([ + [lex.TokenType.TAG_OPEN_START, 'svg', 'title'], + [lex.TokenType.TAG_OPEN_END], + [lex.TokenType.TAG_OPEN_START, '', 'f'], + [lex.TokenType.TAG_OPEN_END], + [lex.TokenType.TEXT, 'test'], + [lex.TokenType.TAG_CLOSE, '', 'f'], + [lex.TokenType.TAG_CLOSE, 'svg', 'title'], + [lex.TokenType.EOF], + ]); + }); + }); + describe('expansion forms', () => { it('should parse an expansion form', () => { expect( diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 6025848b9d..97077bb6b5 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -196,7 +196,7 @@ class HtmlVisitor implements Visitor { // any test cases for it. const element = this.htmlPath.first(Element); if (element && - getHtmlTagDefinition(element.name).contentType !== TagContentType.PARSABLE_DATA) { + getHtmlTagDefinition(element.name).getContentType() !== TagContentType.PARSABLE_DATA) { return []; } // This is to account for cases like <h1> <a> text | </h1> where the