From ec05b1f5e30176ee36b4007af1b52ac161362a23 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Thu, 8 Jul 2021 11:33:25 +0200
Subject: [PATCH] [FIX] slots: fix bad interaction between t-call and slots

The previous changes in the combine method (used to copy all the
variables defined in the current scope for use in a slot) had the effect
of squashing the prototype chain: instead of `Component -> Obj1 ->
Obj2 -> Obj3`, the combined scope had: `Component -> Obj1'`.

This has an unfortunate interaction with the way t-call is implemented,
which uses the fact that we are in a subscope to add a own
__access_mode__. It depends specifially on the prototype chain, and that
the parent scope may have a different value for that property. But with
the way combine was implemented, we lost all that subtlety since
everything is squashed.

In this commit, we reimplement that function in a way to make sure we
keep the prototype chain structure
---
 src/qweb/qweb.ts              | 18 ++++++++--
 tests/component/slots.test.ts | 67 +++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts
index 2de45442..0e006c5d 100644
--- a/src/qweb/qweb.ts
+++ b/src/qweb/qweb.ts
@@ -137,12 +137,26 @@ const UTILS: Utils = {
     }
     return result;
   },
+  /**
+   * This method combines the current context with the variables defined in a
+   * scope for use in a slot.
+   * 
+   * The implementation is kind of tricky because we want to preserve the
+   * prototype chain structure of the cloned result. So we need to traverse the
+   * prototype chain, cloning each level respectively.
+   */
   combine(context, scope) {
-    const clone = Object.create(context);
+    let clone = context;
+    const scopeStack = [];
     while (!isComponent(scope)) {
-      Object.assign(clone, scope);
+      scopeStack.push(scope);
       scope = scope.__proto__;
     }
+    while (scopeStack.length) {
+      let scope = scopeStack.pop();
+      clone = Object.create(clone);
+      Object.assign(clone, scope);
+    }
     return clone;
   },
   shallowEqual,
diff --git a/tests/component/slots.test.ts b/tests/component/slots.test.ts
index 54297cdc..ca9f88e4 100644
--- a/tests/component/slots.test.ts
+++ b/tests/component/slots.test.ts
@@ -1303,4 +1303,71 @@ describe("t-slot directive", () => {
     document.querySelector("button").click();
     await nextTick();
   });
+
+  test("t-slot in recursive templates", async () => {
+    QWeb.registerTemplate(
+      "_test_recursive_template",
+      `
+      <Wrapper>
+          <t t-esc="name" />
+          <t t-foreach="items" t-as="item">
+
+            <t t-if="!item.children.length">
+              <t t-esc="item.name" />
+            </t>
+
+            <t t-else="" t-call="_test_recursive_template">
+              <t t-set="name" t-value="item.name" />
+              <t t-set="items" t-value="item.children" />
+            </t>
+
+          </t>
+      </Wrapper>`
+    );
+
+    class Wrapper extends Component {
+      static template = xml`
+        <wrapper>
+          <t t-slot="default"/>
+        </wrapper>`;
+    }
+
+    class Parent extends Component {
+      static template = "_test_recursive_template";
+      static components = { Wrapper };
+      name = "foo";
+      items = [
+        {
+          name: "foo-0",
+          children: [
+            { name: "foo-00", children: [] },
+            {
+              name: "foo-01",
+              children: [
+                { name: "foo-010", children: [] },
+                { name: "foo-011", children: [] },
+                {
+                  name: "foo-012",
+                  children: [
+                    { name: "foo-0120", children: [] },
+                    { name: "foo-0121", children: [] },
+                    { name: "foo-0122", children: [] },
+                  ],
+                },
+              ],
+            },
+            { name: "foo-02", children: [] },
+          ],
+        },
+        { name: "foo-1", children: [] },
+        { name: "foo-2", children: [] },
+      ];
+    }
+
+    await mount(Parent, { target: fixture });
+
+    expect(fixture.innerHTML).toBe(
+      "<wrapper>foo<wrapper>foo-0foo-00<wrapper>foo-01foo-010foo-011<wrapper>foo-012foo-0120foo-0121foo-0122</wrapper></wrapper>foo-02</wrapper>foo-1foo-2</wrapper>"
+    );
+  });
 });