From b910f3405db9ab88b2da1b839a7a2b5e26a2ecf5 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Wed, 28 Sep 2022 19:11:17 +0000 Subject: [PATCH] [FIX] web, web_editor: fix utils wherever the main page scroll is With [1], the frontend main scrollbar was moved to the #wrapwrap element and with that change came many scroll utils and code adaptation. The goal was for the code to be generic but after multiple bug fixes, only the case of #wrapwrap being the element which scrolls (this stable version's standard case) was actually working. As the scroll is being moved back out the #wrapwrap in master (see [2]), those non-properly working generic features were found. This commit solves the stable utils in preparation for that master merge. Indeed even if the standard 14.0 case was not impacted by those faulty utils, they were still wrong and could impact users migrated from 13.0 and earlier. Note: some adaptation actually handles the case of multi-documents in the page (like triggering a scroll in an iframe from out-of-the-iframe JS code). This is not needed here in 14.0 but will be in the forward- ported version in master for the 'website-in-backend' features merged at [3]. This actually includes some (parts of) fixes that were done in the wrong version, like [4]. [1]: https://github.com/odoo/odoo/commit/4e7be69825163c0a0ff41c882a196fc7f3158fb3 [2]: https://github.com/odoo/odoo/pull/98429 [3]: https://github.com/odoo/odoo/commit/31cc10b91dc7762e23b4bde9b945be0c4ce3fe3b [4]: https://github.com/odoo/odoo/commit/f240a42ab501fe37b52f92957c144536a9faf09d X-original-commit: ffc19547c8da2ef7fee8e2ac743ab99a607dcf90 Part-of: odoo/odoo#101517 --- addons/web/static/src/legacy/js/core/dom.js | 4 +++- addons/web/static/src/legacy/js/libs/jquery.js | 18 +++++++++++++++--- .../static/src/js/editor/snippets.editor.js | 10 +++++++--- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/addons/web/static/src/legacy/js/core/dom.js b/addons/web/static/src/legacy/js/core/dom.js index bfadd57bdd4..d1936859653 100644 --- a/addons/web/static/src/legacy/js/core/dom.js +++ b/addons/web/static/src/legacy/js/core/dom.js @@ -561,7 +561,9 @@ var dom = { offsetTop = $el.offset().top; el.classList.add('d-none'); } - let elPosition = $scrollable[0].scrollTop + (offsetTop - $scrollable.offset().top); + const isDocScrollingEl = $scrollable.is(el.ownerDocument.scrollingElement); + let elPosition = offsetTop + - ($scrollable.offset().top - (isDocScrollingEl ? 0 : $scrollable[0].scrollTop)); if (!isInOneDocument && $iframe.length) { elPosition += $iframe.offset().top; } diff --git a/addons/web/static/src/legacy/js/libs/jquery.js b/addons/web/static/src/legacy/js/libs/jquery.js index 93404f98994..79ff64f298b 100644 --- a/addons/web/static/src/legacy/js/libs/jquery.js +++ b/addons/web/static/src/legacy/js/libs/jquery.js @@ -123,13 +123,21 @@ $.fn.extend({ }); }, /** + * @todo Should really be converted to no jQuery and probably even removed + * from jQuery utilities in master * @return {jQuery} */ closestScrollable() { + const document = this.length ? this[0].ownerDocument : window.document; + let $el = this; while ($el[0] !== document.scrollingElement) { - // Avoid infinite loop. - if (!$el.length) { + if (!$el.length || $el[0] instanceof Document) { + // Ensure that $().closestScrollable() -> $() and handle the + // case of elements not attached to the DOM. + // Also, .parent() used to loop through ancestors can + // theoretically reach the document if nothing up to the HTML + // included is not scrollable. return $(); } if ($el.isScrollable()) { @@ -173,9 +181,13 @@ $.fn.extend({ * @returns {boolean} */ isScrollable() { + if (!this.length) { + return false; + } const overflow = this.css('overflow-y'); + const el = this[0]; return overflow === 'auto' || overflow === 'scroll' - || (overflow === 'visible' && this === document.scrollingElement); + || (overflow === 'visible' && el === el.ownerDocument.scrollingElement); }, }); diff --git a/addons/web_editor/static/src/js/editor/snippets.editor.js b/addons/web_editor/static/src/js/editor/snippets.editor.js index b755bc9b014..8ffb992f94e 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -1693,6 +1693,9 @@ var SnippetsMenu = Widget.extend({ if (!this.$scrollingElement[0]) { this.$scrollingElement = $(this.ownerDocument).find('.o_editable'); } + this.$scrollingTarget = this.$scrollingElement.is(this.ownerDocument.scrollingElement) + ? $(this.ownerDocument.defaultView) + : this.$scrollingElement; this._onScrollingElementScroll = _.throttle(() => { for (const editor of this.snippetEditors) { editor.toggleOverlayVisibility(false); @@ -1710,7 +1713,7 @@ var SnippetsMenu = Widget.extend({ // Setting capture to true allows to take advantage of event bubbling // for events that otherwise don’t support it. (e.g. useful when // scrolling a modal) - this.$scrollingElement[0].addEventListener('scroll', this._onScrollingElementScroll, {capture: true}); + this.$scrollingTarget[0].addEventListener('scroll', this._onScrollingElementScroll, {capture: true}); // Auto-selects text elements with a specific class and remove this // on text changes @@ -1805,8 +1808,9 @@ var SnippetsMenu = Widget.extend({ } this.$window.off('.snippets_menu'); this.$document.off('.snippets_menu'); - if (this.$scrollingElement) { - this.$scrollingElement[0].removeEventListener('scroll', this._onScrollingElementScroll, {capture: true}); + + if (this.$scrollingTarget) { + this.$scrollingTarget[0].removeEventListener('scroll', this._onScrollingElementScroll, {capture: true}); } } core.bus.off('deactivate_snippet', this, this._onDeactivateSnippet);