From cdad48d3a6e0d0ba5a71d200ce7c10f279b9c61f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Wed, 1 Mar 2023 12:21:40 +0100 Subject: [PATCH] [FIX] compiler: properly handle readonly attribute/readOnly property Before this commit, Owl template compiler would handle readonly attribute as readonly properties. But this is incorrect, since the property is actually named readOnly. With this commit, we make sure that readonly AND readOnly are both interpreted as the `readOnly` property. This is due to the fact that QWeb does not discriminate between attribute and properties, so we have to infer which is which. closes #1362 --- src/compiler/code_generator.ts | 7 ++- .../__snapshots__/properties.test.ts.snap | 54 +++++++++++++++++++ tests/compiler/properties.test.ts | 25 +++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/src/compiler/code_generator.ts b/src/compiler/code_generator.ts index f3b74f94..76c37fca 100644 --- a/src/compiler/code_generator.ts +++ b/src/compiler/code_generator.ts @@ -65,12 +65,13 @@ function isProp(tag: string, key: string): boolean { key === "indeterminate" || key === "value" || key === "readonly" || + key === "readOnly" || key === "disabled" ); case "option": return key === "selected" || key === "disabled"; case "textarea": - return key === "value" || key === "readonly" || key === "disabled"; + return key === "value" || key === "readonly" || key === "readOnly" || key === "disabled"; case "select": return key === "value" || key === "disabled"; case "button": @@ -600,6 +601,10 @@ export class CodeGenerator { attrName = key === "t-att" ? null : key.slice(6); expr = compileExpr(ast.attrs[key]); if (attrName && isProp(ast.tag, attrName)) { + if (attrName === "readonly") { + // the property has a different name than the attribute + attrName = "readOnly"; + } // we force a new string or new boolean to bypass the equality check in blockdom when patching same value if (attrName === "value") { // When the expression is falsy (except 0), fall back to an empty string diff --git a/tests/compiler/__snapshots__/properties.test.ts.snap b/tests/compiler/__snapshots__/properties.test.ts.snap index 4abcc25a..557537e8 100644 --- a/tests/compiler/__snapshots__/properties.test.ts.snap +++ b/tests/compiler/__snapshots__/properties.test.ts.snap @@ -140,6 +140,60 @@ exports[`input, type checkbox, with t-att-checked (patching with same value 1`] }" `; +exports[`readonly and readOnly are both interpreted as the readOnly property 1`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + +exports[`readonly and readOnly are both interpreted as the readOnly property 2`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + return block1(); + } +}" +`; + +exports[`readonly and readOnly are both interpreted as the readOnly property 3`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + let prop1 = new Boolean(ctx['val']); + return block1([prop1]); + } +}" +`; + +exports[`readonly and readOnly are both interpreted as the readOnly property 4`] = ` +"function anonymous(app, bdom, helpers +) { + let { text, createBlock, list, multi, html, toggler, comment } = bdom; + + let block1 = createBlock(\`\`); + + return function template(ctx, node, key = \\"\\") { + let prop1 = new Boolean(ctx['val']); + return block1([prop1]); + } +}" +`; + exports[`select with t-att-value 1`] = ` "function anonymous(app, bdom, helpers ) { diff --git a/tests/compiler/properties.test.ts b/tests/compiler/properties.test.ts index 98624117..23f60c89 100644 --- a/tests/compiler/properties.test.ts +++ b/tests/compiler/properties.test.ts @@ -186,3 +186,28 @@ test("select with t-att-value", () => { patch(bnode1, bnode2); expect(elm.value).toBe("onion"); }); + +test("readonly and readOnly are both interpreted as the readOnly property", () => { + // render input with initial value + const staticTemplates = [``, ``]; + for (let template of staticTemplates) { + const fixture = makeTestFixture(); + const bnode1 = renderToBdom(template); + mount(bnode1, fixture); + const input = fixture.querySelector("input")!; + expect(input.readOnly).toBe(true); + } + + const dynamicTemplates = [``, ``]; + for (let template of dynamicTemplates) { + const fixture = makeTestFixture(); + const bnode1 = renderToBdom(template, { val: true }); + mount(bnode1, fixture); + const input = fixture.querySelector("input")!; + expect(input.readOnly).toBe(true); + + const bnode2 = renderToBdom(template, { val: false }); + patch(bnode1, bnode2); + expect(input.readOnly).toBe(false); + } +});