From 0bc8d169f59657b8741109f39486ff7bce4722fc Mon Sep 17 00:00:00 2001
From: Lucas Perais <lpe@odoo.com>
Date: Wed, 15 Nov 2023 08:59:31 +0000
Subject: [PATCH] [FIX] web: popover validation target props

Using the OdooEditor, the following use case arises often:
- create an Element with document.createElement.
- append that node into the DOM tree of an iframe.
- spawn a popover with that node as target.

The spec (https://developer.mozilla.org/en-US/docs/Web/API/Document/importNode) considers this a malpractice:
`Before they can be inserted into the current document, nodes from external documents should either be:
cloned using document.importNode(); or
adopted using document.adoptNode().

Note: Although Firefox doesn't currently enforce this rule, we encourage you to follow this rule for improved future compatibility.`

Before this commit, in debug mode, there was a crash because the class of the new Element
did not match the class of the element's ownerDocument defaultView.

After this commit, we keep the check that says that the target should be an instance of
the iframe's document's Element class, but we fallback onto the main Window Element class as well.

closes odoo/odoo#142464

X-original-commit: 0cdd8cf9354e494c3e56d9eefe81c3fb6a59ac09
Signed-off-by: Lucas Perais (lpe) <lpe@odoo.com>
---
 addons/web/static/src/core/popover/popover.js |  2 +-
 .../tests/core/popover/popover_tests.js       | 52 +++++++++++++++----
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/addons/web/static/src/core/popover/popover.js b/addons/web/static/src/core/popover/popover.js
index c0f26f04b9b..2355763c84d 100644
--- a/addons/web/static/src/core/popover/popover.js
+++ b/addons/web/static/src/core/popover/popover.js
@@ -146,7 +146,7 @@ Popover.props = {
             // target may be inside an iframe, so get the Element constructor
             // to test against from its owner document's default view
             const Element = target?.ownerDocument?.defaultView.Element;
-            return Boolean(Element) && target instanceof Element;
+            return Boolean(Element) && (target instanceof Element || target instanceof window.Element);
         },
     },
     slots: {
diff --git a/addons/web/static/tests/core/popover/popover_tests.js b/addons/web/static/tests/core/popover/popover_tests.js
index ecdc5464ee7..05ddb9d57c7 100644
--- a/addons/web/static/tests/core/popover/popover_tests.js
+++ b/addons/web/static/tests/core/popover/popover_tests.js
@@ -196,15 +196,11 @@ QUnit.test("reposition popover should properly change classNames", async (assert
     const TestPopover = class extends Popover {
         setup() {
             // Don't call super.setup() in order to replace the use of usePosition hook...
-            usePosition(
-                "ref",
-                () => this.props.target,
-                {
-                    container,
-                    onPositioned: this.onPositioned.bind(this),
-                    position: this.props.position,
-                }
-            );
+            usePosition("ref", () => this.props.target, {
+                container,
+                onPositioned: this.onPositioned.bind(this),
+                position: this.props.position,
+            });
         }
     };
 
@@ -287,6 +283,44 @@ QUnit.test("within iframe", async (assert) => {
     assert.strictEqual(popoverBox.left, expectedLeft);
 });
 
+QUnit.test("within iframe -- wrong element class", async (assert) => {
+    /**
+     * This use case exists in real life, when adding some blocks with the OdooEditor
+     * in an iframe. The HTML spec discourages it though.
+     * https://developer.mozilla.org/en-US/docs/Web/API/Document/importNode
+     */
+    const iframe = document.createElement("iframe");
+    iframe.style.height = "200px";
+    iframe.srcdoc = `<div id="target" style="height:400px;">Within iframe</div>`;
+    const def = makeDeferred();
+    iframe.onload = def.resolve;
+    fixture.appendChild(iframe);
+    await def;
+
+    const wrongElement = document.createElement("div");
+    wrongElement.classList.add("wrong-element");
+    iframe.contentDocument.body.appendChild(wrongElement);
+    class TestPopover extends Popover {
+        static props = {
+            ...Popover.props,
+            target: {
+                validate: (...args) => {
+                    const val = Popover.props.target.validate(...args);
+                    assert.step(`validate target props: "${val}"`);
+                    return val;
+                },
+            },
+        };
+    }
+    await mount(TestPopover, fixture, {
+        env,
+        props: { target: wrongElement },
+        test: true,
+    });
+    assert.containsOnce(fixture, ".o_popover");
+    assert.verifySteps(['validate target props: "true"']);
+});
+
 QUnit.test("popover fixed position", async (assert) => {
     const container = document.createElement("div");
     container.id = "container";