[FIX] website: restore the use of some snippets inside a TOC
Steps to reproduce the bug: - Drop a "Table of Content" snippet inside a page. - Drop a "Facebook" snippet inside the "Table of Content". -> The page is blocked. This may happen with other snippets than the facebook one (e.g. the image gallery or the countdown). A mechanism that interests us here is the mutation observer. Up to this commit its goal was to regenerate the navbar of the TOC when its DOM content changed. In practice, since [1], a "widgets_start_request" message was triggered up each time there was a change of the TOC content. In the case of the Facebook snippet, this behavior led to an infinite loop. Indeed, when the Facebook snippet was dropped inside a TOC, its content changed so a "widgets_start_request" was triggered up. This message led to the execution of the start function of the TOC but also of the start function of the Facebook snippet. Because there is a change of the DOM in this last function, the mutation observer triggered up a "widgets_start_request" message again that caused the program to loop forever. The goal of this commit is to regenerate the content of the navbar only when its content changes. To do so, the content of the navbar before the change of the DOM is compared with its content after the change of the DOM. If those two contents are different, it means that the navbar should be updated. Thanks to this commit, it is now possible to add dynamic snippets such as "Facebook" or "Countdown" inside the TOC. With this in hands, another bug appeared: - Drop a "Table of Content" snippet inside a page. - Drop a "Text-Image" snippet on the page but not inside the TOC. - Drop a "Countdown" snippet inside the "Text-Image". - Drag the entire "Text-Image" snippet with "Countdown" and drop it inside the TOC. -> The page is blocked. This bug comes from the fact that the drop of a snippet that stands on the page inside an other snippet leads to the destroy and the restart of the option of the outer snippet. However, in our case, the mutation observer was still listening to DOM changes even after the destroy of the snippet option. This led to parasite function call. This bug is fixed by disconnecting the mutation observer when the option is destroyed. Note that this is done in [2] as this was a problem in older versions as well. [1]: https://github.com/odoo/odoo/commit/a05f782871c61b2d58ca2fd4277cb7aed65a301d [2]: https://github.com/odoo/odoo/pull/112960 task-3122249 opw-3164969 opw-3173006 closes odoo/odoo#113070 X-original-commit: 9139b27ee9216f4b6111d0596f101d4927b493b2 Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
This commit is contained in:
@@ -9,6 +9,7 @@ options.registry.TableOfContent = options.Class.extend({
|
||||
*/
|
||||
start: function () {
|
||||
this.targetedElements = 'h1, h2';
|
||||
this.oldHeadingsEls = [];
|
||||
const $headings = this.$target.find(this.targetedElements);
|
||||
if ($headings.length > 0) {
|
||||
this._generateNav();
|
||||
@@ -77,10 +78,23 @@ options.registry.TableOfContent = options.Class.extend({
|
||||
*/
|
||||
_generateNav: function (ev) {
|
||||
this.options.wysiwyg && this.options.wysiwyg.odooEditor.unbreakableStepUnactive();
|
||||
const $headings = this.$target.find(this.targetedElements);
|
||||
const areHeadingsEqual = this.oldHeadingsEls.length === $headings.length
|
||||
&& this.oldHeadingsEls.every((el, i) => el.isEqualNode($headings[i]));
|
||||
if (areHeadingsEqual) {
|
||||
// If the content of the navbar before the change of the DOM is
|
||||
// equal to the content of the navbar after the change of the DOM,
|
||||
// then there is no need to regenerate the navbar.
|
||||
// This is especially important as to regenerate it, we also have
|
||||
// to restart scrollSpy, which is done by restarting widgets. But
|
||||
// restarting all widgets inside the ToC would certainly lead to
|
||||
// DOM changes... which would then regenerate the navbar and lead to
|
||||
// an infinite loop.
|
||||
return;
|
||||
}
|
||||
// We dispose the scrollSpy because the navbar will be updated.
|
||||
this._disposeScrollSpy();
|
||||
const $nav = this.$target.find('.s_table_of_content_navbar');
|
||||
const $headings = this.$target.find(this.targetedElements);
|
||||
$nav.empty();
|
||||
_.each($headings, el => {
|
||||
const $el = $(el);
|
||||
@@ -94,6 +108,7 @@ options.registry.TableOfContent = options.Class.extend({
|
||||
});
|
||||
const exception = (tocEl) => !tocEl.querySelector('.s_table_of_content_navbar a');
|
||||
this._activateScrollSpy(exception);
|
||||
this.oldHeadingsEls = [...($headings.clone())];
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user