From 61cf499b0b6bb759bad6251319c2f5722445f855 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 8 Jan 2016 12:01:29 -0800 Subject: [PATCH] fix(DomRenderer): correctly handle namespaced attributes Closes #6363 --- modules/angular2/src/compiler/html_parser.ts | 6 +-- modules/angular2/src/compiler/html_tags.ts | 4 ++ .../angular2/src/compiler/template_parser.ts | 9 +++- .../src/platform/browser/browser_adapter.dart | 10 ++++ .../src/platform/browser/browser_adapter.ts | 7 +++ .../angular2/src/platform/dom/dom_adapter.ts | 3 ++ .../angular2/src/platform/dom/dom_renderer.ts | 12 +++-- .../server/abstract_html_adapter.dart | 12 +++++ .../src/platform/server/parse5_adapter.ts | 3 ++ .../test/core/linker/integration_spec.ts | 49 ++++++++++++++++++- modules/angular2/test/public_api_spec.ts | 3 ++ tools/public_api_guard/public_api_spec.ts | 3 ++ 12 files changed, 109 insertions(+), 12 deletions(-) diff --git a/modules/angular2/src/compiler/html_parser.ts b/modules/angular2/src/compiler/html_parser.ts index af1fe27c08..301f1a3ad7 100644 --- a/modules/angular2/src/compiler/html_parser.ts +++ b/modules/angular2/src/compiler/html_parser.ts @@ -16,7 +16,7 @@ import {HtmlAst, HtmlAttrAst, HtmlTextAst, HtmlElementAst} from './html_ast'; import {Injectable} from 'angular2/src/core/di'; import {HtmlToken, HtmlTokenType, tokenizeHtml} from './html_lexer'; import {ParseError, ParseLocation, ParseSourceSpan} from './parse_util'; -import {HtmlTagDefinition, getHtmlTagDefinition, getNsPrefix} from './html_tags'; +import {HtmlTagDefinition, getHtmlTagDefinition, getNsPrefix, mergeNsAndName} from './html_tags'; export class HtmlTreeError extends ParseError { static create(elementName: string, location: ParseLocation, msg: string): HtmlTreeError { @@ -238,10 +238,6 @@ class TreeBuilder { } } -function mergeNsAndName(prefix: string, localName: string): string { - return isPresent(prefix) ? `@${prefix}:${localName}` : localName; -} - function getElementFullName(prefix: string, localName: string, parentElement: HtmlElementAst): string { if (isBlank(prefix)) { diff --git a/modules/angular2/src/compiler/html_tags.ts b/modules/angular2/src/compiler/html_tags.ts index d749cf3844..315f0521dd 100644 --- a/modules/angular2/src/compiler/html_tags.ts +++ b/modules/angular2/src/compiler/html_tags.ts @@ -420,3 +420,7 @@ export function splitNsName(elementName: string): string[] { export function getNsPrefix(elementName: string): string { return splitNsName(elementName)[0]; } + +export function mergeNsAndName(prefix: string, localName: string): string { + return isPresent(prefix) ? `@${prefix}:${localName}` : localName; +} diff --git a/modules/angular2/src/compiler/template_parser.ts b/modules/angular2/src/compiler/template_parser.ts index dd3c970d8a..f7796caaff 100644 --- a/modules/angular2/src/compiler/template_parser.ts +++ b/modules/angular2/src/compiler/template_parser.ts @@ -7,11 +7,10 @@ import {Parser, AST, ASTWithSource} from 'angular2/src/core/change_detection/cha import {TemplateBinding} from 'angular2/src/core/change_detection/parser/ast'; import {CompileDirectiveMetadata, CompilePipeMetadata} from './directive_metadata'; import {HtmlParser} from './html_parser'; -import {splitNsName} from './html_tags'; +import {splitNsName, mergeNsAndName} from './html_tags'; import {ParseSourceSpan, ParseError, ParseLocation} from './parse_util'; import {RecursiveAstVisitor, BindingPipe} from 'angular2/src/core/change_detection/parser/ast'; - import { ElementAst, BoundElementPropertyAst, @@ -584,6 +583,12 @@ class TemplateParseVisitor implements HtmlAstVisitor { } else { if (parts[0] == ATTRIBUTE_PREFIX) { boundPropertyName = parts[1]; + let nsSeparatorIdx = boundPropertyName.indexOf(':'); + if (nsSeparatorIdx > -1) { + let ns = boundPropertyName.substring(0, nsSeparatorIdx); + let name = boundPropertyName.substring(nsSeparatorIdx + 1); + boundPropertyName = mergeNsAndName(ns, name); + } bindingType = PropertyBindingType.Attribute; } else if (parts[0] == CLASS_PREFIX) { boundPropertyName = parts[1]; diff --git a/modules/angular2/src/platform/browser/browser_adapter.dart b/modules/angular2/src/platform/browser/browser_adapter.dart index 34ace25a21..8ad87e73b1 100644 --- a/modules/angular2/src/platform/browser/browser_adapter.dart +++ b/modules/angular2/src/platform/browser/browser_adapter.dart @@ -365,9 +365,15 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter { bool hasAttribute(Element element, String attribute) => element.attributes.containsKey(attribute); + bool hasAttributeNS(Element element, String ns, String attribute) => + element.getAttributeNS(ns, attribute) != null; + String getAttribute(Element element, String attribute) => element.getAttribute(attribute); + String getAttributeNS(Element element, String ns, String attribute) => + element.getAttributeNS(ns, attribute); + void setAttribute(Element element, String name, String value) { element.setAttribute(name, value); } @@ -382,6 +388,10 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter { element.attributes.remove(name); } + void removeAttributeNS(Element element, String ns, String name) { + element.getNamespacedAttributes(ns).remove(name); + } + Node templateAwareRoot(Element el) => el is TemplateElement ? el.content : el; HtmlDocument createHtmlDocument() => diff --git a/modules/angular2/src/platform/browser/browser_adapter.ts b/modules/angular2/src/platform/browser/browser_adapter.ts index d01b723923..474e4ea1ff 100644 --- a/modules/angular2/src/platform/browser/browser_adapter.ts +++ b/modules/angular2/src/platform/browser/browser_adapter.ts @@ -225,12 +225,19 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter { return res; } hasAttribute(element, attribute: string): boolean { return element.hasAttribute(attribute); } + hasAttributeNS(element, ns: string, attribute: string): boolean { + return element.hasAttributeNS(ns, attribute); + } getAttribute(element, attribute: string): string { return element.getAttribute(attribute); } + getAttributeNS(element, ns: string, name: string): string { + return element.getAttributeNS(ns, name); + } setAttribute(element, name: string, value: string) { element.setAttribute(name, value); } setAttributeNS(element, ns: string, name: string, value: string) { element.setAttributeNS(ns, name, value); } removeAttribute(element, attribute: string) { element.removeAttribute(attribute); } + removeAttributeNS(element, ns: string, name: string) { element.removeAttributeNS(ns, name); } templateAwareRoot(el): any { return this.isTemplateElement(el) ? this.content(el) : el; } createHtmlDocument(): HTMLDocument { return document.implementation.createHTMLDocument('fakeTitle'); diff --git a/modules/angular2/src/platform/dom/dom_adapter.ts b/modules/angular2/src/platform/dom/dom_adapter.ts index 6f183da665..b058046cc4 100644 --- a/modules/angular2/src/platform/dom/dom_adapter.ts +++ b/modules/angular2/src/platform/dom/dom_adapter.ts @@ -97,10 +97,13 @@ export abstract class DomAdapter { abstract tagName(element): string; abstract attributeMap(element): Map; abstract hasAttribute(element, attribute: string): boolean; + abstract hasAttributeNS(element, ns: string, attribute: string): boolean; abstract getAttribute(element, attribute: string): string; + abstract getAttributeNS(element, ns: string, attribute: string): string; abstract setAttribute(element, name: string, value: string); abstract setAttributeNS(element, ns: string, name: string, value: string); abstract removeAttribute(element, attribute: string); + abstract removeAttributeNS(element, ns: string, attribute: string); abstract templateAwareRoot(el); abstract createHtmlDocument(): HTMLDocument; abstract defaultDoc(): HTMLDocument; diff --git a/modules/angular2/src/platform/dom/dom_renderer.ts b/modules/angular2/src/platform/dom/dom_renderer.ts index 5b7f36551f..d64ce1094e 100644 --- a/modules/angular2/src/platform/dom/dom_renderer.ts +++ b/modules/angular2/src/platform/dom/dom_renderer.ts @@ -178,17 +178,21 @@ export class DomRenderer implements Renderer { var attrNs; var nsAndName = splitNamespace(attributeName); if (isPresent(nsAndName[0])) { - attributeName = nsAndName[0] + ':' + nsAndName[1]; + attributeName = nsAndName[1]; attrNs = NAMESPACE_URIS[nsAndName[0]]; } if (isPresent(attributeValue)) { if (isPresent(attrNs)) { DOM.setAttributeNS(renderElement, attrNs, attributeName, attributeValue); } else { - DOM.setAttribute(renderElement, nsAndName[1], attributeValue); + DOM.setAttribute(renderElement, attributeName, attributeValue); } } else { - DOM.removeAttribute(renderElement, attributeName); + if (isPresent(attrNs)) { + DOM.removeAttributeNS(renderElement, attrNs, attributeName); + } else { + DOM.removeAttribute(renderElement, attributeName); + } } } @@ -332,4 +336,4 @@ function splitNamespace(name: string): string[] { } let match = RegExpWrapper.firstMatch(NS_PREFIX_RE, name); return [match[1], match[2]]; -} \ No newline at end of file +} diff --git a/modules/angular2/src/platform/server/abstract_html_adapter.dart b/modules/angular2/src/platform/server/abstract_html_adapter.dart index 7b136601a6..f0b009eee0 100644 --- a/modules/angular2/src/platform/server/abstract_html_adapter.dart +++ b/modules/angular2/src/platform/server/abstract_html_adapter.dart @@ -295,6 +295,10 @@ abstract class AbstractHtml5LibAdapter implements DomAdapter { return element.attributes.keys.any((key) => '$key' == attribute); } + hasAttributeNS(element, String ns, String attribute) { + throw 'not implemented'; + } + getAttribute(element, String attribute) { // `attributes` keys can be {@link AttributeName}s. var key = element.attributes.keys.firstWhere((key) => '$key' == attribute, @@ -302,6 +306,10 @@ abstract class AbstractHtml5LibAdapter implements DomAdapter { return element.attributes[key]; } + getAttributeNS(element, String ns, String attribute) { + throw 'not implemented'; + } + setAttribute(element, String name, String value) { element.attributes[name] = value; } @@ -314,6 +322,10 @@ abstract class AbstractHtml5LibAdapter implements DomAdapter { element.attributes.remove(attribute); } + removeAttributeNS(element, String ns, String attribute) { + throw 'not implemented'; + } + templateAwareRoot(el) => el; createHtmlDocument() { diff --git a/modules/angular2/src/platform/server/parse5_adapter.ts b/modules/angular2/src/platform/server/parse5_adapter.ts index 9570c74e74..1d9e776229 100644 --- a/modules/angular2/src/platform/server/parse5_adapter.ts +++ b/modules/angular2/src/platform/server/parse5_adapter.ts @@ -429,11 +429,13 @@ export class Parse5DomAdapter extends DomAdapter { hasAttribute(element, attribute: string): boolean { return element.attribs && element.attribs.hasOwnProperty(attribute); } + hasAttributeNS(element, ns: string, attribute: string): boolean { throw 'not implemented'; } getAttribute(element, attribute: string): string { return element.attribs && element.attribs.hasOwnProperty(attribute) ? element.attribs[attribute] : null; } + getAttributeNS(element, ns: string, attribute: string): string { throw 'not implemented'; } setAttribute(element, attribute: string, value: string) { if (attribute) { element.attribs[attribute] = value; @@ -448,6 +450,7 @@ export class Parse5DomAdapter extends DomAdapter { StringMapWrapper.delete(element.attribs, attribute); } } + removeAttributeNS(element, ns: string, name: string) { throw 'not implemented'; } templateAwareRoot(el): any { return this.isTemplateElement(el) ? this.content(el) : el; } createHtmlDocument(): Document { var newDoc = treeAdapter.createDocument(); diff --git a/modules/angular2/test/core/linker/integration_spec.ts b/modules/angular2/test/core/linker/integration_spec.ts index 6eca60cb15..8318aea3a5 100644 --- a/modules/angular2/test/core/linker/integration_spec.ts +++ b/modules/angular2/test/core/linker/integration_spec.ts @@ -1848,7 +1848,7 @@ function declareTests() { if (!IS_DART) { var firstAttribute = DOM.getProperty(use, 'attributes')[0]; - expect(firstAttribute.name).toEqual('xlink:href'); + expect(firstAttribute.name).toEqual('href'); expect(firstAttribute.namespaceURI).toEqual('http://www.w3.org/1999/xlink'); } else { // For Dart where '_Attr' has no instance getter 'namespaceURI' @@ -1860,6 +1860,48 @@ function declareTests() { })); }); + + describe('attributes', () => { + + it('should support attributes with namespace', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, + async) => { + tcb.overrideView(SomeCmp, new ViewMetadata({template: ''})) + .createAsync(SomeCmp) + .then((fixture) => { + let useEl = DOM.firstChild(fixture.debugElement.nativeElement); + expect(DOM.getAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) + .toEqual('#id'); + async.done(); + }); + })); + + it('should support binding to attributes with namespace', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, + async) => { + tcb.overrideView(SomeCmp, + new ViewMetadata({template: ''})) + .createAsync(SomeCmp) + .then((fixture) => { + let cmp = fixture.debugElement.componentInstance; + let useEl = DOM.firstChild(fixture.debugElement.nativeElement); + + cmp.value = "#id"; + fixture.detectChanges(); + + expect(DOM.getAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) + .toEqual('#id'); + + cmp.value = null; + fixture.detectChanges(); + + expect(DOM.hasAttributeNS(useEl, 'http://www.w3.org/1999/xlink', 'href')) + .toEqual(false); + + async.done(); + }); + })); + }); } }); } @@ -2439,3 +2481,8 @@ class DirectiveWithPropDecorators { fireEvent(msg) { ObservableWrapper.callEmit(this.event, msg); } } + +@Component({selector: 'some-cmp'}) +class SomeCmp { + value: any; +} diff --git a/modules/angular2/test/public_api_spec.ts b/modules/angular2/test/public_api_spec.ts index 61051abdeb..1a1e118939 100644 --- a/modules/angular2/test/public_api_spec.ts +++ b/modules/angular2/test/public_api_spec.ts @@ -1556,6 +1556,7 @@ var NG_PLATFORM_BROWSER = [ 'BrowserDomAdapter.firstChild():js', 'BrowserDomAdapter.getAnimationPrefix():js', 'BrowserDomAdapter.getAttribute():js', + 'BrowserDomAdapter.getAttributeNS():js', 'BrowserDomAdapter.getBaseHref():js', 'BrowserDomAdapter.getBoundingClientRect():js', 'BrowserDomAdapter.getChecked():js', @@ -1582,6 +1583,7 @@ var NG_PLATFORM_BROWSER = [ 'BrowserDomAdapter.getValue():js', 'BrowserDomAdapter.getXHR():js', 'BrowserDomAdapter.hasAttribute():js', + 'BrowserDomAdapter.hasAttributeNS():js', 'BrowserDomAdapter.hasClass():js', 'BrowserDomAdapter.hasProperty():js', 'BrowserDomAdapter.hasShadowRoot():js', @@ -1615,6 +1617,7 @@ var NG_PLATFORM_BROWSER = [ 'BrowserDomAdapter.querySelectorAll():js', 'BrowserDomAdapter.remove():js', 'BrowserDomAdapter.removeAttribute():js', + 'BrowserDomAdapter.removeAttributeNS():js', 'BrowserDomAdapter.removeChild():js', 'BrowserDomAdapter.removeClass():js', 'BrowserDomAdapter.removeStyle():js', diff --git a/tools/public_api_guard/public_api_spec.ts b/tools/public_api_guard/public_api_spec.ts index fbd6a20ac3..da842083c0 100644 --- a/tools/public_api_guard/public_api_spec.ts +++ b/tools/public_api_guard/public_api_spec.ts @@ -987,6 +987,7 @@ const BROWSER = [ 'BrowserDomAdapter.elementMatches(n:any, selector:string):boolean', 'BrowserDomAdapter.firstChild(el:any):Node', 'BrowserDomAdapter.getAttribute(element:any, attribute:string):string', + 'BrowserDomAdapter.getAttributeNS(element:any, ns:string, name:string):string', 'BrowserDomAdapter.getBaseHref():string', 'BrowserDomAdapter.getBoundingClientRect(el:any):any', 'BrowserDomAdapter.getChecked(el:any):boolean', @@ -1010,6 +1011,7 @@ const BROWSER = [ 'BrowserDomAdapter.getUserAgent():string', 'BrowserDomAdapter.getValue(el:any):string', 'BrowserDomAdapter.hasAttribute(element:any, attribute:string):boolean', + 'BrowserDomAdapter.hasAttributeNS(element:any, ns:string, attribute:string):boolean', 'BrowserDomAdapter.hasClass(element:any, className:string):boolean', 'BrowserDomAdapter.hasProperty(element:any, name:string):boolean', 'BrowserDomAdapter.hasShadowRoot(node:any):boolean', @@ -1044,6 +1046,7 @@ const BROWSER = [ 'BrowserDomAdapter.querySelectorAll(el:any, selector:string):any[]', 'BrowserDomAdapter.remove(node:any):Node', 'BrowserDomAdapter.removeAttribute(element:any, attribute:string):any', + 'BrowserDomAdapter.removeAttributeNS(element:any, ns:string, name:string):any', 'BrowserDomAdapter.removeChild(el:any, node:any):any', 'BrowserDomAdapter.removeClass(element:any, className:string):any', 'BrowserDomAdapter.removeStyle(element:any, stylename:string):any',