From 2487d8d27800cd77cd3a8f1d75488e60fb963672 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Wed, 20 Dec 2023 12:00:15 +0100 Subject: [PATCH] [FIX] web_editor, *: restore change checks on link tooltip open/close *: website This basically reverts the parts of [1] that are the known cause of race conditions in runbot tests (in particular, the link tools one: `test_15_website_link_tools`). But more importantly, this just did not make sense to stop observing changes for async period of time, at least unless there is no alternative given the state of the related code. Commit [1] test-hack was actually adapted with [2] which probably made it even more unreliable and hacky. On top of that, this commit is of course trying to restore whatever purpose this was made for ("dirty" flag addition, the history steps recording, etc). Although fixing those the most robust way would probably require a deeper refactoring in master and other stable fixes. Note: in this forward-ported version, the test is still disabled on runbot and broken for other reasons. Those will be fixed later. [1]: https://github.com/odoo/odoo/commit/8cab404dce3a65c0bdd635a93682ee14282a3ebb [2]: https://github.com/odoo/odoo/commit/99b50d18e220aedf14de806f4bf1b2d35c32de35 runbot-18747 closes odoo/odoo#148728 X-original-commit: 003422a73f0b5fb5540fbf2bd95a26a6dd663530 Signed-off-by: Romain Derie (rde) Signed-off-by: Quentin Smetz (qsm) --- .../src/js/wysiwyg/widgets/link_popover_widget.js | 11 ++--------- .../web_editor/static/src/js/wysiwyg/wysiwyg.js | 15 +++++++++++++++ addons/website/static/tests/tours/link_tools.js | 10 ---------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js b/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js index e0cefe2d649..cba0d63e66b 100644 --- a/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js +++ b/addons/web_editor/static/src/js/wysiwyg/widgets/link_popover_widget.js @@ -15,7 +15,7 @@ export class LinkPopoverWidget { } const popoverWidget = new this(params); params.wysiwyg?.odooEditor.observerUnactive('LinkPopoverWidget'); - popoverWidget.start(); + popoverWidget.start(); // This is not async params.wysiwyg?.odooEditor.observerActive('LinkPopoverWidget'); return popoverWidget; }; @@ -100,19 +100,13 @@ export class LinkPopoverWidget { container: this.container, }) .on('show.bs.popover.link_popover', () => { - this.wysiwyg.odooEditor.observerUnactive('show.bs.popover'); this._loadAsyncLinkPreview(); popoverShown = true; }) - .on('inserted.bs.popover', () => { - this.wysiwyg.odooEditor.observerActive('show.bs.popover'); - }) .on('hide.bs.popover.link_popover', () => { - this.wysiwyg.odooEditor.observerUnactive('hide.bs.popover'); popoverShown = false; }) .on('hidden.bs.popover.link_popover', () => { - this.wysiwyg.odooEditor.observerActive('hide.bs.popover'); for (const tooltip of tooltips) { tooltip.hide(); } @@ -123,7 +117,6 @@ export class LinkPopoverWidget { }) .popover('show'); - this.popover = Popover.getInstance(this.target); this.$target.on('mousedown.link_popover', (e) => { if (!popoverShown) { @@ -151,7 +144,7 @@ export class LinkPopoverWidget { this.popover.hide(); } } - } + }; $(document).on('mouseup.link_popover', onClickDocument); if (document !== this.wysiwyg.odooEditor.document) { $(this.wysiwyg.odooEditor.document).on('mouseup.link_popover', onClickDocument); diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 37f32a7b0c0..1c01c6b1356 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -446,7 +446,22 @@ export class Wysiwyg extends Component { }, filterMutationRecords: (records) => { return records.filter((record) => { + if (record.type === 'attributes' + && record.attributeName === 'aria-describedby') { + const value = (record.oldValue || record.target.getAttribute(record.attributeName)); + if (value && value.startsWith('popover')) { + // TODO maybe we should just always return false at + // this point: never considering the + // aria-describedby attribute for any tooltip? + const popoverData = Popover.getInstance(record.target); + return !popoverData + || popoverData.tip.id !== value + || !popoverData.tip.classList.contains('o_edit_menu_popover'); + } + } return !( + // TODO should probably not check o_header_standard + // here, since it is a website class ? (record.target.classList && record.target.classList.contains('o_header_standard')) || (record.type === 'attributes' && record.attributeName === 'data-last-history-steps') ); diff --git a/addons/website/static/tests/tours/link_tools.js b/addons/website/static/tests/tours/link_tools.js index 0b2cb319b3c..56de6387b5d 100644 --- a/addons/website/static/tests/tours/link_tools.js +++ b/addons/website/static/tests/tours/link_tools.js @@ -94,16 +94,6 @@ wTourUtils.registerWebsitePreviewTour('link_tools', { content: "The new link content should be odoo website and url odoo.be", trigger: '#toolbar:not(.oe-floating) .dropdown:has([name="link_style_color"]) > button', }, - { - // When doing automated testing, the link popover takes time to - // hide. While hidding, the editor observer is unactive in order to - // prevent the popover mutation to be recorded. In a manual - // scenario, the popover has plenty of time to be hidden and the - // obsever would be re-activated in time. As this problem arise only - // in test, we make sure the popover is hidden - trigger: 'iframe html:not(:has(.popover))', - run: () => null, // it's a check - }, { content: "Click on the secondary style button.", trigger: '#toolbar:not(.oe-floating) we-button[data-value="secondary"]',