From 96b6b6591fca4b17babc7755de8444c52eef0a26 Mon Sep 17 00:00:00 2001 From: Jorge Pinna Puissant Date: Thu, 27 Jul 2023 08:55:21 +0000 Subject: [PATCH] [FIX] web: code editor shouldn't accept markup as value Before this commit, when using the ace field widget with a html field, the markup value of the field was sent to the code editor. The issue with this is that the ace library only accept strings (it will extract the string from the markup), and when ask for the current value, it will respond in string. Because of this, the ace field will always be considered as dirty, even if not modified, because it will compare the initially sent value (a markup) with the value given by the library (a string). Now, the formatText used on the ace field will always send a string (remove the markup). And the code editor will check the props to only accept strings. X-original-commit: 1778672e9b0076bf385d46e68efb4914d6bf790c Part-of: odoo/odoo#129995 --- .../src/core/code_editor/code_editor.js | 2 +- .../static/src/views/fields/ace/ace_field.js | 3 +- .../web/static/src/views/fields/formatters.js | 25 ++++++++- .../static/tests/core/code_editor_tests.js | 49 ++++++++++++++++- .../views/fields/ace_editor_field_tests.js | 53 +++++++++++++++++++ .../tests/views/fields/formatters_tests.js | 12 +++++ 6 files changed, 139 insertions(+), 5 deletions(-) diff --git a/addons/web/static/src/core/code_editor/code_editor.js b/addons/web/static/src/core/code_editor/code_editor.js index 335350ca031..5d50d1c6da6 100644 --- a/addons/web/static/src/core/code_editor/code_editor.js +++ b/addons/web/static/src/core/code_editor/code_editor.js @@ -31,7 +31,7 @@ export class CodeEditor extends Component { optional: true, validate: (mode) => CodeEditor.MODES.includes(mode), }, - value: { type: String, optional: true }, + value: { validate: (v) => typeof v === "string", optional: true }, readonly: { type: Boolean, optional: true }, onChange: { type: Function, optional: true }, onBlur: { type: Function, optional: true }, diff --git a/addons/web/static/src/views/fields/ace/ace_field.js b/addons/web/static/src/views/fields/ace/ace_field.js index 846d50f9b97..731cea07a12 100644 --- a/addons/web/static/src/views/fields/ace/ace_field.js +++ b/addons/web/static/src/views/fields/ace/ace_field.js @@ -8,6 +8,7 @@ import { standardFieldProps } from "../standard_field_props"; import { CodeEditor } from "@web/core/code_editor/code_editor"; import { Component, useState } from "@odoo/owl"; import { useRecordObserver } from "@web/model/relational_model/utils"; +import { formatText } from "@web/views/fields/formatters"; export class AceField extends Component { static template = "web.AceField"; @@ -25,7 +26,7 @@ export class AceField extends Component { this.state = useState({}); useRecordObserver((record) => { - this.state.initialValue = record.data[this.props.name]; + this.state.initialValue = formatText(record.data[this.props.name]); }); this.isDirty = false; diff --git a/addons/web/static/src/views/fields/formatters.js b/addons/web/static/src/views/fields/formatters.js index 05d391a9a82..e82e351b4b0 100644 --- a/addons/web/static/src/views/fields/formatters.js +++ b/addons/web/static/src/views/fields/formatters.js @@ -483,6 +483,27 @@ export function formatSelection(value, options = {}) { return option ? option[1] : ""; } +/** + * Returns the value or an empty string if it's falsy. + * + * @param {string | false} value + * @returns {string} + */ +export function formatText(value) { + return value ? value.toString() : ""; +} + +/** + * Returns the value. + * Note that, this function is added to be coherent with the rest of the formatters. + * + * @param {html} value + * @returns {html} + */ +export function formatHtml(value) { + return value; +} + export function formatJson(value) { return (value && JSON.stringify(value)) || ""; } @@ -497,7 +518,7 @@ registry .add("float", formatFloat) .add("float_factor", formatFloatFactor) .add("float_time", formatFloatTime) - .add("html", (value) => value) + .add("html", formatHtml) .add("integer", formatInteger) .add("json", formatJson) .add("many2one", formatMany2one) @@ -510,4 +531,4 @@ registry .add("properties_definition", formatProperties) .add("reference", formatReference) .add("selection", formatSelection) - .add("text", (value) => value); + .add("text", formatText); diff --git a/addons/web/static/tests/core/code_editor_tests.js b/addons/web/static/tests/core/code_editor_tests.js index 0888984868c..5cb00ce0422 100644 --- a/addons/web/static/tests/core/code_editor_tests.js +++ b/addons/web/static/tests/core/code_editor_tests.js @@ -10,8 +10,11 @@ import { editInput, } from "@web/../tests/helpers/utils"; -import { Component, useState, xml } from "@odoo/owl"; +import { Component, markup, useState, xml } from "@odoo/owl"; import { CodeEditor } from "@web/core/code_editor/code_editor"; +import { registerCleanup } from "@web/../tests/helpers/cleanup"; +import { registry } from "@web/core/registry"; +import { errorService } from "@web/core/errors/error_service"; QUnit.module("Web Components", (hooks) => { QUnit.module("Code Editor"); @@ -80,6 +83,50 @@ QUnit.module("Web Components", (hooks) => { assert.containsOnce(target, ".ace_editor", "Code editor is rendered"); }); + QUnit.test("CodeEditor shouldn't accepts markup values", async (assert) => { + const _console = window.console; + window.console = Object.assign(Object.create(_console), { + warn(msg) { + assert.step(msg); + }, + }); + registerCleanup(() => { + window.console = _console; + }); + registry.category("services").add("error", errorService); + const handler = (ev) => { + assert.step(ev.reason.message); + // need to preventDefault to remove error from console (so python test pass) + ev.preventDefault(); + }; + window.addEventListener("unhandledrejection", handler); + registerCleanup(() => window.removeEventListener("unhandledrejection", handler)); + + patchWithCleanup(QUnit, { + onUnhandledRejection: () => {}, + }); + class Parent extends Component { + static components = { CodeEditor }; + static template = xml``; + } + class GrandParent extends Component { + static components = { Parent }; + static template = xml``; + setup() { + this.state = useState({ value: `
Some Text
` }); + } + } + + const codeEditor = await mount(GrandParent, target, { env }); + const textMarkup = markup`
Some Text
`; + codeEditor.state.value = textMarkup; + await nextTick(); // wait for the errorService to be called + assert.verifySteps([ + "[Owl] Unhandled error. Destroying the root component", + "Invalid props for component 'CodeEditor': 'value' is not valid", + ]); + }); + QUnit.test("onChange props called when code is edited", async (assert) => { class Parent extends Component { static components = { CodeEditor }; diff --git a/addons/web/static/tests/views/fields/ace_editor_field_tests.js b/addons/web/static/tests/views/fields/ace_editor_field_tests.js index ae75accc667..5b1575cf0d7 100644 --- a/addons/web/static/tests/views/fields/ace_editor_field_tests.js +++ b/addons/web/static/tests/views/fields/ace_editor_field_tests.js @@ -3,6 +3,8 @@ import { registry } from "@web/core/registry"; import { click, + clickSave, + editInput, getFixture, nextTick, triggerEvent, @@ -66,6 +68,57 @@ QUnit.module("Fields", (hooks) => { assert.ok(target.querySelector(".o_field_code").textContent.includes("yop")); }); + QUnit.test("AceEditorField on html fields works", async function (assert) { + serverData.models.partner.fields.htmlField = { + string: "HTML Field", + type: "html", + }; + serverData.models.partner.records.push({ + id: 3, + htmlField: "

My little HTML Test

", + }); + serverData.models.partner.onchanges = { htmlField: function () {} }; + await makeView({ + type: "form", + resModel: "partner", + resId: 3, + serverData, + arch: ` +
+ + + `, + mockRPC(route, args) { + if (args.method) { + assert.step(args.method); + if (args.method === "write") { + assert.deepEqual(args.args[1], { foo: "DEF" }); + } + if (args.method === "onchange") { + throw new Error("Should not call onchange, htmlField wasn't changed"); + } + } + }, + }); + + assert.ok("ace" in window, "the ace library should be loaded"); + assert.containsOnce( + target, + "div.ace_content", + "should have rendered something with ace editor" + ); + + assert.ok( + target.querySelector(".o_field_code").textContent.includes("My little HTML Test") + ); + + // Modify foo and save + await editInput(target, ".o_field_widget[name=foo] textarea", "DEF"); + await clickSave(target); + + assert.verifySteps(["get_views", "web_read", "write", "web_read"]); + }); + QUnit.test("AceEditorField doesn't crash when editing", async (assert) => { await makeView({ type: "form", diff --git a/addons/web/static/tests/views/fields/formatters_tests.js b/addons/web/static/tests/views/fields/formatters_tests.js index 97fcfc5a87e..43a3e27ac25 100644 --- a/addons/web/static/tests/views/fields/formatters_tests.js +++ b/addons/web/static/tests/views/fields/formatters_tests.js @@ -1,5 +1,6 @@ /** @odoo-module **/ +import { markup } from "@odoo/owl"; import { defaultLocalization } from "@web/../tests/helpers/mock_services"; import { patchWithCleanup } from "@web/../tests/helpers/utils"; import { currencies } from "@web/core/currency"; @@ -15,6 +16,7 @@ import { formatMonetary, formatPercentage, formatReference, + formatText, formatX2many, } from "@web/views/fields/formatters"; @@ -239,6 +241,16 @@ QUnit.module("Fields", (hooks) => { assert.strictEqual(formatMany2one([1, "M2O value"], { escape: true }), "M2O%20value"); }); + QUnit.test("formatText", function (assert) { + assert.strictEqual(formatText(false), ""); + assert.strictEqual(formatText("value"), "value"); + assert.strictEqual(formatText(1), "1"); + assert.strictEqual(formatText(1.5), "1.5"); + assert.strictEqual(formatText(markup("

This is a Test

")), "

This is a Test

"); + assert.strictEqual(formatText([1, 2, 3, 4, 5]), "1,2,3,4,5"); + assert.strictEqual(formatText({ a: 1, b: 2 }), "[object Object]"); + }); + QUnit.test("formatX2many", function (assert) { // Results are cast as strings since they're lazy translated. assert.strictEqual(String(formatX2many({ currentIds: [] })), "No records");