From 2a40907e35c0bd707b2c62ef6b843bbcc78f499b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Mon, 9 Sep 2019 11:48:15 +0200 Subject: [PATCH] [FIX] qweb: do not crash in some rare t-call situation Before this commit, QWeb crashed when it had to deal with a t-call with multiple sub nodes on the same line, and one text node: hey hey This was because we need to compile the content to be able to extract all possible t-set t-value statements. But doing so means that we had a multiple root templates, which caused a crash in this case. Solving this issue is simple: the sub template is compiled with the flag allowMultipleRoots set to true. --- src/qweb/base_directives.ts | 1 + tests/component/component.test.ts | 24 ++++++++--------- tests/qweb/__snapshots__/qweb.test.ts.snap | 22 ++++++++++++++++ tests/qweb/qweb.test.ts | 30 +++++++++++++++++----- 4 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/qweb/base_directives.ts b/src/qweb/base_directives.ts index a1bc2293..16a17b39 100644 --- a/src/qweb/base_directives.ts +++ b/src/qweb/base_directives.ts @@ -200,6 +200,7 @@ QWeb.addDirective({ // extract variables from nodecopy const tempCtx = new Context(); tempCtx.nextID = ctx.rootContext.nextID; + tempCtx.allowMultipleRoots = true; qweb._compileNode(nodeCopy, tempCtx); const vars = Object.assign({}, ctx.variables, tempCtx.variables); ctx.rootContext.nextID = tempCtx.nextID; diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index 21407951..b90e7294 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -3051,8 +3051,8 @@ describe("t-slot directive", () => { ); expect(env.qweb.templates.Parent.fn.toString()).toMatchSnapshot(); expect(env.qweb.templates.Dialog.fn.toString()).toMatchSnapshot(); - expect(env.qweb.slots['1_header'].toString()).toMatchSnapshot(); - expect(env.qweb.slots['1_footer'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_header"].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_footer"].toString()).toMatchSnapshot(); }); test("slots are rendered with proper context", async () => { @@ -3088,7 +3088,7 @@ describe("t-slot directive", () => { expect(fixture.innerHTML).toBe( '
1
' ); - expect(env.qweb.slots['1_footer'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_footer"].toString()).toMatchSnapshot(); }); test("slots are rendered with proper context, part 2", async () => { @@ -3126,7 +3126,7 @@ describe("t-slot directive", () => { expect(fixture.innerHTML).toBe( '
  • User Aaron
  • User Mathieu
    • ' ); - expect(env.qweb.slots['1_default'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_default"].toString()).toMatchSnapshot(); }); test("slots are rendered with proper context, part 3", async () => { @@ -3165,7 +3165,7 @@ describe("t-slot directive", () => { expect(fixture.innerHTML).toBe( '
  • User Aaron
  • User Mathieu
    • ' ); - expect(env.qweb.slots['1_default'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_default"].toString()).toMatchSnapshot(); }); test("slots are rendered with proper context, part 4", async () => { @@ -3198,7 +3198,7 @@ describe("t-slot directive", () => { app.state.user.name = "David"; await nextTick(); expect(fixture.innerHTML).toBe('
    User David
    '); - expect(env.qweb.slots['1_default'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_default"].toString()).toMatchSnapshot(); }); test("refs are properly bound in slots", async () => { @@ -3234,7 +3234,7 @@ describe("t-slot directive", () => { expect(fixture.innerHTML).toBe( '
    1
    ' ); - expect(env.qweb.slots['1_footer'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_footer"].toString()).toMatchSnapshot(); }); test("content is the default slot", async () => { @@ -3256,7 +3256,7 @@ describe("t-slot directive", () => { await parent.mount(fixture); expect(fixture.innerHTML).toBe("
    sts rocks
    "); - expect(env.qweb.slots['1_default'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_default"].toString()).toMatchSnapshot(); }); test("default slot work with text nodes", async () => { @@ -3276,7 +3276,7 @@ describe("t-slot directive", () => { await parent.mount(fixture); expect(fixture.innerHTML).toBe("
    sts rocks
    "); - expect(env.qweb.slots['1_default'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_default"].toString()).toMatchSnapshot(); }); test("multiple roots are allowed in a named slot", async () => { @@ -3301,7 +3301,7 @@ describe("t-slot directive", () => { await parent.mount(fixture); expect(fixture.innerHTML).toBe("
    stsrocks
    "); - expect(env.qweb.slots['1_content'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_content"].toString()).toMatchSnapshot(); }); test("multiple roots are allowed in a default slot", async () => { @@ -3324,7 +3324,7 @@ describe("t-slot directive", () => { await parent.mount(fixture); expect(fixture.innerHTML).toBe("
    stsrocks
    "); - expect(env.qweb.slots['1_default'].toString()).toMatchSnapshot(); + expect(env.qweb.slots["1_default"].toString()).toMatchSnapshot(); }); test("missing slots are ignored", async () => { @@ -3624,7 +3624,7 @@ describe("t-model directive", () => { `); class SomeComponent extends Widget { - state = { something: {text: "" }}; + state = { something: { text: "" } }; } const comp = new SomeComponent(env); await comp.mount(fixture); diff --git a/tests/qweb/__snapshots__/qweb.test.ts.snap b/tests/qweb/__snapshots__/qweb.test.ts.snap index 31cc678a..3ca6146f 100644 --- a/tests/qweb/__snapshots__/qweb.test.ts.snap +++ b/tests/qweb/__snapshots__/qweb.test.ts.snap @@ -775,6 +775,28 @@ exports[`t-call (template calling basic caller 1`] = ` }" `; +exports[`t-call (template calling call with several sub nodes on same line 1`] = ` +"function anonymous(context,extra +) { + var h = this.h; + let c1 = [], p1 = {key:1}; + var vn1 = h('div', p1, c1); + let c5 = [], p5 = {key:5}; + var vn5 = h('div', p5, c5); + c1.push(vn5); + let c6 = [], p6 = {key:6}; + var vn6 = h('span', p6, c6); + c5.push(vn6); + c6.push({text: \`hey\`}); + c5.push({text: \` \`}); + let c7 = [], p7 = {key:7}; + var vn7 = h('span', p7, c7); + c5.push(vn7); + c7.push({text: \`yay\`}); + return vn1; +}" +`; + exports[`t-call (template calling inherit context 1`] = ` "function anonymous(context,extra ) { diff --git a/tests/qweb/qweb.test.ts b/tests/qweb/qweb.test.ts index c8c4d398..7bcd8e62 100644 --- a/tests/qweb/qweb.test.ts +++ b/tests/qweb/qweb.test.ts @@ -574,6 +574,24 @@ describe("t-call (template calling", () => { expect(trim(renderToString(qweb, "caller"))).toBe(expected); }); + test("call with several sub nodes on same line", () => { + qweb.addTemplates(` + +
    + +
    + +
    + + hey yay + +
    +     + `); + const expected = "
    hey yay
    "; + expect(renderToString(qweb, "main")).toBe(expected); + }); + test("recursive template, part 1", () => { qweb.addTemplates(` @@ -589,7 +607,6 @@ describe("t-call (template calling", () => { expect(renderToString(qweb, "recursive")).toBe(expected); const recursiveFn = Object.values(qweb.recursiveFns)[0]; expect(recursiveFn.toString()).toMatchSnapshot(); - }); test("recursive template, part 2", () => { @@ -611,9 +628,9 @@ describe("t-call (template calling", () => { `); - const root = { val: "a", children: [{val: "b"}, {val: "c"}]}; + const root = { val: "a", children: [{ val: "b" }, { val: "c" }] }; const expected = "

    a

    b

    c

    "; - expect(renderToString(qweb, "Parent", {root })).toBe(expected); + expect(renderToString(qweb, "Parent", { root })).toBe(expected); const recursiveFn = Object.values(qweb.recursiveFns)[0]; expect(recursiveFn.toString()).toMatchSnapshot(); }); @@ -637,9 +654,10 @@ describe("t-call (template calling", () => { `); - const root = { val: "a", children: [{val: "b", children: [{val: "d"}]}, {val: "c"}]}; - const expected = "

    a

    b

    d

    c

    "; - expect(renderToString(qweb, "Parent", {root })).toBe(expected); + const root = { val: "a", children: [{ val: "b", children: [{ val: "d" }] }, { val: "c" }] }; + const expected = + "

    a

    b

    d

    c

    "; + expect(renderToString(qweb, "Parent", { root })).toBe(expected); const recursiveFn = Object.values(qweb.recursiveFns)[0]; expect(recursiveFn.toString()).toMatchSnapshot(); });