From ecf12f6eeebf25d6b13e5e6bcb5a15340b1de77f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Tue, 24 Sep 2019 21:38:51 +0200
Subject: [PATCH] [FIX] qweb: properly supports svg

Snabbdom supports svg, but it adds the namespace at the creation of the
virtual node.  However, owl works slightly differently: it adds children
after creating the parent virtual node.

So, we need to actually call the addNS method after the children nodes
have been created.

As a bonus, this is slightly faster than snabbdom: we only check at
template compilation time once if a node is a svg.
---
 src/qweb/qweb.ts                           | 10 ++++++++-
 src/vdom/vdom.ts                           | 10 +--------
 tests/qweb/__snapshots__/qweb.test.ts.snap | 25 ++++++++++++++++++++++
 tests/qweb/qweb.test.ts                    |  7 ++++++
 4 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts
index 5e442ded..9ec5ce2c 100644
--- a/src/qweb/qweb.ts
+++ b/src/qweb/qweb.ts
@@ -2,6 +2,7 @@ import { EventBus } from "../core/event_bus";
 import { h, patch, VNode } from "../vdom/index";
 import { Context } from "./context";
 import { shallowEqual } from "../utils";
+import { addNS } from "../vdom/vdom";
 
 /**
  * Owl QWeb Engine
@@ -93,7 +94,10 @@ const UTILS: Utils = {
     }
     return expr;
   },
-  shallowEqual
+  shallowEqual,
+  addNameSpace(vnode) {
+      addNS(vnode.data, vnode.children, vnode.sel);
+  },
 };
 
 function parseXML(xml: string): Document {
@@ -537,6 +541,10 @@ export class QWeb extends EventBus {
     }
 
     this._compileChildren(node, ctx);
+    if (node.nodeName === 'svg') {
+        ctx.rootContext.shouldDefineUtils = true;
+        ctx.addLine(`utils.addNameSpace(vn${ctx.parentNode});`);
+    }
 
     for (let { directive, value, fullName } of validDirectives) {
       if (directive.finalize) {
diff --git a/src/vdom/vdom.ts b/src/vdom/vdom.ts
index 1e766ada..38fe34f5 100644
--- a/src/vdom/vdom.ts
+++ b/src/vdom/vdom.ts
@@ -564,7 +564,7 @@ type VNodeChildElement = VNode | string | number | undefined | null;
 type ArrayOrElement<T> = T | T[];
 type VNodeChildren = ArrayOrElement<VNodeChildElement>;
 
-function addNS(data: any, children: VNodes | undefined, sel: string | undefined): void {
+export function addNS(data: any, children: VNodes | undefined, sel: string | undefined): void {
   data.ns = "http://www.w3.org/2000/svg";
   if (sel !== "foreignObject" && children !== undefined) {
     for (let i = 0, iLen = children.length; i < iLen; ++i) {
@@ -612,13 +612,5 @@ export function h(sel: any, b?: any, c?: any): VNode {
         children[i] = vnode(undefined, undefined, undefined, children[i], undefined);
     }
   }
-  if (
-    sel[0] === "s" &&
-    sel[1] === "v" &&
-    sel[2] === "g" &&
-    (sel.length === 3 || sel[3] === "." || sel[3] === "#")
-  ) {
-    addNS(data, children, sel);
-  }
   return vnode(sel, data, children, text, undefined);
 }
diff --git a/tests/qweb/__snapshots__/qweb.test.ts.snap b/tests/qweb/__snapshots__/qweb.test.ts.snap
index eb49df37..7de37f63 100644
--- a/tests/qweb/__snapshots__/qweb.test.ts.snap
+++ b/tests/qweb/__snapshots__/qweb.test.ts.snap
@@ -676,6 +676,31 @@ exports[`misc global 1`] = `
 }"
 `;
 
+exports[`properly support svg add proper namespace to svg 1`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.constructor.utils;
+    var h = this.h;
+    var _1 = '100px';
+    var _2 = '90px';
+    let c3 = [], p3 = {key:3,attrs:{width: _1,height: _2}};
+    var vn3 = h('svg', p3, c3);
+    result = vn3;
+    var _4 = '50';
+    var _5 = '50';
+    var _6 = '4';
+    var _7 = 'green';
+    var _8 = '1';
+    var _9 = 'yellow';
+    let c10 = [], p10 = {key:10,attrs:{cx: _4,cy: _5,r: _6,stroke: _7,\\"stroke-width\\": _8,fill: _9}};
+    var vn10 = h('circle', p10, c10);
+    c3.push(vn10);
+    c3.push({text: \` \`});
+    utils.addNameSpace(vn3);
+    return vn3;
+}"
+`;
+
 exports[`special cases for some boolean html attributes/properties input type= checkbox, with t-att-checked 1`] = `
 "function anonymous(context,extra
 ) {
diff --git a/tests/qweb/qweb.test.ts b/tests/qweb/qweb.test.ts
index 982da66d..2016ad7e 100644
--- a/tests/qweb/qweb.test.ts
+++ b/tests/qweb/qweb.test.ts
@@ -1312,3 +1312,10 @@ describe("global template registration", () => {
     expect((vnode as any).children[0].text).toBe("global");
   });
 });
+
+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>`);
+  });
+});