[FIX] compiler: better handle update of properties with same value

Before this commit, owl would incorrectly skip patching html properties
when the new value is the same as the value used in the previous render.
However, this is incorrect, since the user may have changed the value by
clicking on a checkbox, or changing some text in an input.

So, to be more correct, owl has to force the update whenever it
encounters a prop.  This is however hard to do without impacting the
main update loop, so we kind of work around the problem by using String
and Boolean instance, instead of primitive values.

src/compiler/code_generator.ts

@@ -1,3 +1,4 @@
+import { isProp } from "../runtime/blockdom/attributes";
 import {
   compileExpr,
   compileExprToArray,
@@ -22,12 +23,12 @@ import {
   ASTTif,
   ASTTKey,
   ASTTOut,
-  ASTTSet,
-  ASTTranslation,
-  ASTType,
   ASTTPortal,
-  EventHandlers,
+  ASTTranslation,
+  ASTTSet,
+  ASTType,
   Attrs,
+  EventHandlers,
 } from "./parser";
 
 type BlockType = "block" | "text" | "multi" | "list" | "html" | "comment";
@@ -578,13 +579,18 @@ export class CodeGenerator {
         attrName = key.slice(7);
         attrs["block-attribute-" + idx] = attrName;
       } else if (key.startsWith("t-att")) {
+        attrName = key === "t-att" ? null : key.slice(6);
         expr = compileExpr(ast.attrs[key]);
+        if (attrName && isProp(ast.tag, attrName)) {
+          // we force a new string or new boolean to bypass the equality check in blockdom when patching same value
+          const C = attrName === "value" ? "String" : "Boolean";
+          expr = `new ${C}(${expr})`;
+        }
         const idx = block!.insertData(expr, "attr");
         if (key === "t-att") {
           attrs[`block-attributes`] = String(idx);
         } else {
-          attrName = key.slice(6);
-          attrs[`block-attribute-${idx}`] = attrName;
+          attrs[`block-attribute-${idx}`] = attrName!;
         }
       } else if (this.translatableAttributes.includes(key)) {
         attrs[key] = this.translateFn(ast.attrs[key]);

src/runtime/blockdom/attributes.ts

@@ -144,7 +144,7 @@ export function updateClass(this: HTMLElement, val: any, oldVal: any) {
 export function makePropSetter(name: string): Setter<HTMLElement> {
   return function setProp(this: HTMLElement, value: any) {
     // support 0, fallback to empty string for other falsy values
-    (this as any)[name] = value === 0 ? 0 : value || "";
+    (this as any)[name] = value === 0 ? 0 : value ? value.valueOf() : "";
   };
 }
 

tests/compiler/__snapshots__/attributes.test.ts.snap

@@ -745,7 +745,7 @@ exports[`special cases for some specific html attributes/properties input of typ
   let block1 = createBlock(\`<input type=\\"checkbox\\" block-attribute-0=\\"indeterminate\\"/>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    let attr1 = ctx['v'];
+    let attr1 = new Boolean(ctx['v']);
     return block1([attr1]);
   }
 }"
@@ -759,7 +759,21 @@ exports[`special cases for some specific html attributes/properties input type=
   let block1 = createBlock(\`<input type=\\"checkbox\\" block-attribute-0=\\"checked\\"/>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    let attr1 = ctx['flag'];
+    let attr1 = new Boolean(ctx['flag']);
+    return block1([attr1]);
+  }
+}"
+`;
+
+exports[`special cases for some specific html attributes/properties input with t-att-value (patching with same value 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<input block-attribute-0=\\"value\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = new String(ctx['v']);
     return block1([attr1]);
   }
 }"
@@ -773,7 +787,21 @@ exports[`special cases for some specific html attributes/properties input with t
   let block1 = createBlock(\`<input block-attribute-0=\\"value\\"/>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    let attr1 = ctx['v'];
+    let attr1 = new String(ctx['v']);
+    return block1([attr1]);
+  }
+}"
+`;
+
+exports[`special cases for some specific html attributes/properties input, type checkbox, with t-att-checked (patching with same value 1`] = `
+"function anonymous(app, bdom, helpers
+) {
+  let { text, createBlock, list, multi, html, toggler, comment } = bdom;
+  
+  let block1 = createBlock(\`<input type=\\"checkbox\\" block-attribute-0=\\"checked\\"/>\`);
+  
+  return function template(ctx, node, key = \\"\\") {
+    let attr1 = new Boolean(ctx['v']);
     return block1([attr1]);
   }
 }"
@@ -787,7 +815,7 @@ exports[`special cases for some specific html attributes/properties select with
   let block1 = createBlock(\`<select block-attribute-0=\\"value\\"><option value=\\"potato\\">Potato</option><option value=\\"tomato\\">Tomato</option><option value=\\"onion\\">Onion</option></select>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    let attr1 = ctx['value'];
+    let attr1 = new String(ctx['value']);
     return block1([attr1]);
   }
 }"
@@ -801,7 +829,7 @@ exports[`special cases for some specific html attributes/properties textarea wit
   let block1 = createBlock(\`<textarea block-attribute-0=\\"value\\"/>\`);
   
   return function template(ctx, node, key = \\"\\") {
-    let attr1 = ctx['v'];
+    let attr1 = new String(ctx['v']);
     return block1([attr1]);
   }
 }"

tests/compiler/__snapshots__/misc.test.ts.snap

@@ -269,7 +269,7 @@ exports[`misc other complex template 1`] = `
         ctx[\`category\`] = v_block15[i1];
         const key1 = ctx['category'].id;
         let attr6 = ctx['category'].id;
-        let attr7 = ctx['category'].id==ctx['options'].active_category_id;
+        let attr7 = new Boolean(ctx['category'].id==ctx['options'].active_category_id);
         let txt5 = ctx['category'].name;
         c_block15[i1] = withKey(block16([attr6, attr7, txt5]), key1);
       }
@@ -277,7 +277,7 @@ exports[`misc other complex template 1`] = `
       const b15 = list(c_block15);
       b14 = block14([], [b15]);
     }
-    let attr8 = ctx['search'].value;
+    let attr8 = new String(ctx['search'].value);
     let hdlr4 = [ctx['updateFilter'], ctx];
     let hdlr5 = [ctx['updateFilter'], ctx];
     let hdlr6 = [ctx['clearSearch'], ctx];
@@ -291,7 +291,7 @@ exports[`misc other complex template 1`] = `
         if (!ctx['trigger'].manual&&ctx['trigger'].project_id===ctx['project'].id&&ctx['trigger'].category_id===ctx['options'].active_category_id) {
           let attr9 = \`trigger_\${ctx['trigger'].id}\`;
           let attr10 = \`trigger_\${ctx['trigger'].id}\`;
-          let attr11 = ctx['options'].trigger_display[ctx['trigger'].id];
+          let attr11 = new Boolean(ctx['options'].trigger_display[ctx['trigger'].id]);
           let attr12 = ctx['trigger'].id;
           let hdlr7 = [ctx['updateTriggerDisplay'], ctx];
           let attr13 = \`trigger_\${ctx['trigger'].id}\`;

tests/compiler/attributes.test.ts

@@ -429,6 +429,42 @@ describe("special cases for some specific html attributes/properties", () => {
     expect(input.value).toBe("potato");
   });
 
+  test("input with t-att-value (patching with same value", () => {
+    // render input with initial value
+    const template = `<input t-att-value="v"/>`;
+    const bnode1 = renderToBdom(template, { v: "zucchini" });
+    const fixture = makeTestFixture();
+    mount(bnode1, fixture);
+    const input = fixture.querySelector("input")!;
+    expect(input.value).toBe("zucchini");
+
+    // change value manually in input, to simulate user input
+    input.value = "tomato";
+    expect(input.value).toBe("tomato");
+
+    const bnode2 = renderToBdom(template, { v: "zucchini" });
+    patch(bnode1, bnode2);
+    expect(input.value).toBe("zucchini");
+  });
+
+  test("input, type checkbox, with t-att-checked (patching with same value", () => {
+    // render input with initial value
+    const template = `<input type="checkbox" t-att-checked="v"/>`;
+    const bnode1 = renderToBdom(template, { v: true });
+    const fixture = makeTestFixture();
+    mount(bnode1, fixture);
+    const input = fixture.querySelector("input")!;
+    expect(input.checked).toBe(true);
+
+    // change checked manually in input, to simulate user input
+    input.checked = false;
+    expect(input.checked).toBe(false);
+
+    const bnode2 = renderToBdom(template, { v: true });
+    patch(bnode1, bnode2);
+    expect(input.checked).toBe(true);
+  });
+
   test("input of type checkbox with t-att-indeterminate", () => {
     const template = `<input type="checkbox" t-att-indeterminate="v"/>`;
     const bnode1 = renderToBdom(template, { v: true });