From 0dda97ea669fbf8b49fc5f2ecabfac43f7d666b5 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 26 Aug 2020 16:21:49 +0200 Subject: [PATCH] fix(compiler): incorrectly inferring namespace for HTML nodes inside SVG (#38477) The HTML parser gets an element's namespace either from the tag name (e.g. ``) or from its parent element ``) which breaks down when an element is inside of an SVG `foreignElement`, because foreign elements allow nodes from a different namespace to be inserted into an SVG. These changes add another flag to the tag definitions which tells child nodes whether to try to inherit their namespaces from their parents. It also adds a definition for `foreignObject` with the new flag, allowing elements placed inside it to infer their namespaces instead. Fixes #37218. PR Close #38477 --- .../test/ngtsc/template_typecheck_spec.ts | 49 +++++++++++++++++++ packages/compiler/src/ml_parser/html_tags.ts | 24 +++++++-- packages/compiler/src/ml_parser/parser.ts | 8 ++- packages/compiler/src/ml_parser/tags.ts | 1 + packages/compiler/src/ml_parser/xml_tags.ts | 1 + 5 files changed, 78 insertions(+), 5 deletions(-) diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 63c6a168eb..26212e3bc1 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -2017,6 +2017,28 @@ export declare class AnimationEvent { expect(diags.length).toBe(0); }); + it('should allow HTML elements without explicit namespace inside SVG foreignObject', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + template: \` + + +
Hello
+ + + \`, + }) + export class FooCmp {} + @NgModule({ + declarations: [FooCmp], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(0); + }); + it('should check for unknown elements inside an SVG foreignObject', () => { env.write('test.ts', ` import {Component, NgModule} from '@angular/core'; @@ -2042,6 +2064,33 @@ export declare class AnimationEvent { 1. If 'foo' is an Angular component, then verify that it is part of this module. 2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`); }); + + it('should check for unknown elements without explicit namespace inside an SVG foreignObject', + () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + selector: 'blah', + template: \` + + + Hello + + + \`, + }) + export class FooCmp {} + @NgModule({ + declarations: [FooCmp], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`'foo' is not a known element: +1. If 'foo' is an Angular component, then verify that it is part of this module. +2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`); + }); }); // Test both sync and async compilations, see https://github.com/angular/angular/issues/32538 diff --git a/packages/compiler/src/ml_parser/html_tags.ts b/packages/compiler/src/ml_parser/html_tags.ts index 969fb0f95f..e83d7aa670 100644 --- a/packages/compiler/src/ml_parser/html_tags.ts +++ b/packages/compiler/src/ml_parser/html_tags.ts @@ -17,6 +17,7 @@ export class HtmlTagDefinition implements TagDefinition { isVoid: boolean; ignoreFirstLf: boolean; canSelfClose: boolean = false; + preventNamespaceInheritance: boolean; constructor({ closedByChildren, @@ -24,14 +25,16 @@ export class HtmlTagDefinition implements TagDefinition { contentType = TagContentType.PARSABLE_DATA, closedByParent = false, isVoid = false, - ignoreFirstLf = false + ignoreFirstLf = false, + preventNamespaceInheritance = false }: { closedByChildren?: string[], closedByParent?: boolean, implicitNamespacePrefix?: string, contentType?: TagContentType, isVoid?: boolean, - ignoreFirstLf?: boolean + ignoreFirstLf?: boolean, + preventNamespaceInheritance?: boolean } = {}) { if (closedByChildren && closedByChildren.length > 0) { closedByChildren.forEach(tagName => this.closedByChildren[tagName] = true); @@ -41,6 +44,7 @@ export class HtmlTagDefinition implements TagDefinition { this.implicitNamespacePrefix = implicitNamespacePrefix || null; this.contentType = contentType; this.ignoreFirstLf = ignoreFirstLf; + this.preventNamespaceInheritance = preventNamespaceInheritance; } isClosedByChild(name: string): boolean { @@ -88,6 +92,17 @@ export function getHtmlTagDefinition(tagName: string): HtmlTagDefinition { 'th': new HtmlTagDefinition({closedByChildren: ['td', 'th'], closedByParent: true}), 'col': new HtmlTagDefinition({isVoid: true}), 'svg': new HtmlTagDefinition({implicitNamespacePrefix: 'svg'}), + 'foreignObject': new HtmlTagDefinition({ + // Usually the implicit namespace here would be redundant since it will be inherited from + // the parent `svg`, but we have to do it for `foreignObject`, because the way the parser + // works is that the parent node of an end tag is its own start tag which means that + // the `preventNamespaceInheritance` on `foreignObject` would have it default to the + // implicit namespace which is `html`, unless specified otherwise. + implicitNamespacePrefix: 'svg', + // We want to prevent children of foreignObject from inheriting its namespace, because + // the point of the element is to allow nodes from other namespaces to be inserted. + preventNamespaceInheritance: true, + }), 'math': new HtmlTagDefinition({implicitNamespacePrefix: 'math'}), 'li': new HtmlTagDefinition({closedByChildren: ['li'], closedByParent: true}), 'dt': new HtmlTagDefinition({closedByChildren: ['dt', 'dd']}), @@ -111,5 +126,8 @@ export function getHtmlTagDefinition(tagName: string): HtmlTagDefinition { {contentType: TagContentType.ESCAPABLE_RAW_TEXT, ignoreFirstLf: true}), }; } - return TAG_DEFINITIONS[tagName.toLowerCase()] || _DEFAULT_TAG_DEFINITION; + // We have to make both a case-sensitive and a case-insesitive lookup, because + // HTML tag names are case insensitive, whereas some SVG tags are case sensitive. + return TAG_DEFINITIONS[tagName] ?? TAG_DEFINITIONS[tagName.toLowerCase()] ?? + _DEFAULT_TAG_DEFINITION; } diff --git a/packages/compiler/src/ml_parser/parser.ts b/packages/compiler/src/ml_parser/parser.ts index b31db0c25c..3d3131989c 100644 --- a/packages/compiler/src/ml_parser/parser.ts +++ b/packages/compiler/src/ml_parser/parser.ts @@ -10,7 +10,7 @@ import {ParseError, ParseSourceSpan} from '../parse_util'; import * as html from './ast'; import * as lex from './lexer'; -import {getNsPrefix, isNgContainer, mergeNsAndName, TagDefinition} from './tags'; +import {getNsPrefix, mergeNsAndName, splitNsName, TagDefinition} from './tags'; export class TreeError extends ParseError { static create(elementName: string|null, span: ParseSourceSpan, msg: string): TreeError { @@ -353,7 +353,11 @@ class _TreeBuilder { if (prefix === '') { prefix = this.getTagDefinition(localName).implicitNamespacePrefix || ''; if (prefix === '' && parentElement != null) { - prefix = getNsPrefix(parentElement.name); + const parentTagName = splitNsName(parentElement.name)[1]; + const parentTagDefinition = this.getTagDefinition(parentTagName); + if (!parentTagDefinition.preventNamespaceInheritance) { + prefix = getNsPrefix(parentElement.name); + } } } diff --git a/packages/compiler/src/ml_parser/tags.ts b/packages/compiler/src/ml_parser/tags.ts index 279ab0aacb..32b3aab2af 100644 --- a/packages/compiler/src/ml_parser/tags.ts +++ b/packages/compiler/src/ml_parser/tags.ts @@ -19,6 +19,7 @@ export interface TagDefinition { isVoid: boolean; ignoreFirstLf: boolean; canSelfClose: boolean; + preventNamespaceInheritance: boolean; isClosedByChild(name: string): boolean; } diff --git a/packages/compiler/src/ml_parser/xml_tags.ts b/packages/compiler/src/ml_parser/xml_tags.ts index 81af50d689..e4a76ed245 100644 --- a/packages/compiler/src/ml_parser/xml_tags.ts +++ b/packages/compiler/src/ml_parser/xml_tags.ts @@ -20,6 +20,7 @@ export class XmlTagDefinition implements TagDefinition { isVoid: boolean = false; ignoreFirstLf: boolean = false; canSelfClose: boolean = true; + preventNamespaceInheritance: boolean = false; requireExtraParent(currentParent: string): boolean { return false;