From 1b513a1637ad5c2f3da02b03a5ec49c4889e3592 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Sat, 3 Jul 2021 08:39:11 +0200
Subject: [PATCH] [FIX] qweb: add support for t-att-value on <select> tags

Before this commit, it was not possible to set the value of a select tag
by using the t-att-value attribute.

Doing so is not actually trivial because of the way the vdom works: it
processes the node attributes before its children are created, which
means that the vdom code tries to set the initial value of the select
before its children are created, which means that it is ignored. To make
it work, I added a node create hook which is called after the children
are completely processed.

closes #873
---
 src/qweb/qweb.ts                           | 31 ++++++++++++++------
 tests/qweb/__snapshots__/qweb.test.ts.snap | 33 ++++++++++++++++++++++
 tests/qweb/qweb.test.ts                    | 24 ++++++++++++++++
 3 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/src/qweb/qweb.ts b/src/qweb/qweb.ts
index e90a5625..947680ed 100644
--- a/src/qweb/qweb.ts
+++ b/src/qweb/qweb.ts
@@ -617,13 +617,21 @@ export class QWeb extends EventBus {
     }
 
     if (node.nodeName !== "t" || node.hasAttribute("t-tag")) {
-      let nodeID = this._compileGenericNode(node, ctx, withHandlers);
-      ctx = ctx.withParent(nodeID);
       let nodeHooks = {};
       let addNodeHook = function (hook, handler) {
         nodeHooks[hook] = nodeHooks[hook] || [];
         nodeHooks[hook].push(handler);
       };
+      if (node.tagName === "select" && node.hasAttribute("t-att-value")) {
+        const value = node.getAttribute("t-att-value");
+        let exprId = ctx.generateID();
+        ctx.addLine(`let expr${exprId} = ${ctx.formatExpression(value)};`);
+        let expr = `expr${exprId}`;
+        node.setAttribute("t-att-value", expr);
+        addNodeHook("create", `n.elm.value=${expr};`);
+      }
+      let nodeID = this._compileGenericNode(node, ctx, withHandlers);
+      ctx = ctx.withParent(nodeID);
 
       for (let { directive, value, fullName } of validDirectives) {
         if (directive.atNodeCreation) {
@@ -707,14 +715,16 @@ export class QWeb extends EventBus {
         case "textarea":
           isProp = key === "readonly" || key === "disabled" || key === "value";
           break;
-        case "button":
         case "select":
+          isProp = key === "disabled" || key === "value";
+          break;
+        case "button":
         case "optgroup":
           isProp = key === "disabled";
           break;
       }
       if (isProp) {
-        props.push(`${key}: _${val}`);
+        props.push(`${key}: ${val}`);
       }
     }
     let classObj = "";
@@ -750,7 +760,7 @@ export class QWeb extends EventBus {
             name = '"' + name + '"';
           }
           attrs.push(`${name}: _${attID}`);
-          handleProperties(name, attID);
+          handleProperties(name, "_" + attID);
         }
       }
 
@@ -785,9 +795,14 @@ export class QWeb extends EventBus {
             const attrIndex = attrs.findIndex((att) => att.startsWith(attName + ":"));
             attrs.splice(attrIndex, 1);
           }
-          ctx.addLine(`let _${attID} = ${formattedValue};`);
-          attrs.push(`${attName}: _${attID}`);
-          handleProperties(attName, attID);
+          if (node.nodeName === "select" && attName === "value") {
+            attrs.push(`${attName}: ${v}`);
+            handleProperties(attName, v);
+          } else {
+            ctx.addLine(`let _${attID} = ${formattedValue};`);
+            attrs.push(`${attName}: _${attID}`);
+            handleProperties(attName, "_" + attID);
+          }
         }
       }
 
diff --git a/tests/qweb/__snapshots__/qweb.test.ts.snap b/tests/qweb/__snapshots__/qweb.test.ts.snap
index fa717f67..26dc0ced 100644
--- a/tests/qweb/__snapshots__/qweb.test.ts.snap
+++ b/tests/qweb/__snapshots__/qweb.test.ts.snap
@@ -1073,6 +1073,39 @@ exports[`special cases for some specific html attributes/properties input type=
 }"
 `;
 
+exports[`special cases for some specific html attributes/properties select with t-att-value 1`] = `
+"function anonymous(context, extra
+) {
+    // Template name: \\"test\\"
+    let scope = Object.create(context);
+    let h = this.h;
+    let expr1 = scope['value'];
+    let c3 = [], p3 = {key:3,attrs:{value: expr1},props:{value: expr1}};
+    let vn3 = h('select', p3, c3);
+    p3.hook = {
+      create: (_, n) => {
+        n.elm.value=expr1;
+      },
+    };
+    let _4 = 'potato';
+    let c5 = [], p5 = {key:5,attrs:{value: _4}};
+    let vn5 = h('option', p5, c5);
+    c3.push(vn5);
+    c5.push({text: \`Potato\`});
+    let _6 = 'tomato';
+    let c7 = [], p7 = {key:7,attrs:{value: _6}};
+    let vn7 = h('option', p7, c7);
+    c3.push(vn7);
+    c7.push({text: \`Tomato\`});
+    let _8 = 'onion';
+    let c9 = [], p9 = {key:9,attrs:{value: _8}};
+    let vn9 = h('option', p9, c9);
+    c3.push(vn9);
+    c9.push({text: \`Onion\`});
+    return vn3;
+}"
+`;
+
 exports[`special cases for some specific html attributes/properties various boolean html attributes 1`] = `
 "function anonymous(context, extra
 ) {
diff --git a/tests/qweb/qweb.test.ts b/tests/qweb/qweb.test.ts
index 6f1724be..aaac2895 100644
--- a/tests/qweb/qweb.test.ts
+++ b/tests/qweb/qweb.test.ts
@@ -1950,6 +1950,30 @@ describe("special cases for some specific html attributes/properties", () => {
     patch(vnode2, vnode3);
     expect(elm.value).toBe("potato");
   });
+
+  test("select with t-att-value", () => {
+    const template = `
+      <select t-att-value="value">
+        <option value="potato">Potato</option>
+        <option value="tomato">Tomato</option>
+        <option value="onion">Onion</option>
+      </select>`;
+    qweb.addTemplate("test", template);
+    const vnode1 = qweb.render("test", { value: "tomato" });
+    const vnode2 = patch(document.createElement("select"), vnode1);
+    let elm = vnode2.elm as HTMLSelectElement;
+    expect(elm.value).toBe("tomato");
+
+    elm.value = "potato";
+    expect(elm.value).toBe("potato");
+
+    // rerender with a different value, and patch actual dom, to check that
+    // select value was properly reset by owl
+    const vnode3 = qweb.render("test", { value: "onion" });
+    patch(vnode2, vnode3);
+    expect(elm.value).toBe("onion");
+    expect(qweb.templates.test.fn.toString()).toMatchSnapshot();
+  });
 });
 
 describe("whitespace handling", () => {