[FIX] website: destroy widgets before instantiating the wysiwyg

Steps to reproduce the problem:
- Add a "Countdown" snippet on the website.
- Save.
- Enter in edit mode again.

-> Problem: the `destroy()` of the `CountdownWidget` public widget comes
from a `widgets_start_request` that is triggered when the
`WysiwygAdapterComponent` has been mounted and patched (see
[useEffect documentation]). The problem is that, as the widgets are only
destroyed at this moment, it is possible that a widget could replace an
element that has been modified during the instantiation of the
`WysiwygAdapterComponent`. For example:
- At the `onWillStart()` hook of the `WysiwygAdapterComponent`, the
`o_editable` is added on some elements.
- A widget could then replace one of these elements.
- The widget is destroyed after the rendering of the
`WysiwygAdapterComponent`.

-> The `o_editable` class is not on the element anymore.

The goal of this commit is to, as before [1], destroy the existing
widgets before instantiating the `wysiwyg`.

This commit also adds a test to ensure that the order of the calls to
the widgets and wysiwyg lifecycle methods is coherent.

Related to runbot-28700

[1]: https://github.com/odoo/odoo/commit/a3e34512bf229d5d55f9e9d9eb0a9f7211a5a826
[useEffect documentation]: https://github.com/odoo/owl/blob/master/doc/reference/hooks.md#useeffect

closes odoo/odoo#151411

X-original-commit: 723c0dfeb7d39ca2522e65cf021664aacfc7099c
Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
This commit is contained in:
Louis (loco)
2024-01-29 23:16:16 +00:00
committed by qsm-odoo
parent c70278cade
commit 59813f0cff
7 changed files with 195 additions and 2 deletions
+3 -1
View File
@@ -174,7 +174,9 @@
('prepend', 'website/static/src/scss/secondary_variables.scss'),
],
'web.assets_tests': [
'website/static/tests/tour_utils/**/*',
'website/static/tests/tour_utils/focus_blur_snippets_options.js',
'website/static/tests/tour_utils/website_preview_test.js',
'website/static/tests/tour_utils/widget_lifecycle_dep_widget.js',
'website/static/tests/tours/**/*',
],
'web.assets_backend': [
@@ -92,6 +92,15 @@ export class WysiwygAdapterComponent extends Wysiwyg {
useHotkey('control+k', () => {});
onWillStart(() => {
// Destroy the widgets before instantiating the wysiwyg.
// grep: RESTART_WIDGETS_EDIT_MODE
// TODO ideally this should be done as close as the restart as
// as possible to avoid long flickering when entering edit mode. At
// moment some RPC are awaited before the restart so it is not
// ideal. But this has to be done before adding o_editable classes
// in the DOM. To review once everything is OWLified.
this._websiteRootEvent("widgets_stop_request");
const pageOptionEls = this.websiteService.pageDocument.querySelectorAll('.o_page_option_data');
for (const pageOptionEl of pageOptionEls) {
const optionName = pageOptionEl.name;
@@ -199,6 +208,7 @@ export class WysiwygAdapterComponent extends Wysiwyg {
}
// The jquery instance inside the iframe needs to be aware of the wysiwyg.
this.websiteService.contentWindow.$('#wrapwrap').data('wysiwyg', this);
// grep: RESTART_WIDGETS_EDIT_MODE
await new Promise((resolve, reject) => this._websiteRootEvent('widgets_start_request', {
editableMode: true,
onSuccess: resolve,
@@ -1,7 +1,7 @@
/** @odoo-module **/
odoo.loader.bus.addEventListener("module-started", (e) => {
if (e.detail.moduleName === "@web_editor/js/editor/snippets.options"){
if (e.detail.moduleName === "@web_editor/js/editor/snippets.options") {
const options = e.detail.module[Symbol.for("default")];
const FocusBlur = options.Class.extend({
onFocus() {
@@ -0,0 +1,42 @@
/** @odoo-module **/
odoo.loader.bus.addEventListener("module-started", (e) => {
if (e.detail.moduleName !== "@web/legacy/js/public/public_widget") {
return;
}
const publicWidget = e.detail.module[Symbol.for("default")];
const localStorageKey = 'widgetAndWysiwygLifecycle';
if (!window.localStorage.getItem(localStorageKey)) {
window.localStorage.setItem(localStorageKey, '[]');
}
function addLifecycleStep(step) {
const oldValue = window.localStorage.getItem(localStorageKey);
const newValue = JSON.stringify(JSON.parse(oldValue).concat(step));
window.localStorage.setItem(localStorageKey, newValue);
}
publicWidget.registry.CountdownPatch = publicWidget.Widget.extend({
selector: ".s_countdown",
disabledInEditableMode: false,
/**
* @override
*/
async start() {
addLifecycleStep('widgetStart');
await this._super(...arguments);
this.el.classList.add("public_widget_started");
},
/**
* @override
*/
destroy() {
this.el.classList.remove("public_widget_started");
addLifecycleStep('widgetStop');
this._super(...arguments);
},
});
});
@@ -0,0 +1,62 @@
/** @odoo-module **/
import { onMounted, onWillRender } from "@odoo/owl";
import { patch } from "@web/core/utils/patch";
odoo.loader.bus.addEventListener("module-started", (e) => {
if (e.detail.moduleName !== "@website/components/wysiwyg_adapter/wysiwyg_adapter") {
return;
}
const { WysiwygAdapterComponent } = e.detail.module;
// Duplicated from "@website/../tests/tour_utils/widget_lifecycle_dep_widget"
// Cannot be imported for some reason, probably because of this being lazy
// loaded?
function addLifecycleStep(step) {
const localStorageKey = 'widgetAndWysiwygLifecycle';
const oldValue = window.localStorage.getItem(localStorageKey);
const newValue = JSON.stringify(JSON.parse(oldValue).concat(step));
window.localStorage.setItem(localStorageKey, newValue);
}
patch(WysiwygAdapterComponent.prototype, {
/**
* @override
*/
setup() {
super.setup(...arguments);
// The Wysiwyg class is very messy at the moment: it touches the DOM in
// onWillStart hook, mixes OWL & legacy widget, etc. Here we want to
// test "when the Wysiwyg is started"... for now we will settle on
// testing "the first time it touches the DOM", relying on it to be when
// he reads what "editable elements" are for the first time, thanks to
// the `editableElements` method.
// TODO to be reviewed (probably as soon as we touch the editable DOM
// only when considered initialized).
onWillRender(() => {
this.__consideredInitialized = true;
});
onMounted(() => {
addLifecycleStep('wysiwygStarted');
});
},
/**
* @override
*/
editableElements() {
if (!this.__consideredInitialized) {
addLifecycleStep('wysiwygStart');
}
return super.editableElements(...arguments);
},
/**
* @override
*/
destroy() {
addLifecycleStep('wysiwygStop');
super.destroy(...arguments);
},
});
});
@@ -0,0 +1,69 @@
/** @odoo-module **/
import wTourUtils from '@website/js/tours/tour_utils';
// Note: cannot import @website/../tests/tour_utils/widget_lifecycle_dep_widget
// here because that module requires web.public.widget which is not available
// in the backend, where this tour definition is loaded. Easier to duplicate
// that key for now rather than create a whole file to handle this localStorage
// key only.
const localStorageKey = 'widgetAndWysiwygLifecycle';
wTourUtils.registerWebsitePreviewTour("widget_lifecycle", {
test: true,
url: "/",
edition: true,
}, () => [
wTourUtils.dragNDrop({
id: "s_countdown",
name: "Countdown",
}),
{
content: "Wait for the widget to be started and empty the widgetAndWysiwygLifecycle list",
trigger: "iframe .s_countdown.public_widget_started",
run: () => {
// Start recording the calls to the "start" and "destroy" method of
// the widget and the wysiwyg.
window.localStorage.setItem(localStorageKey, '[]');
},
},
...wTourUtils.clickOnSave(),
{
content: "Wait for the widget to be started",
trigger: "iframe .s_countdown.public_widget_started",
run: () => {}, // It's a check
},
...wTourUtils.clickOnEditAndWaitEditMode(),
{
content: "Wait for the widget to be started and check the order of the lifecycle method call of the widget and the wysiwyg",
trigger: "iframe .s_countdown.public_widget_started",
run: () => {
const result = JSON.parse(window.localStorage.widgetAndWysiwygLifecycle);
const expected = ["widgetStop", "wysiwygStop", "widgetStart",
"widgetStop", "wysiwygStart", "wysiwygStarted", "widgetStart",
];
const alternative = ["widgetStop", "widgetStart", "wysiwygStop",
"widgetStop", "wysiwygStart", "wysiwygStarted", "widgetStart",
];
const resultIsEqualTo = (arr) => {
return arr.length === result.length
&& arr.every((item, i) => item === result[i]);
};
if (!(resultIsEqualTo(expected) || resultIsEqualTo(alternative))) {
// The "destroy" method of the wysiwyg is called two times when
// leaving the edit mode: the first one comes explicitly from
// the "leaveEditMode" of the "wysiwyg_adapter". The second
// comes from the OWL mechanism as the wysiwyg is not present in
// the DOM when the page is reloaded. Because it is not
// guaranteed that this last call happens before the start of
// the widget at the page reload, two sequences are acceptable
// as a result.
console.error(`
Expected: ${expected.toString()}
Or: ${alternative.toString()}
Result: ${result.toString()}
`);
}
},
},
]);
+8
View File
@@ -504,3 +504,11 @@ class TestUi(odoo.tests.HttpCase):
website.menu_id.child_id[1:].unlink()
self.start_tour('/', 'website_no_dirty_page', login='admin')
def test_widget_lifecycle(self):
self.env['ir.asset'].create({
'name': 'wysiwyg_patch_start_and_destroy',
'bundle': 'website.assets_wysiwyg',
'path': 'website/static/tests/tour_utils/widget_lifecycle_patch_wysiwyg.js',
})
self.start_tour(self.env['website'].get_client_action_url('/'), 'widget_lifecycle', login='admin')