From 6f6e2d887d097ae0c8175d7d87f97c83392875da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Fri, 25 Jan 2019 16:00:32 +0100
Subject: [PATCH] qweb: make sure on directive is properly bound

---
 web/static/src/ts/core/qweb_vdom.ts |  3 +-
 web/static/tests/qweb_vdom.test.ts  | 96 ++++++++++-------------------
 2 files changed, 33 insertions(+), 66 deletions(-)

diff --git a/web/static/src/ts/core/qweb_vdom.ts b/web/static/src/ts/core/qweb_vdom.ts
index 4ea5588c..1635ff2b 100644
--- a/web/static/src/ts/core/qweb_vdom.ts
+++ b/web/static/src/ts/core/qweb_vdom.ts
@@ -194,6 +194,7 @@ export class QWeb {
     const ctx = new Context();
     // this is necessary to prevent some directives (t-forach for ex) to
     // pollute the rendering context by adding some keys in it.
+    ctx.addLine("let owner = context");
     ctx.addLine("context = Object.create(context)");
     const mainNode = doc.firstChild!;
     this._compileNode(mainNode, ctx);
@@ -652,7 +653,7 @@ const onDirective: Directive = {
       return "";
     });
     ctx.addLine(
-      `p${nodeID}.on = {${eventName}: context['${handler}'].bind(context${
+      `p${nodeID}.on = {${eventName}: context['${handler}'].bind(owner${
         extraArgs ? ", " + qweb._formatExpression(extraArgs) : ""
       })}`
     );
diff --git a/web/static/tests/qweb_vdom.test.ts b/web/static/tests/qweb_vdom.test.ts
index 839dc66c..f15cf817 100644
--- a/web/static/tests/qweb_vdom.test.ts
+++ b/web/static/tests/qweb_vdom.test.ts
@@ -1,10 +1,23 @@
-import { QWeb, EvalContext } from "../src/ts/core/qweb_vdom";
-import { init } from "../libs/snabbdom/src/snabbdom";
 import sdAttributes from "../libs/snabbdom/src/modules/attributes";
 import sdListeners from "../libs/snabbdom/src/modules/eventlisteners";
+import { init } from "../libs/snabbdom/src/snabbdom";
+import { EvalContext, QWeb } from "../src/ts/core/qweb_vdom";
+
+//------------------------------------------------------------------------------
+// Setup and helpers
+//------------------------------------------------------------------------------
+
+// We create before each test:
+// - qweb: a new QWeb instance
 
 const patch = init([sdAttributes, sdListeners]);
 
+let qweb: QWeb;
+
+beforeEach(() => {
+  qweb = new QWeb();
+});
+
 function trim(str: string): string {
   return str.replace(/\s/g, "");
 }
@@ -32,27 +45,27 @@ function renderToString(
   return node instanceof Text ? node.textContent! : node.outerHTML;
 }
 
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
 describe("static templates", () => {
   test("simple string", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", "<t>hello vdom</t>");
     expect(renderToString(qweb, "test")).toBe("hello vdom");
   });
 
   test("empty div", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", "<div></div>");
     expect(renderToString(qweb, "test")).toBe("<div></div>");
   });
 
   test("div with a text node", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", "<div>word</div>");
     expect(renderToString(qweb, "test")).toBe("<div>word</div>");
   });
 
   test("div with a span child node", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", "<div><span>word</span></div>");
     expect(renderToString(qweb, "test")).toBe("<div><span>word</span></div>");
   });
@@ -60,15 +73,12 @@ describe("static templates", () => {
 
 describe("error handling", () => {
   test("invalid xml", () => {
-    const qweb = new QWeb();
-
     expect(() => qweb.addTemplate("test", "<div>")).toThrow(
       "Invalid XML in template"
     );
   });
 
   test("template with text node and tag", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<t>text<span>other node</span></t>`);
 
     expect(() => renderToString(qweb, "test")).toThrow(
@@ -77,26 +87,22 @@ describe("error handling", () => {
   });
 
   test("nice warning if no template with given name", () => {
-    const qweb = new QWeb();
     expect(() => qweb.render("invalidname")).toThrow("does not exist");
   });
 });
 
 describe("t-esc", () => {
   test("literal", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span><t t-esc="'ok'"/></span>`);
     expect(renderToString(qweb, "test")).toBe("<span>ok</span>");
   });
 
   test("variable", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span><t t-esc="var"/></span>`);
     expect(renderToString(qweb, "test", { var: "ok" })).toBe("<span>ok</span>");
   });
 
   test.skip("escaping", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span><t t-esc="var"/></span>`);
     expect(renderToString(qweb, "test", { var: "<ok>" })).toBe(
       "<span>&lt;ok&gt;</span>"
@@ -104,19 +110,16 @@ describe("t-esc", () => {
   });
 
   test("escaping on a node", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span t-esc="'ok'"/>`);
     expect(renderToString(qweb, "test")).toBe("<span>ok</span>");
   });
 
   test("escaping on a node with a body", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span t-esc="'ok'">nope</span>`);
     expect(renderToString(qweb, "test")).toBe("<span>ok</span>");
   });
 
   test("escaping on a node with a body, as a default", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span t-esc="var">nope</span>`);
     expect(renderToString(qweb, "test")).toBe("<span>nope</span>");
   });
@@ -124,19 +127,16 @@ describe("t-esc", () => {
 
 describe("t-raw", () => {
   test("literal", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span><t t-raw="'ok'"/></span>`);
     expect(renderToString(qweb, "test")).toBe("<span>ok</span>");
   });
 
   test("variable", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span><t t-raw="var"/></span>`);
     expect(renderToString(qweb, "test", { var: "ok" })).toBe("<span>ok</span>");
   });
 
   test("not escaping", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div><t t-raw="var"/></div>`);
     expect(renderToString(qweb, "test", { var: "<ok></ok>" })).toBe(
       "<div><ok></ok></div>"
@@ -146,7 +146,6 @@ describe("t-raw", () => {
 
 describe("t-set", () => {
   test("set from attribute literal", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-set="value" t-value="'ok'"/><t t-esc="value"/></div>`
@@ -155,7 +154,6 @@ describe("t-set", () => {
   });
 
   test("set from body literal", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<t><t t-set="value">ok</t><t t-esc="value"/></t>`
@@ -164,7 +162,6 @@ describe("t-set", () => {
   });
 
   test("set from attribute lookup", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-set="stuff" t-value="value"/><t t-esc="stuff"/></div>`
@@ -173,7 +170,6 @@ describe("t-set", () => {
   });
 
   test("set from body lookup", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-set="stuff"><t t-esc="value"/></t><t t-esc="stuff"/></div>`
@@ -182,13 +178,11 @@ describe("t-set", () => {
   });
 
   test("set from empty body", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div><t t-set="stuff"/><t t-esc="stuff"/></div>`);
     expect(renderToString(qweb, "test")).toBe("<div></div>");
   });
 
   test("value priority", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-set="value" t-value="1">2</t><t t-esc="value"/></div>`
@@ -197,7 +191,6 @@ describe("t-set", () => {
   });
 
   test("evaluate value expression", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-set="value" t-value="1 + 2"/><t t-esc="value"/></div>`
@@ -206,7 +199,6 @@ describe("t-set", () => {
   });
 
   test("evaluate value expression, part 2", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-set="value" t-value="somevariable + 2"/><t t-esc="value"/></div>`
@@ -219,7 +211,6 @@ describe("t-set", () => {
 
 describe("t-if", () => {
   test("boolean value true condition", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div><t t-if="condition">ok</t></div>`);
     expect(renderToString(qweb, "test", { condition: true })).toBe(
       "<div>ok</div>"
@@ -227,7 +218,6 @@ describe("t-if", () => {
   });
 
   test("boolean value false condition", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div><t t-if="condition">ok</t></div>`);
     expect(renderToString(qweb, "test", { condition: false })).toBe(
       "<div></div>"
@@ -235,13 +225,11 @@ describe("t-if", () => {
   });
 
   test("boolean value condition missing", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<span><t t-if="condition">fail</t></span>`);
     expect(renderToString(qweb, "test")).toBe("<span></span>");
   });
 
   test("boolean value condition elif", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-if="color == 'black'">black pearl</t>
@@ -256,7 +244,6 @@ describe("t-if", () => {
   });
 
   test("boolean value condition else", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div>
@@ -272,7 +259,6 @@ describe("t-if", () => {
   });
 
   test("boolean value condition false else", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><span>begin</span><t t-if="condition">fail</t>
@@ -288,7 +274,6 @@ describe("t-if", () => {
 
 describe("attributes", () => {
   test("static attributes", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div foo="a" bar="b" baz="c"/>`);
     const result = renderToString(qweb, "test");
     const expected = `<div foo="a" bar="b" baz="c"></div>`;
@@ -296,49 +281,42 @@ describe("attributes", () => {
   });
 
   test("static attributes on void elements", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<img src="/test.jpg" alt="Test"/>`);
     const result = renderToString(qweb, "test");
     expect(result).toBe(`<img src="/test.jpg" alt="Test">`);
   });
 
   test("dynamic attributes", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-att-foo="'bar'"/>`);
     const result = renderToString(qweb, "test");
     expect(result).toBe(`<div foo="bar"></div>`);
   });
 
   test("fixed variable", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-att-foo="value"/>`);
     const result = renderToString(qweb, "test", { value: "ok" });
     expect(result).toBe(`<div foo="ok"></div>`);
   });
 
   test("dynamic attribute falsy variable ", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-att-foo="value"/>`);
     const result = renderToString(qweb, "test", { value: false });
     expect(result).toBe(`<div></div>`);
   });
 
   test("tuple literal", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-att="['foo', 'bar']"/>`);
     const result = renderToString(qweb, "test");
     expect(result).toBe(`<div foo="bar"></div>`);
   });
 
   test("tuple variable", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-att="value"/>`);
     const result = renderToString(qweb, "test", { value: ["foo", "bar"] });
     expect(result).toBe(`<div foo="bar"></div>`);
   });
 
   test("object", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-att="value"/>`);
     const result = renderToString(qweb, "test", {
       value: { a: 1, b: 2, c: 3 }
@@ -347,28 +325,24 @@ describe("attributes", () => {
   });
 
   test("format literal", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-attf-foo="bar"/>`);
     const result = renderToString(qweb, "test");
     expect(result).toBe(`<div foo="bar"></div>`);
   });
 
   test("format value", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-attf-foo="b{{value}}r"/>`);
     const result = renderToString(qweb, "test", { value: "a" });
     expect(result).toBe(`<div foo="bar"></div>`);
   });
 
   test("format expression", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div t-attf-foo="{{value + 37}}"/>`);
     const result = renderToString(qweb, "test", { value: 5 });
     expect(result).toBe(`<div foo="42"></div>`);
   });
 
   test("format multiple", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div t-attf-foo="a {{value1}} is {{value2}} of {{value3}} ]"/>`
@@ -383,7 +357,6 @@ describe("attributes", () => {
 
   test.skip("various escapes", () => {
     // not needed??
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `
@@ -405,7 +378,6 @@ describe("attributes", () => {
 
 describe("t-call (template calling", () => {
   test("basic caller", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_basic-callee", "<div>ok</div>");
     qweb.addTemplate("caller", '<t t-call="_basic-callee"/>');
     const expected = "<div>ok</div>";
@@ -413,14 +385,12 @@ describe("t-call (template calling", () => {
   });
 
   test("t-call not allowed on a non t node", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_basic-callee", "<t>ok</t>");
     qweb.addTemplate("caller", '<div t-call="_basic-callee"/>');
     expect(() => renderToString(qweb, "caller")).toThrow("Invalid tag");
   });
 
   test("with unused body", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_basic-callee", "<div>ok</div>");
     qweb.addTemplate("caller", '<t t-call="_basic-callee">WHEEE</t>');
     const expected = "<div>ok</div>";
@@ -428,7 +398,6 @@ describe("t-call (template calling", () => {
   });
 
   test("with unused setbody", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_basic-callee", "<div>ok</div>");
     qweb.addTemplate(
       "caller",
@@ -439,7 +408,6 @@ describe("t-call (template calling", () => {
   });
 
   test("with used body", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_callee-printsbody", '<h1><t t-esc="0"/></h1>');
     qweb.addTemplate("caller", '<t t-call="_callee-printsbody">ok</t>');
     const expected = "<h1>ok</h1>";
@@ -447,7 +415,6 @@ describe("t-call (template calling", () => {
   });
 
   test("with used set body", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_callee-uses-foo", '<t t-esc="foo"/>');
     qweb.addTemplate(
       "caller",
@@ -459,7 +426,6 @@ describe("t-call (template calling", () => {
   });
 
   test("inherit context", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_callee-uses-foo", '<t t-esc="foo"/>');
     qweb.addTemplate(
       "caller",
@@ -471,7 +437,6 @@ describe("t-call (template calling", () => {
   });
 
   test("scoped parameters", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_basic-callee", `<t>ok</t>`);
     qweb.addTemplate(
       "caller",
@@ -491,7 +456,6 @@ describe("t-call (template calling", () => {
 
 describe("foreach", () => {
   test("iterate on items", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `
@@ -507,7 +471,6 @@ describe("foreach", () => {
   });
 
   test("iterate on items (on a element node)", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `
@@ -521,7 +484,6 @@ describe("foreach", () => {
   });
 
   test("iterate, position", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `
@@ -537,7 +499,6 @@ describe("foreach", () => {
   });
 
   test("iterate, integer param", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div><t t-foreach="3" t-as="item">
@@ -550,7 +511,6 @@ describe("foreach", () => {
   });
 
   test("iterate, dict param", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `
@@ -568,7 +528,6 @@ describe("foreach", () => {
   });
 
   test("does not pollute the rendering context", () => {
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `<div>
@@ -583,7 +542,6 @@ describe("foreach", () => {
 
 describe("misc", () => {
   test("global", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("_callee-asc", `<Año t-att-falló="'agüero'" t-raw="0"/>`);
     qweb.addTemplate(
       "_callee-uses-foo",
@@ -645,7 +603,6 @@ describe("misc", () => {
 
 describe("t-on", () => {
   test("can bind event handler", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<button t-on-click="add">Click</button>`);
     let a = 1;
     const node = renderToDOM(qweb, "test", {
@@ -658,7 +615,6 @@ describe("t-on", () => {
   });
 
   test("can bind handlers with arguments", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<button t-on-click="add(5)">Click</button>`);
     let a = 1;
     const node = renderToDOM(qweb, "test", {
@@ -672,7 +628,6 @@ describe("t-on", () => {
 
   test("can bind handlers with loop variable as argument", () => {
     expect.assertions(1);
-    const qweb = new QWeb();
     qweb.addTemplate(
       "test",
       `
@@ -687,11 +642,22 @@ describe("t-on", () => {
     });
     (<HTMLElement>node).getElementsByTagName("a")[0].click();
   });
+
+  test("handler is bound to proper owner", () => {
+    expect.assertions(1);
+    qweb.addTemplate("test", `<button t-on-click="add">Click</button>`);
+    let owner = {
+      add() {
+        expect(this).toBe(owner);
+      }
+    };
+    const node = renderToDOM(qweb, "test", owner);
+    (<HTMLElement>node).click();
+  });
 });
 
 describe("t-ref", () => {
   test("can get a ref on a node", () => {
-    const qweb = new QWeb();
     qweb.addTemplate("test", `<div><span t-ref="myspan"/></div>`);
     let refs: any = {};
     renderToDOM(qweb, "test", { refs });