From f12d3373c91934ff5f6525a5fdca896fea908752 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Tue, 3 Dec 2019 14:02:58 +0100
Subject: [PATCH] [FIX] qweb: properly handle empty class attribute

The classList.add method actually crashes when given an empty string (or
a string with just white spaces).

closes #530
---
 src/qweb/qweb.ts                           | 15 +++----
 src/vdom/modules.ts                        |  2 +-
 tests/component/component.test.ts          | 13 ++++++
 tests/qweb/__snapshots__/qweb.test.ts.snap | 47 ++++++++++++++++++++++
 tests/qweb/qweb.test.ts                    | 22 ++++++++++
 5 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts
index 2189eb0e..835bf32b 100644
--- a/src/qweb/qweb.ts
+++ b/src/qweb/qweb.ts
@@ -650,13 +650,14 @@ export class QWeb extends EventBus {
       if (!name.startsWith("t-") && !(<Element>node).getAttribute("t-attf-" + name)) {
         const attID = ctx.generateID();
         if (name === "class") {
-          let classDef = value
-            .trim()
-            .split(/\s+/)
-            .map(a => `'${a}':true`)
-            .join(",");
-          classObj = `_${ctx.generateID()}`;
-          ctx.addLine(`let ${classObj} = {${classDef}};`);
+          if (value = value.trim()) {
+            let classDef = value
+              .split(/\s+/)
+              .map(a => `'${a}':true`)
+              .join(",");
+            classObj = `_${ctx.generateID()}`;
+            ctx.addLine(`let ${classObj} = {${classDef}};`);
+          }
         } else {
           ctx.addLine(`var _${attID} = '${value}';`);
           if (!name.match(/^[a-zA-Z]+$/)) {
diff --git a/src/vdom/modules.ts b/src/vdom/modules.ts
index c887bb4e..1dcec67a 100644
--- a/src/vdom/modules.ts
+++ b/src/vdom/modules.ts
@@ -219,7 +219,7 @@ function updateClass(oldVnode: VNode, vnode: VNode): void {
   elm = vnode.elm as Element;
 
   for (name in oldClass) {
-    if (!klass[name]) {
+    if (name && !klass[name]) {
       elm.classList.remove(name);
     }
   }
diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts
index 7e587899..445edf61 100644
--- a/tests/component/component.test.ts
+++ b/tests/component/component.test.ts
@@ -1614,6 +1614,19 @@ describe("class and style attributes with t-component", () => {
     expect(fixture.innerHTML).toBe(`<div><div class="c a b"></div></div>`);
   });
 
+  test("empty class attribute is not added on widget root el", async () => {
+    class Child extends Component<any, any> {
+      static template = xml`<span/>`;
+    }
+    class Parent extends Component<any, any> {
+      static template = xml`<div><Child class=""/></div>`;
+      static components = { Child };
+    }
+    const widget = new Parent();
+    await widget.mount(fixture);
+    expect(fixture.innerHTML).toBe(`<div><span></span></div>`);
+  });
+
   test("t-att-class is properly added/removed on widget root el", async () => {
     class Child extends Widget {
       static template = xml`<div class="c"/>`;
diff --git a/tests/qweb/__snapshots__/qweb.test.ts.snap b/tests/qweb/__snapshots__/qweb.test.ts.snap
index 14ac1876..0d617962 100644
--- a/tests/qweb/__snapshots__/qweb.test.ts.snap
+++ b/tests/qweb/__snapshots__/qweb.test.ts.snap
@@ -44,6 +44,30 @@ exports[`attributes dynamic attributes 1`] = `
 }"
 `;
 
+exports[`attributes dynamic class attribute 1`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.constructor.utils;
+    var h = this.h;
+    let _1 = utils.toObj(context['c']);
+    let c2 = [], p2 = {key:2,class:_1};
+    var vn2 = h('div', p2, c2);
+    return vn2;
+}"
+`;
+
+exports[`attributes dynamic empty class attribute 1`] = `
+"function anonymous(context,extra
+) {
+    let utils = this.constructor.utils;
+    var h = this.h;
+    let _1 = utils.toObj(context['c']);
+    let c2 = [], p2 = {key:2,class:_1};
+    var vn2 = h('div', p2, c2);
+    return vn2;
+}"
+`;
+
 exports[`attributes dynamic formatted attributes with a dash 1`] = `
 "function anonymous(context,extra
 ) {
@@ -787,6 +811,29 @@ exports[`special cases for some boolean html attributes/properties various boole
 }"
 `;
 
+exports[`static templates div with a class attribute 1`] = `
+"function anonymous(context,extra
+) {
+    var h = this.h;
+    let _2 = {'abc':true};
+    let c3 = [], p3 = {key:3,class:_2};
+    var vn3 = h('div', p3, c3);
+    c3.push({text: \`word\`});
+    return vn3;
+}"
+`;
+
+exports[`static templates div with a empty class attribute 1`] = `
+"function anonymous(context,extra
+) {
+    var h = this.h;
+    let c2 = [], p2 = {key:2};
+    var vn2 = h('div', p2, c2);
+    c2.push({text: \`word\`});
+    return vn2;
+}"
+`;
+
 exports[`static templates div with a span child node 1`] = `
 "function anonymous(context,extra
 ) {
diff --git a/tests/qweb/qweb.test.ts b/tests/qweb/qweb.test.ts
index 01e8de53..91ee0495 100644
--- a/tests/qweb/qweb.test.ts
+++ b/tests/qweb/qweb.test.ts
@@ -46,6 +46,16 @@ describe("static templates", () => {
     expect(renderToString(qweb, "test")).toBe("<div>word</div>");
   });
 
+  test("div with a class attribute", () => {
+    qweb.addTemplate("test", `<div class="abc">word</div>`);
+    expect(renderToString(qweb, "test")).toBe(`<div class="abc">word</div>`);
+  });
+
+  test("div with a empty class attribute", () => {
+    qweb.addTemplate("test", `<div class="">word</div>`);
+    expect(renderToString(qweb, "test")).toBe(`<div>word</div>`);
+  });
+
   test("div with a span child node", () => {
     qweb.addTemplate("test", "<div><span>word</span></div>");
     expect(renderToString(qweb, "test")).toBe("<div><span>word</span></div>");
@@ -432,6 +442,18 @@ describe("attributes", () => {
     expect(result).toBe(`<div foo="bar"></div>`);
   });
 
+  test("dynamic class attribute", () => {
+    qweb.addTemplate("test", `<div t-att-class="c"/>`);
+    const result = renderToString(qweb, "test", { c: "abc" });
+    expect(result).toBe(`<div class="abc"></div>`);
+  });
+
+  test("dynamic empty class attribute", () => {
+    qweb.addTemplate("test", `<div t-att-class="c"/>`);
+    const result = renderToString(qweb, "test", { c: "" });
+    expect(result).toBe(`<div></div>`);
+  });
+
   test("dynamic attribute with a dash", () => {
     qweb.addTemplate("test", `<div t-att-data-action-id="id"/>`);
     const result = renderToString(qweb, "test", { id: 32 });