From e6909bda898a3a7913383b65b8b6e24fad3f4285 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 2 Dec 2019 20:18:31 +0100 Subject: [PATCH] fix(ivy): incorrectly validating html foreign objects inside svg (#34178) Fixes ngtsc incorrectly logging an unknown element diagnostic for HTML elements that are inside an SVG `foreignObject` with the `xhtml` namespace. Fixes #34171. PR Close #34178 --- .../src/ngtsc/typecheck/src/dom.ts | 16 ++++-- .../test/ngtsc/template_typecheck_spec.ts | 49 +++++++++++++++++++ .../core/test/acceptance/ng_module_spec.ts | 25 ++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts index 73d67ad68b..c50a727bc7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts @@ -14,6 +14,7 @@ import {ErrorCode} from '../../diagnostics'; import {TcbSourceResolver, makeTemplateDiagnostic} from './diagnostics'; const REGISTRY = new DomElementSchemaRegistry(); +const REMOVE_XHTML_REGEX = /^:xhtml:/; /** * Checks every non-Angular element/property processed in a template and potentially produces @@ -69,15 +70,20 @@ export class RegistryDomSchemaChecker implements DomSchemaChecker { constructor(private resolver: TcbSourceResolver) {} checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void { - if (!REGISTRY.hasElement(element.name, schemas)) { + // HTML elements inside an SVG `foreignObject` are declared in the `xhtml` namespace. + // We need to strip it before handing it over to the registry because all HTML tag names + // in the registry are without a namespace. + const name = element.name.replace(REMOVE_XHTML_REGEX, ''); + + if (!REGISTRY.hasElement(name, schemas)) { const mapping = this.resolver.getSourceMapping(id); - let errorMsg = `'${element.name}' is not a known element:\n`; + let errorMsg = `'${name}' is not a known element:\n`; errorMsg += - `1. If '${element.name}' is an Angular component, then verify that it is part of this module.\n`; - if (element.name.indexOf('-') > -1) { + `1. If '${name}' is an Angular component, then verify that it is part of this module.\n`; + if (name.indexOf('-') > -1) { errorMsg += - `2. If '${element.name}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`; + `2. If '${name}' is a Web Component then add 'CUSTOM_ELEMENTS_SCHEMA' to the '@NgModule.schemas' of this component to suppress this message.`; } else { errorMsg += `2. To allow any element add 'NO_ERRORS_SCHEMA' to the '@NgModule.schemas' of this component.`; diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index e6e6dcccd7..16c1f76ceb 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -1362,6 +1362,55 @@ export declare class AnimationEvent { const diags = env.driveDiagnostics(); expect(diags).toEqual([]); }); + + it('should allow HTML elements inside 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(0); + }); + + it('should check for unknown elements 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/core/test/acceptance/ng_module_spec.ts b/packages/core/test/acceptance/ng_module_spec.ts index 78f1457ea5..8e2877dbd0 100644 --- a/packages/core/test/acceptance/ng_module_spec.ts +++ b/packages/core/test/acceptance/ng_module_spec.ts @@ -311,5 +311,30 @@ describe('NgModule', () => { }).not.toThrow(); }); + it('should not throw for HTML elements inside an SVG foreignObject', () => { + @Component({ + template: ` + + + Hello + + + `, + }) + class MyComp { + } + + @NgModule({declarations: [MyComp]}) + class MyModule { + } + + TestBed.configureTestingModule({imports: [MyModule]}); + + expect(() => { + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + }).not.toThrow(); + }); + }); });