[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>
This commit is contained in:
Lucas Perais
2023-11-20 11:13:25 +00:00
parent addfbb5a39
commit 0bc8d169f5
2 changed files with 44 additions and 10 deletions
@@ -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: {
@@ -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";