From a0f3ed514c52053b2ad83f26543b227878054a2f Mon Sep 17 00:00:00 2001 From: "Louis (loco)" Date: Mon, 18 Sep 2023 14:52:31 +0200 Subject: [PATCH] [IMP] website: enable to use special characters in a form Before [1], the usage of some characters as field label inside a form led to a traceback. As explained in [1], the problem came from the selector given to the `querySelector()` method. To solve the problem, [1] encoded the problematic characters. The problem is that the backslash character was not taken into account and still led to a traceback if it was used as a field label. [2] solved the problem by adding this special character in the list of the characters to encode. The problem of encoding problematic terms manually is that others (still unknown) can still be problematic and would have to be encoded in the future. To solve the problem, it has been decided to escape the problematic selectors with the `CSS.escape()` method, which is the right method to promote when adding anything unknown inside a selector anyway. [1]: https://github.com/odoo/odoo/commit/ccaf4f1e52f10308ef507059316732c02ad6e6e4 [2]: https://github.com/odoo/odoo/commit/03f230a77c0d38b46944d45b4991f48e45b275dd task-3510450 Part-of: odoo/odoo#135797 --- .../static/src/snippets/s_website_form/000.js | 2 +- .../src/snippets/s_website_form/options.js | 28 ++++++++++++++----- .../static/tests/tours/website_form_editor.js | 1 + 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/addons/website/static/src/snippets/s_website_form/000.js b/addons/website/static/src/snippets/s_website_form/000.js index f8edc9584cc93..cb4945242fe90 100644 --- a/addons/website/static/src/snippets/s_website_form/000.js +++ b/addons/website/static/src/snippets/s_website_form/000.js @@ -142,7 +142,7 @@ odoo.define('website.s_website_form', function (require) { // the values to submit() for these fields but this could break // customizations that use the current behavior as a feature. for (const name of fieldNames) { - const fieldEl = this.$target[0].querySelector(`[name="${name}"]`); + const fieldEl = this.$target[0].querySelector(`[name="${CSS.escape(name)}"]`); // In general, we want the data-for and prefill values to // take priority over set default values. The 'email_to' diff --git a/addons/website/static/src/snippets/s_website_form/options.js b/addons/website/static/src/snippets/s_website_form/options.js index b16f55d3545d7..c1908d5018df3 100644 --- a/addons/website/static/src/snippets/s_website_form/options.js +++ b/addons/website/static/src/snippets/s_website_form/options.js @@ -123,9 +123,23 @@ const FormEditor = options.Class.extend({ * a `querySelector`. * * @param {string} name + * @returns {string} */ _getQuotesEncodedName(name) { + // Browsers seem to be encoding the double quotation mark character as + // `%22` (URI encoded version) when used inside an input's name. It is + // actually quite weird as a sent `` + // will actually be received as `Hello %22world%22 %22` on the server, + // making it impossible to know which is actually a real double + // quotation mark and not the "%22" string. Values do not have this + // problem: `Hello "world" %22` would be received as-is on the server. + // In the future, we should consider not using label values as input + // names anyway; the idea was bad in the first place. We should probably + // assign random field names (as we do for IDs) and send a mapping + // with the labels, as values (TODO ?). return name.replaceAll(/"/g, character => `"`) + // TODO: in master only keep the conversion of the double quotation + // mark character as selectors are now escaped when doing a search. .replaceAll(/'/g, character => `'`) .replaceAll(/`/g, character => `‘`) .replaceAll("\\", character => `\`); @@ -964,11 +978,11 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ */ onRemove() { const fieldName = this.$target[0].querySelector('.s_website_form_input').name; - const isMultipleField = this.formEl.querySelectorAll(`.s_website_form_input[name="${fieldName}"]`).length > 1; + const isMultipleField = this.formEl.querySelectorAll(`.s_website_form_input[name="${CSS.escape(fieldName)}"]`).length > 1; if (isMultipleField) { return; } - const dependentFieldContainerEl = this.formEl.querySelectorAll(`[data-visibility-dependency="${fieldName}"]`); + const dependentFieldContainerEl = this.formEl.querySelectorAll(`[data-visibility-dependency="${CSS.escape(fieldName)}"]`); for (const fieldContainerEl of dependentFieldContainerEl) { this._deleteConditionalVisibility(fieldContainerEl); } @@ -1030,7 +1044,7 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ inputEls.forEach(el => el.name = value); // Synchronize the fields whose visibility depends on this field - const dependentEls = this.formEl.querySelectorAll(`.s_website_form_field[data-visibility-dependency="${previousInputName}"]`); + const dependentEls = this.formEl.querySelectorAll(`.s_website_form_field[data-visibility-dependency="${CSS.escape(previousInputName)}"]`); for (const dependentEl of dependentEls) { if (!previewMode && this._findCircular(this.$target[0], dependentEl)) { // For all the fields whose visibility depends on this @@ -1119,7 +1133,7 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ const input = inputEls[i]; if (newValuesText[i] && input.value && !newValuesText.includes(input.value)) { for (const dependentEl of this.formEl.querySelectorAll( - `[data-visibility-condition="${input.value}"][data-visibility-dependency="${inputName}"]`)) { + `[data-visibility-condition="${CSS.escape(input.value)}"][data-visibility-dependency="${CSS.escape(inputName)}"]`)) { dependentEl.dataset.visibilityCondition = newValuesText[i]; } break; @@ -1275,7 +1289,7 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ */ _getDependencyEl(fieldEl = this.$target[0]) { const dependencyName = fieldEl.dataset.visibilityDependency; - return this.formEl.querySelector(`.s_website_form_input[name="${dependencyName}"]`); + return this.formEl.querySelector(`.s_website_form_input[name="${CSS.escape(dependencyName)}"]`); }, /** * @param {HTMLElement} dependentFieldEl @@ -1293,7 +1307,7 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ // Get all the fields that have the same label as the dependent // field. let dependentFieldEls = Array.from(this.formEl - .querySelectorAll(`.s_website_form_input[name="${dependentFieldName}"]`)) + .querySelectorAll(`.s_website_form_input[name="${CSS.escape(dependentFieldName)}"]`)) .map((el) => el.closest(".s_website_form_field")); // Remove the duplicated fields. This could happen if the field has // multiple inputs ("Multiple Checkboxes" for example.) @@ -1475,7 +1489,7 @@ options.registry.WebsiteFieldEditor = FieldEditor.extend({ if (hasConditionalVisibility) { this.$target[0].classList.add('s_website_form_field_hidden_if', 'd-none'); } - const dependentFieldEls = this.formEl.querySelectorAll(`.s_website_form_field[data-visibility-dependency="${previousName}"]`); + const dependentFieldEls = this.formEl.querySelectorAll(`.s_website_form_field[data-visibility-dependency="${CSS.escape(previousName)}"]`); const newFormInputEl = this.$target[0].querySelector('.s_website_form_input'); const newName = newFormInputEl.name; const newType = newFormInputEl.type; diff --git a/addons/website/static/tests/tours/website_form_editor.js b/addons/website/static/tests/tours/website_form_editor.js index 9bc542e23e38c..cb93883f12fd3 100644 --- a/addons/website/static/tests/tours/website_form_editor.js +++ b/addons/website/static/tests/tours/website_form_editor.js @@ -29,6 +29,7 @@ odoo.define('website.tour.form_editor', function (require) { }); } + // TODO: in master only keep the conversion of the double quotes character. // Replace all `"` character by `"`, all `'` character by `'` and // all "`" character by `‘`. const getQuotesEncodedName = function (name) {