Skip to content

Commit

Permalink
[IMP] website: enable to use special characters in a form
Browse files Browse the repository at this point in the history
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]: odoo@ccaf4f1
[2]: odoo@03f230a

task-3510450

Part-of: odoo#135797
  • Loading branch information
loco-odoo authored and qsm-odoo committed Apr 25, 2024
1 parent 50fae9e commit a0f3ed5
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
2 changes: 1 addition & 1 deletion addons/website/static/src/snippets/s_website_form/000.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
28 changes: 21 additions & 7 deletions addons/website/static/src/snippets/s_website_form/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<input name='Hello "world" %22'/>`
// 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 => `&quot;`)
// 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 => `&apos;`)
.replaceAll(/`/g, character => `&lsquo;`)
.replaceAll("\\", character => `&bsol;`);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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.)
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions addons/website/static/tests/tours/website_form_editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 `&quot;`, all `'` character by `&apos;` and
// all "`" character by `&lsquo;`.
const getQuotesEncodedName = function (name) {
Expand Down

0 comments on commit a0f3ed5

Please sign in to comment.