From 2b9cc8503d48173492c29f5a271b61126104fbdb Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Mon, 14 Jan 2019 15:58:05 -0800 Subject: [PATCH] fix(ivy): Ensure proper namespace is used to create elements in JIT (#28144) PR Close #28144 --- packages/core/test/linker/integration_spec.ts | 35 +++++++++---------- .../platform-browser/src/dom/dom_renderer.ts | 11 +++++- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index ef8343d4db..fbe76fb9ab 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -1852,25 +1852,24 @@ function declareTests(config?: {useJit: boolean}) { expect(firstAttribute.namespaceURI).toEqual('http://www.w3.org/1999/xlink'); }); - fixmeIvy('FW-811: Align HTML namespaces between Ivy and Render2') - .it('should support foreignObjects with document fragments', () => { - TestBed.configureTestingModule({declarations: [MyComp]}); - const template = - '

Test

'; - TestBed.overrideComponent(MyComp, {set: {template}}); - const fixture = TestBed.createComponent(MyComp); + it('should support foreignObjects with document fragments', () => { + TestBed.configureTestingModule({declarations: [MyComp]}); + const template = + '

Test

'; + TestBed.overrideComponent(MyComp, {set: {template}}); + const fixture = TestBed.createComponent(MyComp); - const el = fixture.nativeElement; - const svg = getDOM().childNodes(el)[0]; - const foreignObject = getDOM().childNodes(svg)[0]; - const p = getDOM().childNodes(foreignObject)[0]; - expect(getDOM().getProperty(svg, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); - expect(getDOM().getProperty(foreignObject, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); - expect(getDOM().getProperty(p, 'namespaceURI')) - .toEqual('http://www.w3.org/1999/xhtml'); - }); + const el = fixture.nativeElement; + const svg = getDOM().childNodes(el)[0]; + const foreignObject = getDOM().childNodes(svg)[0]; + const p = getDOM().childNodes(foreignObject)[0]; + expect(getDOM().getProperty(svg, 'namespaceURI')) + .toEqual('http://www.w3.org/2000/svg'); + expect(getDOM().getProperty(foreignObject, 'namespaceURI')) + .toEqual('http://www.w3.org/2000/svg'); + expect(getDOM().getProperty(p, 'namespaceURI')) + .toEqual('http://www.w3.org/1999/xhtml'); + }); }); describe('attributes', () => { diff --git a/packages/platform-browser/src/dom/dom_renderer.ts b/packages/platform-browser/src/dom/dom_renderer.ts index 4c32f03c43..719ddce74e 100644 --- a/packages/platform-browser/src/dom/dom_renderer.ts +++ b/packages/platform-browser/src/dom/dom_renderer.ts @@ -111,7 +111,9 @@ class DefaultDomRenderer2 implements Renderer2 { createElement(name: string, namespace?: string): any { if (namespace) { - return document.createElementNS(NAMESPACE_URIS[namespace], name); + // In cases where Ivy (not ViewEngine) is giving us the actual namespace, the look up by key + // will result in undefined, so we just return the namespace here. + return document.createElementNS(NAMESPACE_URIS[namespace] || namespace, name); } return document.createElement(name); @@ -154,6 +156,8 @@ 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 + // full URIs for namespaces, therefore this lookup will fail. const namespaceUri = NAMESPACE_URIS[namespace]; if (namespaceUri) { el.setAttributeNS(namespaceUri, name, value); @@ -167,10 +171,15 @@ 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 + // 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 + // this could result in properties like `http://www.w3.org/2000/svg:cx="123"`, + // which is wrong. el.removeAttribute(`${namespace}:${name}`); } } else {