From a6c720c4171309f17cbb8e8e04fa46b3e3eb70f3 Mon Sep 17 00:00:00 2001 From: Rodolpho Lima Date: Fri, 27 Oct 2023 16:03:27 +0000 Subject: [PATCH] [FIX] mass_mailing: switch to basic theme record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steps: - Create a draft mailing based on a theme that has the snippets menu (the "Event Promo" theme, for instance). - Create another draft mailing, this time based on the basic theme ("Plain Text"). - Switch from the former to the latter using the form view pager. The editor for the basic theme record is not supposed to have a sidebar (snippets menu), as it uses the floating toolbar instead. But when switching between records like described above, the basic theme gets replaced by the theme of the preceding record, and the snippets menu is added to the UI. This happens because [1] patched the `setValue` function to handle the case where the element for dropping snippets (.o_mail_wrapper_td) is missing, in particular if it gets removed via the codeview. Such fix most likely had in mind the scenario where the codeview is toggled on and off, but not the call to `setValue` that also happens when switching between records via the form view pager. As a result, the html content of a record based on the basic theme, in which such "dropzone" is absent, is added as a child of the previous record's .o_mail_wrapper_td element. This is certainly not the desired behavior, as it ends up overwriting the basic theme by the one from the previous record (the theme is defined by a class in the .o_layout div, which is a parent of the .o_mail_wrapper_td element). This commit, besides fixing the described issue, takes the opportunity to add a comment with the presumed reasoning behind commit [1] and extends the test tour in order to prevent regressions. [1]: https://github.com/odoo/odoo/commit/9be2cb538f937645be4650af1031c8a1445bb48b task-3573951 closes odoo/odoo#151065 X-original-commit: ed4727d922107745482dbbeb068d4bd6eba788dd Signed-off-by: Nicolas Bayet (nby) Signed-off-by: Geelen Sébastien (sge) Signed-off-by: Antoine Guenet (age) --- .../static/src/js/mass_mailing_html_field.js | 22 +++++- .../static/src/js/mass_mailing_wysiwyg.js | 12 --- .../tests/tours/mailing_editor_theme.js | 75 +++++++++++++++---- 3 files changed, 80 insertions(+), 29 deletions(-) diff --git a/addons/mass_mailing/static/src/js/mass_mailing_html_field.js b/addons/mass_mailing/static/src/js/mass_mailing_html_field.js index b71b8dc753f..d500a383f28 100644 --- a/addons/mass_mailing/static/src/js/mass_mailing_html_field.js +++ b/addons/mass_mailing/static/src/js/mass_mailing_html_field.js @@ -104,7 +104,7 @@ export class MassMailingHtmlField extends HtmlField { this._pendingCommitChanges = (async () => { const codeViewEl = this._getCodeViewEl(); if (codeViewEl) { - this.wysiwyg.setValue(codeViewEl.value); + this.wysiwyg.setValue(this._getCodeViewValue(codeViewEl)); } if (this.wysiwyg.$iframeBody.find('.o_basic_theme').length) { @@ -248,7 +248,7 @@ export class MassMailingHtmlField extends HtmlField { this.wysiwyg.odooEditor.observerActive(); if ($codeview.hasClass('d-none')) { - this.wysiwyg.setValue($codeview.val()); + this.wysiwyg.setValue(this._getCodeViewValue($codeview[0])); this.wysiwyg.odooEditor.sanitize(); this.wysiwyg.odooEditor.historyStep(true); } else { @@ -444,6 +444,24 @@ export class MassMailingHtmlField extends HtmlField { this.wysiwyg.$iframe.contents().find('textarea.o_codeview')[0]; return codeView && !codeView.classList.contains('d-none') && codeView; } + /** + * The .o_mail_wrapper_td element is where snippets can be dropped into. + * This getter wraps the codeview value in such element in case it got + * removed during edition in the codeview, in order to preserve the snippets + * dropping functionality. + */ + _getCodeViewValue(codeViewEl) { + const editable = this.wysiwyg.$editable[0]; + const initialDropZone = editable.querySelector('.o_mail_wrapper_td'); + if (initialDropZone) { + const parsedHtml = new DOMParser().parseFromString(codeViewEl.value, "text/html"); + if (!parsedHtml.querySelector('.o_mail_wrapper_td')) { + initialDropZone.replaceChildren(...parsedHtml.body.childNodes); + return editable.innerHTML; + } + } + return codeViewEl.value; + } /** * This method will take the model in argument and will hide all mailing template * in the mass mailing widget that do not belong to this model. diff --git a/addons/mass_mailing/static/src/js/mass_mailing_wysiwyg.js b/addons/mass_mailing/static/src/js/mass_mailing_wysiwyg.js index a63b5232193..23185dd16db 100644 --- a/addons/mass_mailing/static/src/js/mass_mailing_wysiwyg.js +++ b/addons/mass_mailing/static/src/js/mass_mailing_wysiwyg.js @@ -82,18 +82,6 @@ export class MassMailingWysiwyg extends Wysiwyg { } } - /** - * @override - */ - setValue(currentValue) { - const initialDropZone = this.$editable[0].querySelector('.o_mail_wrapper_td'); - const parsedHtml = new DOMParser().parseFromString(currentValue, "text/html"); - if (initialDropZone && !parsedHtml.querySelector('.o_mail_wrapper_td')) { - initialDropZone.replaceChildren(...parsedHtml.body.childNodes); - } else { - super.setValue(...arguments); - } - } //-------------------------------------------------------------------------- // Private //-------------------------------------------------------------------------- diff --git a/addons/mass_mailing/static/tests/tours/mailing_editor_theme.js b/addons/mass_mailing/static/tests/tours/mailing_editor_theme.js index c35de1e29c5..c3c11910aed 100644 --- a/addons/mass_mailing/static/tests/tours/mailing_editor_theme.js +++ b/addons/mass_mailing/static/tests/tours/mailing_editor_theme.js @@ -18,7 +18,21 @@ registry.category("web_tour.tours").add('mailing_editor_theme', { trigger: 'button.o_list_button_add', }, { - content: "Wait for the theme selector to load and pick the basic theme.", + content: "Fill in Subject", + trigger: '#subject_0', + run: 'text Test Basic Theme', + }, + { + content: "Fill in Mailing list", + trigger: '#contact_list_ids_0', + run: 'text Newsletter', + }, + { + content: "Pick 'Newsletter' option", + trigger: '.o_input_dropdown a:contains(Newsletter)', + }, + { + content: "Pick the basic theme", trigger: 'iframe #basic', extra_trigger: 'iframe .o_mail_theme_selector_new', }, @@ -27,6 +41,47 @@ registry.category("web_tour.tours").add('mailing_editor_theme', { trigger: 'iframe html:has(#oe_snippets.d-none)', run: () => null, // no click, just check }, + { + content: "Click on the New button to create another mailing", + trigger: 'button.o_form_button_create', + }, + { + content: "Fill in Subject", + trigger: '#subject_0', + extra_trigger: 'iframe .o_mail_theme_selector_new', + run: 'text Test Newsletter Theme', + }, + { + content: "Fill in Mailing list", + trigger: '#contact_list_ids_0', + run: 'text Newsletter', + }, + { + content: "Pick 'Newsletter' option", + trigger: '.o_input_dropdown a:contains(Newsletter)', + }, + { + content: "Pick the newsletter theme", + trigger: 'iframe #newsletter', + }, + { + content: "Make sure the snippets menu is displayed", + trigger: 'iframe #oe_snippets', + run: () => null, // no click, just check + }, + { + content: 'Save form', + trigger: '.o_form_button_save', + }, + { + content: 'Go back to previous mailing', + trigger: 'button.o_pager_previous', + }, + { + content: "Make sure the snippets menu is hidden", + trigger: 'iframe html:has(#oe_snippets.d-none)', + run: () => null, + }, { content: "Add some content to be selected afterwards", trigger: 'iframe p', @@ -58,26 +113,16 @@ registry.category("web_tour.tours").add('mailing_editor_theme', { run: () => null, }, { - content: "Fill in Subject", - trigger: '[name="subject"] input', - run: 'text TestFromTour', + content: 'Save changes', + trigger: '.o_form_button_save', }, { - content: "Fill in Mailing list", - trigger: '[name="contact_list_ids"] input', - run: 'text Newsletter', - }, - { - content: "Pick 'Newsletter' option", - trigger: '.o_input_dropdown a:contains(Newsletter)', - }, - { - content: "Save and go to 'Mailings' list view", + content: "Go to 'Mailings' list view", trigger: '.breadcrumb a:contains(Mailings)' }, { content: "Open newly created mailing", - trigger: 'td:contains(TestFromTour)', + trigger: 'td:contains("Test Basic Theme")', }, { content: "Make sure the snippets menu is hidden",