[FIX] component: make sure default slot is applied

Before this commit, owl was erroneously defining default slots in most
cases, even though they are empty. The problem occurs when the content
of a component slots is a t-set-slot, and we remove that, then use the
rest as default slots, even though it is only reduced to a set of text
nodes.

With this commit, we only consider the content as default content if it
is not only a sequence of text nodes containing only spaces.

closes #882

src/component/directive.ts

@@ -452,12 +452,17 @@ QWeb.addDirective({
         }
       }
       if (clone.childNodes.length) {
+        let hasContent = false;
         const t = clone.ownerDocument!.createElement("t");
         for (let child of Object.values(clone.childNodes)) {
+          hasContent =
+            hasContent || (child instanceof Text ? Boolean(child.textContent.trim().length) : true);
           t.appendChild(child);
         }
-        const slotFn = qweb._compile(`slot_default_template`, { elem: t, hasParent: true });
+        if (hasContent) {
-        QWeb.slots[`${slotId}_default`] = slotFn;
+          const slotFn = qweb._compile(`slot_default_template`, { elem: t, hasParent: true });
+          QWeb.slots[`${slotId}_default`] = slotFn;
+        }
       }
     }
 

tests/component/__snapshots__/slots.test.ts.snap

@@ -44,23 +44,23 @@ exports[`t-slot directive can define and call slots 2`] = `
 ) {
     // Template name: \\"Dialog\\"
     let h = this.h;
+    let c8 = [], p8 = {key:8};
+    let vn8 = h('div', p8, c8);
     let c9 = [], p9 = {key:9};
     let vn9 = h('div', p9, c9);
-    let c10 = [], p10 = {key:10};
+    c8.push(vn9);
-    let vn10 = h('div', p10, c10);
+    const slot10 = this.constructor.slots[context.__owl__.slotId + '_' + 'header'];
-    c9.push(vn10);
+    if (slot10) {
-    const slot11 = this.constructor.slots[context.__owl__.slotId + '_' + 'header'];
+        slot10.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c9, parent: extra.parent || context}));
-    if (slot11) {
-        slot11.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c10, parent: extra.parent || context}));
     }
-    let c12 = [], p12 = {key:12};
+    let c11 = [], p11 = {key:11};
-    let vn12 = h('div', p12, c12);
+    let vn11 = h('div', p11, c11);
-    c9.push(vn12);
+    c8.push(vn11);
-    const slot13 = this.constructor.slots[context.__owl__.slotId + '_' + 'footer'];
+    const slot12 = this.constructor.slots[context.__owl__.slotId + '_' + 'footer'];
-    if (slot13) {
+    if (slot12) {
-        slot13.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c12, parent: extra.parent || context}));
+        slot12.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c11, parent: extra.parent || context}));
     }
-    return vn9;
+    return vn8;
 }"
 `;
 
@@ -136,23 +136,23 @@ exports[`t-slot directive can define and call slots using old t-set keyword 2`]
 ) {
     // Template name: \\"__template__1\\"
     let h = this.h;
+    let c8 = [], p8 = {key:8};
+    let vn8 = h('div', p8, c8);
     let c9 = [], p9 = {key:9};
     let vn9 = h('div', p9, c9);
-    let c10 = [], p10 = {key:10};
+    c8.push(vn9);
-    let vn10 = h('div', p10, c10);
+    const slot10 = this.constructor.slots[context.__owl__.slotId + '_' + 'header'];
-    c9.push(vn10);
+    if (slot10) {
-    const slot11 = this.constructor.slots[context.__owl__.slotId + '_' + 'header'];
+        slot10.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c9, parent: extra.parent || context}));
-    if (slot11) {
-        slot11.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c10, parent: extra.parent || context}));
     }
-    let c12 = [], p12 = {key:12};
+    let c11 = [], p11 = {key:11};
-    let vn12 = h('div', p12, c12);
+    let vn11 = h('div', p11, c11);
-    c9.push(vn12);
+    c8.push(vn11);
-    const slot13 = this.constructor.slots[context.__owl__.slotId + '_' + 'footer'];
+    const slot12 = this.constructor.slots[context.__owl__.slotId + '_' + 'footer'];
-    if (slot13) {
+    if (slot12) {
-        slot13.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c12, parent: extra.parent || context}));
+        slot12.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c11, parent: extra.parent || context}));
     }
-    return vn9;
+    return vn8;
 }"
 `;
 
@@ -215,6 +215,45 @@ exports[`t-slot directive dafault slots can define a default content 1`] = `
 }"
 `;
 
+exports[`t-slot directive default slot next to named slot, with default content 1`] = `
+"function anonymous(context, extra
+) {
+    // Template name: \\"__template__2\\"
+    let utils = this.constructor.utils;
+    let QWeb = this.constructor;
+    let parent = context;
+    let scope = Object.create(context);
+    let h = this.h;
+    let c1 = [], p1 = {key:1};
+    let vn1 = h('div', p1, c1);
+    // Component 'Dialog'
+    let w2 = '__3__' in parent.__owl__.cmap ? parent.__owl__.children[parent.__owl__.cmap['__3__']] : false;
+    let props2 = {};
+    if (w2 && w2.__owl__.currentFiber && !w2.__owl__.vnode) {
+        w2.destroy();
+        w2 = false;
+    }
+    if (w2) {
+        w2.__updateProps(props2, extra.fiber, utils.combine(context, scope));
+        let pvnode = w2.__owl__.pvnode;
+        c1.push(pvnode);
+    } else {
+        let componentKey2 = \`Dialog\`;
+        let W2 = scope['Dialog'] || context.constructor.components[componentKey2] || QWeb.components[componentKey2];
+        if (!W2) {throw new Error('Cannot find the definition of component \\"' + componentKey2 + '\\"')}
+        w2 = new W2(parent, props2);
+        parent.__owl__.cmap['__3__'] = w2.__owl__.id;
+        w2.__owl__.slotId = 1;
+        let fiber = w2.__prepare(extra.fiber, utils.combine(context, scope), () => { const vnode = fiber.vnode; pvnode.sel = vnode.sel; });
+        let pvnode = h('dummy', {key: '__3__', hook: {remove() {},destroy(vn) {w2.destroy();}}});
+        c1.push(pvnode);
+        w2.__owl__.pvnode = pvnode;
+    }
+    w2.__owl__.parentLastFiberId = extra.fiber.id;
+    return vn1;
+}"
+`;
+
 exports[`t-slot directive default slot work with text nodes 1`] = `
 "function anonymous(context, extra
 ) {
@@ -233,15 +272,15 @@ exports[`t-slot directive dynamic t-slot call 1`] = `
     let utils = this.constructor.utils;
     let scope = Object.create(context);
     let h = this.h;
-    let c10 = [], p10 = {key:10,on:{}};
+    let c9 = [], p9 = {key:9,on:{}};
-    let vn10 = h('button', p10, c10);
+    let vn9 = h('button', p9, c9);
-    extra.handlers['click__11__'] = extra.handlers['click__11__'] || function (e) {if (context.__owl__.status === 5){return}utils.getComponent(context)['toggle'](e);};
+    extra.handlers['click__10__'] = extra.handlers['click__10__'] || function (e) {if (context.__owl__.status === 5){return}utils.getComponent(context)['toggle'](e);};
-    p10.on['click'] = extra.handlers['click__11__'];
+    p9.on['click'] = extra.handlers['click__10__'];
-    const slot12 = this.constructor.slots[context.__owl__.slotId + '_' + (scope['current'].slot)];
+    const slot11 = this.constructor.slots[context.__owl__.slotId + '_' + (scope['current'].slot)];
-    if (slot12) {
+    if (slot11) {
-        slot12.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c10, parent: extra.parent || context}));
+        slot11.call(this, context.__owl__.scope, Object.assign({}, extra, {parentNode: c9, parent: extra.parent || context}));
     }
-    return vn10;
+    return vn9;
 }"
 `;
 

tests/component/slots.test.ts

@@ -467,6 +467,48 @@ describe("t-slot directive", () => {
     expect(QWeb.slots["1_default"].toString()).toMatchSnapshot();
   });
 
+  test("default slot next to named slot, with default content", async () => {
+    class Dialog extends Component {
+      // We're using 2 slots here: a "default" one and a "footer",
+      // both having default children nodes.
+      static template = xml`
+        <div class="Dialog">
+          <div class="content">
+            <t t-slot="default">
+              Default content
+            </t>
+          </div>
+          <div class="footer">
+            <t t-slot="footer">
+              Default footer
+            </t>
+          </div>
+        </div>
+      `;
+    }
+
+    class App extends Component {
+      // Here we're trying to assign the "footer" slot with some content
+      static components = { Dialog };
+      static template = xml`
+        <div>
+          <Dialog>
+            <t t-set-slot="footer">
+              Overridden footer
+            </t>
+          </Dialog>
+        </div>
+      `;
+    }
+
+    await mount(App, { target: fixture });
+
+    expect(fixture.innerHTML).toBe(
+      '<div><div class="Dialog"><div class="content"> Default content </div><div class="footer"> Overridden footer </div></div></div>'
+    );
+    expect(QWeb.TEMPLATES[App.template].fn.toString()).toMatchSnapshot();
+  });
+
   test("multiple roots are allowed in a named slot", async () => {
     env.qweb.addTemplates(`
           <templates>