From 9f9024b7a17a504f4b57f1ec2f536ca2eb728c5e Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 22 Jan 2019 23:21:53 +0100 Subject: [PATCH] fix(ivy): handle namespaces in attributes (#28242) Adds handling for namespaced attributes when generating the template and in the `elementAttribute` instruction. PR Close #28242 --- .../compiler/src/render3/view/template.ts | 46 ++++++++-- packages/core/src/render3/instructions.ts | 16 +++- packages/core/test/linker/integration_spec.ts | 85 +++++++++---------- packages/core/test/render3/render_util.ts | 2 +- .../platform-server/test/integration_spec.ts | 17 ++-- 5 files changed, 101 insertions(+), 65 deletions(-) diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 99b7bbb517..35a61ce1a6 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -10,7 +10,7 @@ import {flatten, sanitizeIdentifier} from '../../compile_metadata'; import {BindingForm, BuiltinFunctionCall, LocalResolver, convertActionBinding, convertPropertyBinding} from '../../compiler_util/expression_converter'; import {ConstantPool} from '../../constant_pool'; import * as core from '../../core'; -import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEvent, ParsedEventType, PropertyRead} from '../../expression_parser/ast'; +import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast'; import {Lexer} from '../../expression_parser/lexer'; import {Parser} from '../../expression_parser/parser'; import * as i18n from '../../i18n/i18n_ast'; @@ -567,7 +567,9 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } }); - outputAttrs.forEach(attr => attributes.push(o.literal(attr.name), o.literal(attr.value))); + outputAttrs.forEach(attr => { + attributes.push(...getAttributeNameLiterals(attr.name), o.literal(attr.value)); + }); // this will build the instructions so that they fall into the following syntax // add attributes for directive matching purposes @@ -719,13 +721,25 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver const value = input.value.visit(this._valueConverter); if (value !== undefined) { const params: any[] = []; + const [attrNamespace, attrName] = splitNsName(input.name); const isAttributeBinding = input.type === BindingType.Attribute; const sanitizationRef = resolveSanitizationFn(input.securityContext, isAttributeBinding); if (sanitizationRef) params.push(sanitizationRef); + if (attrNamespace) { + const namespaceLiteral = o.literal(attrNamespace); + + if (sanitizationRef) { + params.push(namespaceLiteral); + } else { + // If there wasn't a sanitization ref, we need to add + // an extra param so that we can pass in the namespace. + params.push(o.literal(null), namespaceLiteral); + } + } this.allocateBindingSlots(value); this.updateInstruction(input.sourceSpan, instruction, () => { return [ - o.literal(elementIndex), o.literal(input.name), + o.literal(elementIndex), o.literal(attrName), this.convertPropertyBinding(implicit, value), ...params ]; }); @@ -1038,10 +1052,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver function addAttrExpr(key: string | number, value?: o.Expression): void { if (typeof key === 'string') { if (!alreadySeen.has(key)) { - attrExprs.push(o.literal(key)); - if (value !== undefined) { - attrExprs.push(value); - } + attrExprs.push(...getAttributeNameLiterals(key)); + value !== undefined && attrExprs.push(value); alreadySeen.add(key); } } else { @@ -1264,6 +1276,26 @@ function getLiteralFactory( return o.importExpr(identifier).callFn(args); } +/** + * Gets an array of literals that can be added to an expression + * to represent the name and namespace of an attribute. E.g. + * `:xlink:href` turns into `[AttributeMarker.NamespaceURI, 'xlink', 'href']`. + * + * @param name Name of the attribute, including the namespace. + */ +function getAttributeNameLiterals(name: string): o.LiteralExpr[] { + const [attributeNamespace, attributeName] = splitNsName(name); + const nameLiteral = o.literal(attributeName); + + if (attributeNamespace) { + return [ + o.literal(core.AttributeMarker.NamespaceURI), o.literal(attributeNamespace), nameLiteral + ]; + } + + return [nameLiteral]; +} + /** * Function which is executed whenever a variable is referenced for the first time in a given * scope. diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index fa5706f2ab..3dc4eb0532 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -1050,9 +1050,11 @@ export function elementEnd(): void { * @param value value The attribute is removed when value is `null` or `undefined`. * Otherwise the attribute value is set to the stringified value. * @param sanitizer An optional function used to sanitize the value. + * @param namespace Optional namespace to use when setting the attribute. */ export function elementAttribute( - index: number, name: string, value: any, sanitizer?: SanitizerFn | null): void { + index: number, name: string, value: any, sanitizer?: SanitizerFn | null, + namespace?: string): void { if (value !== NO_CHANGE) { ngDevMode && validateAttribute(name); const lView = getLView(); @@ -1060,15 +1062,21 @@ export function elementAttribute( const element = getNativeByIndex(index, lView); if (value == null) { ngDevMode && ngDevMode.rendererRemoveAttribute++; - isProceduralRenderer(renderer) ? renderer.removeAttribute(element, name) : + isProceduralRenderer(renderer) ? renderer.removeAttribute(element, name, namespace) : element.removeAttribute(name); } else { ngDevMode && ngDevMode.rendererSetAttribute++; const tNode = getTNode(index, lView); const strValue = sanitizer == null ? renderStringify(value) : sanitizer(value, tNode.tagName || '', name); - isProceduralRenderer(renderer) ? renderer.setAttribute(element, name, strValue) : - element.setAttribute(name, strValue); + + + if (isProceduralRenderer(renderer)) { + renderer.setAttribute(element, name, strValue, namespace); + } else { + namespace ? element.setAttributeNS(namespace, name, strValue) : + element.setAttribute(name, strValue); + } } } } diff --git a/packages/core/test/linker/integration_spec.ts b/packages/core/test/linker/integration_spec.ts index fbe76fb9ab..90e11bdd4b 100644 --- a/packages/core/test/linker/integration_spec.ts +++ b/packages/core/test/linker/integration_spec.ts @@ -1832,25 +1832,24 @@ function declareTests(config?: {useJit: boolean}) { if (getDOM().supportsDOMEvents()) { describe('svg', () => { - fixmeIvy('FW-672: SVG attribute xlink:href is output as :xlink:href (extra ":")') - .it('should support svg elements', () => { - TestBed.configureTestingModule({declarations: [MyComp]}); - const template = ''; - TestBed.overrideComponent(MyComp, {set: {template}}); - const fixture = TestBed.createComponent(MyComp); + it('should support svg elements', () => { + TestBed.configureTestingModule({declarations: [MyComp]}); + const template = ''; + TestBed.overrideComponent(MyComp, {set: {template}}); + const fixture = TestBed.createComponent(MyComp); - const el = fixture.nativeElement; - const svg = getDOM().childNodes(el)[0]; - const use = getDOM().childNodes(svg)[0]; - expect(getDOM().getProperty(svg, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); - expect(getDOM().getProperty(use, 'namespaceURI')) - .toEqual('http://www.w3.org/2000/svg'); + const el = fixture.nativeElement; + const svg = getDOM().childNodes(el)[0]; + const use = getDOM().childNodes(svg)[0]; + expect(getDOM().getProperty(svg, 'namespaceURI')) + .toEqual('http://www.w3.org/2000/svg'); + expect(getDOM().getProperty(use, 'namespaceURI')) + .toEqual('http://www.w3.org/2000/svg'); - const firstAttribute = getDOM().getProperty(use, 'attributes')[0]; - expect(firstAttribute.name).toEqual('xlink:href'); - expect(firstAttribute.namespaceURI).toEqual('http://www.w3.org/1999/xlink'); - }); + const firstAttribute = getDOM().getProperty(use, 'attributes')[0]; + expect(firstAttribute.name).toEqual('xlink:href'); + expect(firstAttribute.namespaceURI).toEqual('http://www.w3.org/1999/xlink'); + }); it('should support foreignObjects with document fragments', () => { TestBed.configureTestingModule({declarations: [MyComp]}); @@ -1874,40 +1873,38 @@ function declareTests(config?: {useJit: boolean}) { describe('attributes', () => { - fixmeIvy('FW-672: SVG attribute xlink:href is output as :xlink:href (extra ":")') - .it('should support attributes with namespace', () => { - TestBed.configureTestingModule({declarations: [MyComp, SomeCmp]}); - const template = ''; - TestBed.overrideComponent(SomeCmp, {set: {template}}); - const fixture = TestBed.createComponent(SomeCmp); + it('should support attributes with namespace', () => { + TestBed.configureTestingModule({declarations: [MyComp, SomeCmp]}); + const template = ''; + TestBed.overrideComponent(SomeCmp, {set: {template}}); + const fixture = TestBed.createComponent(SomeCmp); - const useEl = getDOM().firstChild(fixture.nativeElement); - expect(getDOM().getAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) - .toEqual('#id'); - }); + const useEl = getDOM().firstChild(fixture.nativeElement); + expect(getDOM().getAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) + .toEqual('#id'); + }); - fixmeIvy('FW-672: SVG attribute xlink:href is output as :xlink:href (extra ":")') - .it('should support binding to attributes with namespace', () => { - TestBed.configureTestingModule({declarations: [MyComp, SomeCmp]}); - const template = ''; - TestBed.overrideComponent(SomeCmp, {set: {template}}); - const fixture = TestBed.createComponent(SomeCmp); + it('should support binding to attributes with namespace', () => { + TestBed.configureTestingModule({declarations: [MyComp, SomeCmp]}); + const template = ''; + TestBed.overrideComponent(SomeCmp, {set: {template}}); + const fixture = TestBed.createComponent(SomeCmp); - const cmp = fixture.componentInstance; - const useEl = getDOM().firstChild(fixture.nativeElement); + const cmp = fixture.componentInstance; + const useEl = getDOM().firstChild(fixture.nativeElement); - cmp.value = '#id'; - fixture.detectChanges(); + cmp.value = '#id'; + fixture.detectChanges(); - expect(getDOM().getAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) - .toEqual('#id'); + expect(getDOM().getAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) + .toEqual('#id'); - cmp.value = null; - fixture.detectChanges(); + cmp.value = null; + fixture.detectChanges(); - expect(getDOM().hasAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) - .toEqual(false); - }); + expect(getDOM().hasAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) + .toEqual(false); + }); }); } }); diff --git a/packages/core/test/render3/render_util.ts b/packages/core/test/render3/render_util.ts index 439c10185b..6480ef0c1f 100644 --- a/packages/core/test/render3/render_util.ts +++ b/packages/core/test/render3/render_util.ts @@ -394,7 +394,7 @@ class MockRenderer implements ProceduralRenderer3 { destroy(): void {} createComment(value: string): RComment { return document.createComment(value); } createElement(name: string, namespace?: string|null): RElement { - return document.createElement(name); + return namespace ? document.createElementNS(namespace, name) : document.createElement(name); } createText(value: string): RText { return document.createTextNode(value); } appendChild(parent: RElement, newChild: RNode): void { parent.appendChild(newChild); } diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 8a4a25d2c2..a9d0c6978c 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -579,15 +579,14 @@ class HiddenModule { }); }))); - fixmeIvy('FW-672: SVG xlink:href is sanitized to :xlink:href (extra ":")') - .it('works with SVG elements', async(() => { - renderModule(SVGServerModule, {document: doc}).then(output => { - expect(output).toBe( - '' + - ''); - called = true; - }); - })); + it('works with SVG elements', async(() => { + renderModule(SVGServerModule, {document: doc}).then(output => { + expect(output).toBe( + '' + + ''); + called = true; + }); + })); it('works with animation', async(() => { renderModule(AnimationServerModule, {document: doc}).then(output => {