From ae0253f34adad0e37d2a5e6596a08aa049ba3072 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 3 Feb 2020 18:02:03 -0800 Subject: [PATCH] fix(ivy): set namespace for host elements of dynamically created components (#35136) Prior to this change, element namespace was not set for host elements of dynamically created components that resulted in incorrect rendering in a browser. This commit adds the logic to pick and set correct namespace for host element when component is created dynamically. PR Close #35136 --- packages/core/src/render3/component_ref.ts | 17 +++- packages/core/src/render3/namespaces.ts | 10 ++ packages/core/src/render3/state.ts | 5 +- .../acceptance/view_container_ref_spec.ts | 95 ++++++++++++++++++- .../platform-browser/src/dom/dom_renderer.ts | 6 +- .../platform-server/src/server_renderer.ts | 6 ++ 6 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/render3/namespaces.ts diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 4045132cf0..6fb9d8a009 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -19,6 +19,7 @@ import {RendererFactory2} from '../render/api'; import {Sanitizer} from '../sanitization/sanitizer'; import {VERSION} from '../version'; import {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from '../view/provider'; + import {assertComponentType} from './assert'; import {LifecycleHooksFeature, createRootComponent, createRootComponentView, createRootContext} from './component'; import {getComponentDef} from './definition'; @@ -28,6 +29,7 @@ import {ComponentDef} from './interfaces/definition'; import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node'; import {RNode, RendererFactory3, domRendererFactory3} from './interfaces/renderer'; import {LView, LViewFlags, TVIEW, TViewType} from './interfaces/view'; +import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces'; import {stringifyCSSSelectorList} from './node_selector_matcher'; import {enterView, leaveView} from './state'; import {defaultScheduler} from './util/misc_utils'; @@ -59,6 +61,11 @@ function toRefArray(map: {[key: string]: string}): {propName: string; templateNa return array; } +function getNamespace(elementName: string): string|null { + const name = elementName.toLowerCase(); + return name === 'svg' ? SVG_NAMESPACE : (name === 'math' ? MATH_ML_NAMESPACE : null); +} + /** * A change detection scheduler token for {@link RootContext}. This token is the default value used * for the default `RootContext` found in the {@link ROOT_CONTEXT} token. @@ -132,14 +139,14 @@ export class ComponentFactory extends viewEngine_ComponentFactory { const sanitizer = rootViewInjector.get(Sanitizer, null); const hostRenderer = rendererFactory.createRenderer(null, this.componentDef); + // Determine a tag name used for creating host elements when this component is created + // dynamically. Default to 'div' if this component did not specify any tag name in its selector. + const elementName = this.componentDef.selectors[0][0] as string || 'div'; const hostRNode = rootSelectorOrNode ? locateHostElement(hostRenderer, rootSelectorOrNode, this.componentDef.encapsulation) : - // Determine a tag name used for creating host elements when this component is created - // dynamically. Default to 'div' if this component did not specify any tag name in its - // selector. elementCreate( - this.componentDef.selectors[0][0] as string || 'div', - rendererFactory.createRenderer(null, this.componentDef), null); + elementName, rendererFactory.createRenderer(null, this.componentDef), + getNamespace(elementName)); const rootFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot : LViewFlags.CheckAlways | LViewFlags.IsRoot; diff --git a/packages/core/src/render3/namespaces.ts b/packages/core/src/render3/namespaces.ts new file mode 100644 index 0000000000..05d38f6599 --- /dev/null +++ b/packages/core/src/render3/namespaces.ts @@ -0,0 +1,10 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +export const SVG_NAMESPACE = 'http://www.w3.org/2000/svg'; +export const MATH_ML_NAMESPACE = 'http://www.w3.org/1998/MathML/'; \ No newline at end of file diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 63816da418..5bde42a6b2 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -11,6 +11,7 @@ import {assertDefined} from '../util/assert'; import {assertLViewOrUndefined} from './assert'; import {TNode} from './interfaces/node'; import {CONTEXT, DECLARATION_VIEW, LView, OpaqueViewState, TVIEW, TView} from './interfaces/view'; +import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces'; /** @@ -511,7 +512,7 @@ export function setSelectedIndex(index: number) { * @codeGenApi */ export function ɵɵnamespaceSVG() { - instructionState.lFrame.currentNamespace = 'http://www.w3.org/2000/svg'; + instructionState.lFrame.currentNamespace = SVG_NAMESPACE; } /** @@ -520,7 +521,7 @@ export function ɵɵnamespaceSVG() { * @codeGenApi */ export function ɵɵnamespaceMathML() { - instructionState.lFrame.currentNamespace = 'http://www.w3.org/1998/MathML/'; + instructionState.lFrame.currentNamespace = MATH_ML_NAMESPACE; } /** diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index 192b126730..f0b3011774 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -8,12 +8,12 @@ import {CommonModule, DOCUMENT} from '@angular/common'; import {computeMsgId} from '@angular/compiler'; -import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, NO_ERRORS_SCHEMA, NgModule, OnInit, Pipe, PipeTransform, QueryList, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; +import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, NO_ERRORS_SCHEMA, NgModule, OnInit, Pipe, PipeTransform, QueryList, Renderer2, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵsetDocument} from '@angular/core'; import {Input} from '@angular/core/src/metadata'; import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed, TestComponentRenderer} from '@angular/core/testing'; import {clearTranslations, loadTranslations} from '@angular/localize'; -import {By, DomSanitizer} from '@angular/platform-browser'; +import {By, DomSanitizer, ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; @@ -160,6 +160,97 @@ describe('ViewContainerRef', () => { fixture.detectChanges(); expect(fixture.debugElement.nativeElement.innerHTML).toContain('Hello'); }); + + describe('element namespaces', () => { + function runTestWithSelectors(svgSelector: string, mathMLSelector: string) { + it('should be set correctly for host elements of dynamically created components', () => { + @Component({ + selector: svgSelector, + template: '', + }) + class SvgComp { + } + + @Component({ + selector: mathMLSelector, + template: '', + }) + class MathMLComp { + } + + @NgModule({ + entryComponents: [SvgComp, MathMLComp], + declarations: [SvgComp, MathMLComp], + // View Engine doesn't have MathML tags listed in `DomElementSchemaRegistry`, thus + // throwing "unknown element" error (':math:matrix' is not a known element). Ignore + // these errors by adding `NO_ERRORS_SCHEMA` to this NgModule. + schemas: [NO_ERRORS_SCHEMA], + }) + class RootModule { + } + + @Component({ + template: ` + + + ` + }) + class TestComp { + @ViewChild('svg', {read: ViewContainerRef}) svgVCRef !: ViewContainerRef; + @ViewChild('mathml', {read: ViewContainerRef}) mathMLVCRef !: ViewContainerRef; + + constructor(public cfr: ComponentFactoryResolver) {} + + createDynamicComponents() { + const svgFactory = this.cfr.resolveComponentFactory(SvgComp); + this.svgVCRef.createComponent(svgFactory); + + const mathMLFactory = this.cfr.resolveComponentFactory(MathMLComp); + this.mathMLVCRef.createComponent(mathMLFactory); + } + } + + function _document(): any { + // Tell Ivy about the global document + ɵsetDocument(document); + return document; + } + + TestBed.configureTestingModule({ + declarations: [TestComp], + imports: [RootModule], + providers: [ + {provide: DOCUMENT, useFactory: _document, deps: []}, + // TODO(FW-811): switch back to default server renderer (i.e. remove the line below) + // once it starts to support Ivy namespace format (URIs) correctly. For now, use + // `DomRenderer` that supports Ivy namespace format. + {provide: RendererFactory2, useClass: DomRendererFactory2} + ], + }); + const fixture = TestBed.createComponent(TestComp); + fixture.detectChanges(); + + fixture.componentInstance.createDynamicComponents(); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('svg').namespaceURI) + .toEqual('http://www.w3.org/2000/svg'); + + // View Engine doesn't set MathML namespace, since it's not present in the list of + // known namespaces here: + // https://github.com/angular/angular/blob/master/packages/platform-browser/src/dom/dom_renderer.ts#L14 + if (ivyEnabled) { + expect(fixture.nativeElement.querySelector('math').namespaceURI) + .toEqual('http://www.w3.org/1998/MathML/'); + } + }); + } + + runTestWithSelectors('svg[some-attr]', 'math[some-attr]'); + + // Also test with selector that has element name in uppercase + runTestWithSelectors('SVG[some-attr]', 'MATH[some-attr]'); + }); }); describe('insert', () => { diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts index 2ac41a4521..80fc39d85a 100644 --- a/packages/platform-browser/src/dom/dom_renderer.ts +++ b/packages/platform-browser/src/dom/dom_renderer.ts @@ -174,7 +174,7 @@ class DefaultDomRenderer2 implements Renderer2 { setAttribute(el: any, name: string, value: string, namespace?: string): void { if (namespace) { name = namespace + ':' + name; - // TODO(benlesh): Ivy may cause issues here because it's passing around + // TODO(FW-811): Ivy may cause issues here because it's passing around // full URIs for namespaces, therefore this lookup will fail. const namespaceUri = NAMESPACE_URIS[namespace]; if (namespaceUri) { @@ -189,13 +189,13 @@ class DefaultDomRenderer2 implements Renderer2 { removeAttribute(el: any, name: string, namespace?: string): void { if (namespace) { - // TODO(benlesh): Ivy may cause issues here because it's passing around + // TODO(FW-811): Ivy may cause issues here because it's passing around // full URIs for namespaces, therefore this lookup will fail. const namespaceUri = NAMESPACE_URIS[namespace]; if (namespaceUri) { el.removeAttributeNS(namespaceUri, name); } else { - // TODO(benlesh): Since ivy is passing around full URIs for namespaces + // TODO(FW-811): Since ivy is passing around full URIs for namespaces // this could result in properties like `http://www.w3.org/2000/svg:cx="123"`, // which is wrong. el.removeAttribute(`${namespace}:${name}`); diff --git a/packages/platform-server/src/server_renderer.ts b/packages/platform-server/src/server_renderer.ts index 83f2f830dc..306632219a 100644 --- a/packages/platform-server/src/server_renderer.ts +++ b/packages/platform-server/src/server_renderer.ts @@ -73,6 +73,8 @@ class DefaultServerRenderer2 implements Renderer2 { createElement(name: string, namespace?: string, debugInfo?: any): any { if (namespace) { const doc = this.document || getDOM().getDefaultDocument(); + // TODO(FW-811): Ivy may cause issues here because it's passing around + // full URIs for namespaces, therefore this lookup will fail. return doc.createElementNS(NAMESPACE_URIS[namespace], name); } @@ -124,6 +126,8 @@ class DefaultServerRenderer2 implements Renderer2 { setAttribute(el: any, name: string, value: string, namespace?: string): void { if (namespace) { + // TODO(FW-811): Ivy may cause issues here because it's passing around + // full URIs for namespaces, therefore this lookup will fail. el.setAttributeNS(NAMESPACE_URIS[namespace], namespace + ':' + name, value); } else { el.setAttribute(name, value); @@ -132,6 +136,8 @@ class DefaultServerRenderer2 implements Renderer2 { removeAttribute(el: any, name: string, namespace?: string): void { if (namespace) { + // TODO(FW-811): Ivy may cause issues here because it's passing around + // full URIs for namespaces, therefore this lookup will fail. el.removeAttributeNS(NAMESPACE_URIS[namespace], name); } else { el.removeAttribute(name);