Skip to content

fix(render): create svg elements with the right namespace #4949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tbosch wants to merge 1 commit into from
Closed

fix(render): create svg elements with the right namespace #4949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions modules/angular2/src/core/dom/abstract_html_adapter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ abstract class AbstractHtml5LibAdapter implements DomAdapter {
return new Element.tag(tagName);
}

createElementNS(ns, tagName, [doc]) {
throw 'not implemented';
Copy link
Copy Markdown
Contributor

@vicb vicb Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class is abstract, no need to throw (other places should also be modified)

Copy link
Copy Markdown
Contributor

@yjbanov yjbanov Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vicb: could you please file an issue for this? There are lots of places where we need to clean this up. For now Tobias' change is fine. The P1 goal is to roll out pre-compiler.

Copy link
Copy Markdown
Contributor Author

@tbosch tbosch Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then I need to implement it in all sub classes...

}

createTextNode(String text, [doc]) => new Text(text);

createScriptTag(String attrName, String attrValue, [doc]) {
Expand Down Expand Up @@ -297,6 +301,10 @@ abstract class AbstractHtml5LibAdapter implements DomAdapter {
element.attributes[name] = value;
}

setAttributeNS(element, String ns, String name, String value) {
throw 'not implemented';
}

removeAttribute(element, String attribute) {
element.attributes.remove(attribute);
}
Expand Down
9 changes: 9 additions & 0 deletions modules/angular2/src/core/dom/browser_adapter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,11 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter {
return doc.createElement(tagName);
}

Element createElementNS(String ns, String tagName, [HtmlDocument doc = null]) {
if (doc == null) doc = document;
return doc.createElementNS(ns, tagName);
}

Text createTextNode(String text, [HtmlDocument doc = null]) {
return new Text(text);
}
Expand Down Expand Up @@ -354,6 +359,10 @@ class BrowserDomAdapter extends GenericBrowserDomAdapter {
element.setAttribute(name, value);
}

void setAttributeNS(Element element, String ns, String name, String value) {
element.setAttributeNS(ns, name, value);
}

void removeAttribute(Element element, String name) {
//there is no removeAttribute method as of now in Dart:
//https://code.google.com/p/dart/issues/detail?id=19934
Expand Down
4 changes: 4 additions & 0 deletions modules/angular2/src/core/dom/browser_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
return t;
}
createElement(tagName, doc = document): HTMLElement { return doc.createElement(tagName); }
createElementNS(ns, tagName, doc = document): Element { return doc.createElementNS(ns, tagName); }
createTextNode(text: string, doc = document): Text { return doc.createTextNode(text); }
createScriptTag(attrName: string, attrValue: string, doc = document): HTMLScriptElement {
var el = <HTMLScriptElement>doc.createElement('SCRIPT');
Expand Down Expand Up @@ -225,6 +226,9 @@ export class BrowserDomAdapter extends GenericBrowserDomAdapter {
hasAttribute(element, attribute: string): boolean { return element.hasAttribute(attribute); }
getAttribute(element, attribute: string): string { return element.getAttribute(attribute); }
setAttribute(element, name: string, value: string) { element.setAttribute(name, value); }
setAttributeNS(ns: string, element, name: string, value: string) {
element.setAttributeNS(ns, name, value);
}
removeAttribute(element, attribute: string) { element.removeAttribute(attribute); }
templateAwareRoot(el): any { return this.isTemplateElement(el) ? this.content(el) : el; }
createHtmlDocument(): HTMLDocument {
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/core/dom/dom_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export abstract class DomAdapter {
abstract createComment(text: string): any;
abstract createTemplate(html): HTMLElement;
abstract createElement(tagName, doc?): HTMLElement;
abstract createElementNS(ns: string, tagName: string, doc?): Element;
abstract createTextNode(text: string, doc?): Text;
abstract createScriptTag(attrName: string, attrValue: string, doc?): HTMLElement;
abstract createStyleElement(css: string, doc?): HTMLStyleElement;
Expand All @@ -93,6 +94,7 @@ export abstract class DomAdapter {
abstract hasAttribute(element, attribute: string): boolean;
abstract getAttribute(element, 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 templateAwareRoot(el);
abstract createHtmlDocument(): HTMLDocument;
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/core/dom/parse5_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ export class Parse5DomAdapter extends DomAdapter {
createElement(tagName): HTMLElement {
return treeAdapter.createElement(tagName, 'http://www.w3.org/1999/xhtml', []);
}
createElementNS(ns, tagName): HTMLElement { throw 'not implemented'; }
createTextNode(text: string): Text {
var t = <any>this.createComment(text);
t.type = 'text';
Expand Down Expand Up @@ -435,6 +436,7 @@ export class Parse5DomAdapter extends DomAdapter {
}
}
}
setAttributeNS(element, ns: string, attribute: string, value: string) { throw 'not implemented'; }
removeAttribute(element, attribute: string) {
if (attribute) {
StringMapWrapper.delete(element.attribs, attribute);
Expand Down
104 changes: 99 additions & 5 deletions modules/angular2/src/core/render/dom/dom_renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,92 @@ import {createRenderView, NodeFactory} from '../view_factory';
import {DefaultRenderView, DefaultRenderFragmentRef, DefaultProtoViewRef} from '../view';
import {camelCaseToDashCase} from './util';

// TODO(tbosch): solve SVG properly once https://github.com/angular/angular/issues/4417 is done
const XLINK_NAMESPACE = 'http://www.w3.org/1999/xlink';
const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
const SVG_ELEMENT_NAMES = CONST_EXPR({
Copy link
Copy Markdown
Contributor

@vicb vicb Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: SVG_ELEMENTS ?

'altGlyph': true,
'altGlyphDef': true,
'altGlyphItem': true,
'animate': true,
'animateColor': true,
'animateMotion': true,
'animateTransform': true,
'circle': true,
'clipPath': true,
'color-profile': true,
'cursor': true,
'defs': true,
'desc': true,
'ellipse': true,
'feBlend': true,
'feColorMatrix': true,
'feComponentTransfer': true,
'feComposite': true,
'feConvolveMatrix': true,
'feDiffuseLighting': true,
'feDisplacementMap': true,
'feDistantLight': true,
'feFlood': true,
'feFuncA': true,
'feFuncB': true,
'feFuncG': true,
'feFuncR': true,
'feGaussianBlur': true,
'feImage': true,
'feMerge': true,
'feMergeNode': true,
'feMorphology': true,
'feOffset': true,
'fePointLight': true,
'feSpecularLighting': true,
'feSpotLight': true,
'feTile': true,
'feTurbulence': true,
'filter': true,
'font': true,
'font-face': true,
'font-face-format': true,
'font-face-name': true,
'font-face-src': true,
'font-face-uri': true,
'foreignObject': true,
'g': true,
'glyph': true,
'glyphRef': true,
'hkern': true,
'image': true,
'line': true,
'linearGradient': true,
'marker': true,
'mask': true,
'metadata': true,
'missing-glyph': true,
'mpath': true,
'path': true,
'pattern': true,
'polygon': true,
'polyline': true,
'radialGradient': true,
'rect': true,
'set': true,
'stop': true,
'style': true,
'svg': true,
'switch': true,
'symbol': true,
'text': true,
'textPath': true,
'title': true,
'tref': true,
'tspan': true,
'use': true,
'view': true,
'vkern': true
});

const SVG_ATTR_NAMESPACES = CONST_EXPR({'href': XLINK_NAMESPACE});

export abstract class DomRenderer extends Renderer implements NodeFactory<Node> {
abstract registerComponentTemplate(templateId: number, commands: RenderTemplateCmd[],
styles: string[], nativeShadow: boolean);
Expand Down Expand Up @@ -271,17 +357,25 @@ export class DomRenderer_ extends DomRenderer {
wtfLeave(s);
}
createElement(name: string, attrNameAndValues: string[]): Node {
var el = DOM.createElement(name);
this._setAttributes(el, attrNameAndValues);
var isSvg = SVG_ELEMENT_NAMES[name] == true;
var el = isSvg ? DOM.createElementNS(SVG_NAMESPACE, name) : DOM.createElement(name);
this._setAttributes(el, attrNameAndValues, isSvg);
return el;
}
mergeElement(existing: Node, attrNameAndValues: string[]) {
DOM.clearNodes(existing);
this._setAttributes(existing, attrNameAndValues);
this._setAttributes(existing, attrNameAndValues, false);
}
private _setAttributes(node: Node, attrNameAndValues: string[]) {
private _setAttributes(node: Node, attrNameAndValues: string[], isSvg: boolean) {
for (var attrIdx = 0; attrIdx < attrNameAndValues.length; attrIdx += 2) {
DOM.setAttribute(node, attrNameAndValues[attrIdx], attrNameAndValues[attrIdx + 1]);
var attrName = attrNameAndValues[attrIdx];
var attrValue = attrNameAndValues[attrIdx + 1];
var attrNs = isSvg ? SVG_ATTR_NAMESPACES[attrName] : null;
if (isPresent(attrNs)) {
DOM.setAttributeNS(node, XLINK_NAMESPACE, attrName, attrValue);
} else {
DOM.setAttribute(node, attrName, attrValue);
}
}
}
createRootContentInsertionPoint(): Node {
Expand Down
23 changes: 23 additions & 0 deletions modules/angular2/test/core/linker/integration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1741,6 +1741,29 @@ export function main() {
});
}));
});

if (DOM.supportsDOMEvents()) {
describe('svg', () => {
it('should support svg elements',
inject([TestComponentBuilder, AsyncTestCompleter],
(tcb: TestComponentBuilder, async) => {
tcb.overrideView(MyComp, new ViewMetadata({template: '<svg><g></g></svg>'}))
.createAsync(MyComp)
.then((rootTC) => {
var el = rootTC.debugElement.nativeElement;
var svg = DOM.childNodes(el)[0];
var g = DOM.childNodes(svg)[0];
expect(DOM.getProperty(<Element>svg, 'namespaceURI'))
.toEqual('http://www.w3.org/2000/svg');
expect(DOM.getProperty(<Element>g, 'namespaceURI'))
.toEqual('http://www.w3.org/2000/svg');

async.done();
});
}));

});
}
});
}

Expand Down