[FIX] qweb: improved svg support for components

If we want to generate dynamically svg components, it is useful to be
able to define components with a <g> tag as root, and then getting the
correct namespace.

closes #302

src/qweb/qweb.ts

@@ -96,8 +96,8 @@ const UTILS: Utils = {
   },
   shallowEqual,
   addNameSpace(vnode) {
-      addNS(vnode.data, vnode.children, vnode.sel);
-  },
+    addNS(vnode.data, vnode.children, vnode.sel);
+  }
 };
 
 function parseXML(xml: string): Document {
@@ -541,9 +541,16 @@ export class QWeb extends EventBus {
     }
 
     this._compileChildren(node, ctx);
-    if (node.nodeName === 'svg') {
-        ctx.rootContext.shouldDefineUtils = true;
-        ctx.addLine(`utils.addNameSpace(vn${ctx.parentNode});`);
+    // svg support
+    // we hadd svg namespace if it is a svg or if it is a g, but only if it is
+    // the root node.  This is the easiest way to support svg sub components:
+    // they need to have a g tag as root. Otherwise, we would need a complete
+    // list of allowed svg tags.
+    const shouldAddNS =
+      node.nodeName === "svg" || (node.nodeName === "g" && ctx.rootNode === ctx.parentNode);
+    if (shouldAddNS) {
+      ctx.rootContext.shouldDefineUtils = true;
+      ctx.addLine(`utils.addNameSpace(vn${ctx.parentNode});`);
     }
 
     for (let { directive, value, fullName } of validDirectives) {

src/vdom/vdom.ts

@@ -568,9 +568,13 @@ export function addNS(data: any, children: VNodes | undefined, sel: string | und
   data.ns = "http://www.w3.org/2000/svg";
   if (sel !== "foreignObject" && children !== undefined) {
     for (let i = 0, iLen = children.length; i < iLen; ++i) {
-      let childData = children[i].data;
+      const child = children[i];
+      if (child === null) {
+        continue;
+      }
+      let childData = child.data;
       if (childData !== undefined) {
-        addNS(childData, (children[i] as VNode).children as VNodes, children[i].sel);
+        addNS(childData, (child as VNode).children as VNodes, child.sel);
       }
     }
   }

tests/component/component.test.ts

@@ -4418,3 +4418,28 @@ describe("dynamic t-props", () => {
     expect(env.qweb.templates[Parent.template].fn.toString()).toMatchSnapshot();
   });
 });
+
+describe("support svg components", () => {
+  test("add proper namespace to svg", async () => {
+    class GComp extends Widget {
+      static template = xml`
+        <g>
+            <circle cx="50" cy="50" r="4" stroke="green" stroke-width="1" fill="yellow"/>
+        </g>`;
+    }
+
+    class Svg extends Widget {
+      static template = xml`
+        <svg>
+            <GComp/>
+        </svg>`;
+      static components = { GComp };
+    }
+    const widget = new Svg(env);
+    await widget.mount(fixture);
+
+    expect(fixture.innerHTML).toBe(
+      '<svg><g><circle cx="50" cy="50" r="4" stroke="green" stroke-width="1" fill="yellow"></circle></g></svg>'
+    );
+  });
+});

tests/qweb/__snapshots__/qweb.test.ts.snap

@@ -676,6 +676,29 @@ exports[`misc global 1`] = `
 }"
 `;
 
+exports[`properly support svg add proper namespace to g tags 1`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.constructor.utils;
+    var h = this.h;
+    let c1 = [], p1 = {key:1};
+    var vn1 = h('g', p1, c1);
+    result = vn1;
+    var _2 = '50';
+    var _3 = '50';
+    var _4 = '4';
+    var _5 = 'green';
+    var _6 = '1';
+    var _7 = 'yellow';
+    let c8 = [], p8 = {key:8,attrs:{cx: _2,cy: _3,r: _4,stroke: _5,\\"stroke-width\\": _6,fill: _7}};
+    var vn8 = h('circle', p8, c8);
+    c1.push(vn8);
+    c1.push({text: \` \`});
+    utils.addNameSpace(vn1);
+    return vn1;
+}"
+`;
+
 exports[`properly support svg add proper namespace to svg 1`] = `
 "function anonymous(context,extra
 ) {

tests/qweb/qweb.test.ts

@@ -1315,7 +1315,23 @@ describe("global template registration", () => {
 
 describe("properly support svg", () => {
   test("add proper namespace to svg", () => {
-    qweb.addTemplate("test", `<svg width="100px" height="90px"><circle cx="50" cy="50" r="4" stroke="green" stroke-width="1" fill="yellow"/> </svg>`);
-    expect(renderToString(qweb, "test")).toBe(`<svg width=\"100px\" height=\"90px\"><circle cx=\"50\" cy=\"50\" r=\"4\" stroke=\"green\" stroke-width=\"1\" fill=\"yellow\"></circle> </svg>`);
+    qweb.addTemplate(
+      "test",
+      `<svg width="100px" height="90px"><circle cx="50" cy="50" r="4" stroke="green" stroke-width="1" fill="yellow"/> </svg>`
+    );
+    expect(renderToString(qweb, "test")).toBe(
+      `<svg width=\"100px\" height=\"90px\"><circle cx=\"50\" cy=\"50\" r=\"4\" stroke=\"green\" stroke-width=\"1\" fill=\"yellow\"></circle> </svg>`
+    );
+  });
+
+  test("add proper namespace to g tags", () => {
+    // this is necessary if one wants to use components in a svg
+    qweb.addTemplate(
+      "test",
+      `<g><circle cx="50" cy="50" r="4" stroke="green" stroke-width="1" fill="yellow"/> </g>`
+    );
+    expect(renderToString(qweb, "test")).toBe(
+      `<g><circle cx=\"50\" cy=\"50\" r=\"4\" stroke=\"green\" stroke-width=\"1\" fill=\"yellow\"></circle> </g>`
+    );
   });
 });

tests/vdom.test.ts

@@ -1,5 +1,5 @@
 import { h, patch } from "../src/vdom";
-import { init } from "../src/vdom/vdom";
+import { init, addNS } from "../src/vdom/vdom";
 
 function map(list, fn) {
   var ret: any[] = [];
@@ -206,19 +206,17 @@ describe("snabbdom", function() {
       expect(elm.firstChild.namespaceURI).toBe(SVGNamespace);
 
       // verify that svg tag automatically gets svg namespace
-      elm = patch(vnode0, h("svg", [h("foreignObject", [h("div", ["I am HTML embedded in SVG"])])]))
+       const vnode = h("svg", [h("foreignObject", [h("div", ["I am HTML embedded in SVG"])])]);
+       // need to add namespace manually. it is usually done by the template
+       // compiler
+      addNS(vnode.data,(vnode as any).children, vnode.sel);
+
+      elm = patch(vnode0, vnode)
         .elm;
       expect(elm.namespaceURI).toBe(SVGNamespace);
       expect(elm.firstChild.namespaceURI).toBe(SVGNamespace);
       expect(elm.firstChild.firstChild.namespaceURI).toBe(XHTMLNamespace);
 
-      // verify that svg tag with extra selectors gets svg namespace
-      elm = patch(vnode0, h("svg#some-id")).elm;
-      expect(elm.namespaceURI).toBe(SVGNamespace);
-
-      // verify that non-svg tag beginning with 'svg' does NOT get namespace
-      elm = patch(vnode0, h("svg-custom-el")).elm;
-      expect(elm.namespaceURI).not.toBe(SVGNamespace);
     });
 
     test("receives classes in selector", function() {