From 09ef0e4663ad811443018e07720a0ee56755db9b Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Thu, 18 Aug 2022 15:44:41 +0000 Subject: [PATCH] [FIX] web: make scroll utils properly work with website iframe action The `scrollTo` method and related utils (jQuery animate, ...) were not properly adapted to work in the context of multiple documents. E.g. the website editor code (outside the website iframe) is calling the `scrollTo` util to scroll to added snippet elements (inside the website iframe). In that case, the util was performing some searches in the DOM outside of the website iframe while it was meant to be done inside. The result was that the scroll was not considering the website fixed navbar and scrolled "too far". Another bug is visible only when the scrollbar is left to be the natural browser one (not on the `#wrapwrap`). In that case, the `scrollTo` util simply had no effect anymore as the `animate` override of jQuery was not adapted. Note: a further commit will move back the scrollbar behavior out of the `#wrapwrap`, this is thus especially needed. X-original-commit: ab417de98068afaaa12995726c7c6445f075ca4f Part-of: odoo/odoo#102785 --- addons/web/static/src/legacy/js/core/dom.js | 12 +++++++----- addons/web/static/src/legacy/js/libs/jquery.js | 8 +++++--- .../static/src/js/editor/snippets.editor.js | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/addons/web/static/src/legacy/js/core/dom.js b/addons/web/static/src/legacy/js/core/dom.js index 4356cf3737c..404d172b458 100644 --- a/addons/web/static/src/legacy/js/core/dom.js +++ b/addons/web/static/src/legacy/js/core/dom.js @@ -519,11 +519,12 @@ var dom = { * Computes the size by which a scrolling point should be decreased so that * the top fixed elements of the page appear above that scrolling point. * + * @return {Document} [document=window.document] * @returns {number} */ - scrollFixedOffset() { + scrollFixedOffset(document = window.document) { let size = 0; - for (const el of $('.o_top_fixed_element')) { + for (const el of document.querySelectorAll('.o_top_fixed_element')) { size += $(el).outerHeight(); } return size; @@ -552,9 +553,10 @@ var dom = { // If $scrollable and $el are not in the same document, we can safely // assume $el is in an $iframe. We retrieve it by filtering the list of // iframes in $scrollable to keep only the one that contains $el. - const isInOneDocument = $scrollable[0].ownerDocument === $el[0].ownerDocument; + const scrollDocument = $scrollable[0].ownerDocument; + const isInOneDocument = scrollDocument === $el[0].ownerDocument; const $iframe = !isInOneDocument && $scrollable.find('iframe').filter((i, node) => $(node).contents().has($el)); - const $topLevelScrollable = $().getScrollingElement(); + const $topLevelScrollable = $().getScrollingElement(scrollDocument); const isTopScroll = $scrollable.is($topLevelScrollable); function _computeScrollTop() { @@ -579,7 +581,7 @@ var dom = { } let offset = options.forcedOffset; if (offset === undefined) { - offset = (isTopScroll ? dom.scrollFixedOffset() : 0) + (options.extraOffset || 0); + offset = (isTopScroll ? dom.scrollFixedOffset(scrollDocument) : 0) + (options.extraOffset || 0); } return Math.max(0, elPosition - offset); } diff --git a/addons/web/static/src/legacy/js/libs/jquery.js b/addons/web/static/src/legacy/js/libs/jquery.js index 79ff64f298b..9aacdeef68e 100644 --- a/addons/web/static/src/legacy/js/libs/jquery.js +++ b/addons/web/static/src/legacy/js/libs/jquery.js @@ -201,13 +201,14 @@ $.fn.scrollTop = function (value) { // The caller wants to scroll a set of elements including html and/or // body to a specific point -> do that but make sure to add the real // top level element to that set of elements if any different is found. - originalScrollTop.apply(this.not('html, body').add($().getScrollingElement()), arguments); + const $withRealScrollable = this.not('html, body').add($().getScrollingElement(this[0].ownerDocument)); + originalScrollTop.apply($withRealScrollable, arguments); return this; } else if (value === undefined && this.eq(0).is('html, body')) { // The caller wants to get the scroll point of a set of elements, jQuery // will return the scroll point of the first one, if it is html or body // return the scroll point of the real top level element. - return originalScrollTop.apply($().getScrollingElement(), arguments); + return originalScrollTop.apply($().getScrollingElement(this[0].ownerDocument), arguments); } return originalScrollTop.apply(this, arguments); }; @@ -218,7 +219,8 @@ $.fn.animate = function (properties, ...rest) { // The caller wants to scroll a set of elements including html and/or // body to a specific point -> do that but make sure to add the real // top level element to that set of elements if any different is found. - originalAnimate.call(this.not('html, body').add($().getScrollingElement()), {'scrollTop': props['scrollTop']}, ...rest); + const $withRealScrollable = this.not('html, body').add($().getScrollingElement(this[0].ownerDocument)); + originalAnimate.call($withRealScrollable, {'scrollTop': props['scrollTop']}, ...rest); delete props['scrollTop']; } if (!Object.keys(props).length) { 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 029dcab5182..ea153d41792 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -1851,8 +1851,8 @@ 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.$scrollingTarget = this.$scrollingElement.is(this.$body[0].ownerDocument.scrollingElement) + ? $(this.$body[0].ownerDocument.defaultView) : this.$scrollingElement; this._onScrollingElementScroll = _.throttle(() => { for (const editor of this.snippetEditors) {