[FIX] portal: correctly move portal content when target is after it

Previously, when trying to mount a portal into a target that would be
mounted in the same render as the portal itself, the portal content
could not be mounted correctly. In a previous attempt to fix it, a
mistake was made while writing the test causing us to incorrectly
believe in now worked, when in fact, it would just move the portal
content to the end of its parent but without changing it.

This commit fixes the issue by making the `moveBeforeDOMNode` method of
VNodes accept a second optional parameter which is the parent in which
the element should be moved, defaulting to the current parent. This
results in identical behaviour when a parent is not specified, but when
it is, the VNode is "reparented" to the passed parent, which is what is
now done by the Portal

src/runtime/blockdom/block_compiler.ts

@@ -517,8 +517,9 @@ function createBlockClass(template: HTMLElement, ctx: BlockCtx): BlockClass {
       return this.el!;
     }
 
-    moveBeforeDOMNode(node: Node | null) {
-      nodeInsertBefore.call(this.parentEl, this.el!, node);
+    moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+      this.parentEl = parent;
+      nodeInsertBefore.call(parent, this.el!, node);
     }
 
     moveBeforeVNode(other: Block | null, afterNode: Node | null) {

src/runtime/blockdom/event_catcher.ts

@@ -56,9 +56,10 @@ export function createCatcher(eventsSpec: EventsSpec): Catcher {
       }
     }
 
-    moveBeforeDOMNode(node: Node | null) {
-      this.child.moveBeforeDOMNode(node);
-      this.parentEl!.insertBefore(this.afterNode!, node);
+    moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+      this.parentEl = parent;
+      this.child.moveBeforeDOMNode(node, parent);
+      parent!.insertBefore(this.afterNode!, node);
     }
 
     moveBeforeVNode(other: VCatcher | null, afterNode: Node | null) {

src/runtime/blockdom/html.ts

@@ -29,8 +29,8 @@ class VHtml {
     }
   }
 
-  moveBeforeDOMNode(node: Node | null) {
-    const parent = this.parentEl;
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
     for (let elem of this.content) {
       nodeInsertBefore.call(parent, elem, node);
     }

src/runtime/blockdom/index.ts

@@ -10,7 +10,7 @@ export { createCatcher } from "./event_catcher";
 
 export interface VNode<T = any> {
   mount(parent: HTMLElement, afterNode: Node | null): void;
-  moveBeforeDOMNode(node: Node | null): void;
+  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement): void;
   moveBeforeVNode(other: T | null, afterNode: Node | null): void;
   patch(other: T, withBeforeRemove: boolean): void;
   beforeRemove(): void;

src/runtime/blockdom/list.ts

@@ -38,12 +38,13 @@ class VList {
     this.parentEl = parent;
   }
 
-  moveBeforeDOMNode(node: Node | null) {
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
     const children = this.children;
     for (let i = 0, l = children.length; i < l; i++) {
-      children[i].moveBeforeDOMNode(node);
+      children[i].moveBeforeDOMNode(node, parent);
     }
-    this.parentEl!.insertBefore(this.anchor!, node);
+    parent!.insertBefore(this.anchor!, node);
   }
 
   moveBeforeVNode(other: VList | null, afterNode: Node | null) {

src/runtime/blockdom/multi.ts

@@ -38,14 +38,14 @@ export class VMulti {
     this.parentEl = parent;
   }
 
-  moveBeforeDOMNode(node: Node | null) {
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
     const children = this.children;
-    const parent = this.parentEl;
     const anchors = this.anchors;
     for (let i = 0, l = children.length; i < l; i++) {
       let child = children[i];
       if (child) {
-        child.moveBeforeDOMNode(node);
+        child.moveBeforeDOMNode(node, parent);
       } else {
         const anchor = anchors![i];
         nodeInsertBefore.call(parent, anchor, node);

src/runtime/blockdom/text.ts

@@ -23,8 +23,9 @@ abstract class VSimpleNode {
     this.el = node;
   }
 
-  moveBeforeDOMNode(node: Node | null) {
-    nodeInsertBefore.call(this.parentEl, this.el!, node);
+  moveBeforeDOMNode(node: Node | null, parent = this.parentEl) {
+    this.parentEl = parent;
+    nodeInsertBefore.call(parent, this.el!, node);
   }
 
   moveBeforeVNode(other: VText | null, afterNode: Node | null) {

src/runtime/blockdom/toggler.ts

@@ -20,8 +20,8 @@ class VToggler {
     this.child.mount(parent, afterNode);
   }
 
-  moveBeforeDOMNode(node: Node | null) {
-    this.child.moveBeforeDOMNode(node);
+  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement) {
+    this.child.moveBeforeDOMNode(node, parent);
   }
 
   moveBeforeVNode(other: VToggler | null, afterNode: Node | null) {

src/runtime/component_node.ts

@@ -306,8 +306,8 @@ export class ComponentNode<P extends Props = any, E = any> implements VNode<Comp
     this.fiber = null;
   }
 
-  moveBeforeDOMNode(node: Node | null): void {
-    this.bdom!.moveBeforeDOMNode(node);
+  moveBeforeDOMNode(node: Node | null, parent?: HTMLElement): void {
+    this.bdom!.moveBeforeDOMNode(node, parent);
   }
 
   moveBeforeVNode(other: ComponentNode<P, E> | null, afterNode: Node | null) {

src/runtime/portal.ts

@@ -75,7 +75,7 @@ export class Portal extends Component {
       if (!portal.target) {
         const target: HTMLElement = document.querySelector(this.props.target);
         if (target) {
-          portal.content!.moveBeforeVNode(target, null);
+          portal.content!.moveBeforeDOMNode(target.firstChild, target);
         } else {
           throw new OwlError("invalid portal target");
         }

tests/misc/portal.test.ts

@@ -892,7 +892,7 @@ describe("Portal", () => {
     }
     await mount(Parent, fixture);
     expect(fixture.innerHTML).toBe(
-      '<span>child</span><div class="portal"></div><span>portal</span>'
+      '<span>child</span><div class="portal"><span>portal</span></div>'
     );
   });