From 351f894ed9515f82854fd12e83dc78474ef8ffc7 Mon Sep 17 00:00:00 2001 From: "Bruno Boi (boi)" Date: Thu, 11 Apr 2024 15:12:58 +0200 Subject: [PATCH] [FIX] web: correct default container (usePosition) Since [1] positioning of popovers targeting an element contained in an iframe is permitted. This commit will ensure the popper's element positioning is correct in the following case: have a popper - outside an iframe - targeting an element that is inside it - and no container element has been given (default is used) **Before this commit** The default container that is used is the target's owner document element, a.k.a. the iframe's html element. **After this commit** The default container that is used is now the popper's owner document. ** Side notes ** This bug has been found when working for the following taskid-3603843 It is required for this task and will get fw-ported through the master branch. [1]: d6afa9f32512774908a2076f5ece2522efa44042 closes odoo/odoo#161568 X-original-commit: e664b2367f482aa6edc492db16ec92ffff1ac0a1 Signed-off-by: Francois Georis (fge) Signed-off-by: Bruno Boi (boi) --- addons/web/static/src/core/position_hook.js | 2 +- .../static/tests/core/position_hook_tests.js | 72 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/addons/web/static/src/core/position_hook.js b/addons/web/static/src/core/position_hook.js index f140ba97895..cf9caa0d73d 100644 --- a/addons/web/static/src/core/position_hook.js +++ b/addons/web/static/src/core/position_hook.js @@ -122,7 +122,7 @@ function getBestPosition(popper, target, { container, margin, position }, iframe // Retrieve container if (!container) { - container = target.ownerDocument.documentElement; + container = popper.ownerDocument.documentElement; } else if (typeof container === "function") { container = container(); } diff --git a/addons/web/static/tests/core/position_hook_tests.js b/addons/web/static/tests/core/position_hook_tests.js index 7610c18b5a9..102882cfa5d 100644 --- a/addons/web/static/tests/core/position_hook_tests.js +++ b/addons/web/static/tests/core/position_hook_tests.js @@ -555,6 +555,78 @@ QUnit.test("iframe: both popper and target inside", async (assert) => { assert.strictEqual(popperBox.left, onPositionedArgs.solution.left); }); +QUnit.test("iframe: default container is the popper owner's document", async (assert) => { + assert.expect(1); + // Prepare an outer iframe, that will hold the popper element + let def = makeDeferred(); + const outerIframe = document.createElement("iframe"); + Object.assign(outerIframe.style, { height: "450px", width: "450px" }); + outerIframe.onload = def.resolve; + getFixture().prepend(outerIframe); + registerCleanup(() => outerIframe.remove()); + await def; + Object.assign(outerIframe.contentDocument.body.style, { + ...CONTAINER_STYLE, + margin: "0", + justifyContent: "flex-start", + }); + + def = makeDeferred(); + const iframeSheet = outerIframe.contentDocument.createElement("style"); + iframeSheet.onload = def.resolve; + iframeSheet.textContent = ` + #popper { + background-color: plum; + height: 100px; + width: 100px; + } + `; + outerIframe.contentDocument.head.appendChild(iframeSheet); + await def; // wait for the iframe's stylesheet to be loaded + + // Prepare the inner iframe, that will hold the target element + def = makeDeferred(); + const innerIframe = document.createElement("iframe"); + innerIframe.srcdoc = `
`; + Object.assign(innerIframe.style, { + height: "300px", + width: "120px", + marginLeft: "10px", + }); + innerIframe.onload = def.resolve; + outerIframe.contentDocument.body.appendChild(innerIframe); + await def; + Object.assign(innerIframe.contentDocument.body.style, { + ...FLEXBOX_STYLE, + height: "300px", + width: "120px", + margin: "0", + }); + + // Prepare the target element + const target = innerIframe.contentDocument.getElementById("target"); + Object.assign(target.style, TARGET_STYLE); + + // Mount the popper component and check its position + class Popper extends Component { + static template = xml`
`; + setup() { + usePosition("popper", () => target, { + position: "top-start", + onPositioned: (_, { direction, variant }) => { + assert.strictEqual(`${direction}-${variant}`, "top-start"); + // the style setup in this test leaves enough space in the inner iframe + // for the popper to be positioned at top-middle, but this is exactly + // what we want to avoid: the popper's base container should not be the + // inner iframe, but the outer iframe, so the popper should be positioned + // at top-start. + }, + }); + } + } + await mount(Popper, outerIframe.contentDocument.body); +}); + QUnit.test("popper as child of another", async (assert) => { class Child extends Component { static template = /* xml */ xml`