When a parent component has an empty child with a t-on
and an event is triggered inside the parent, blockdom
checks if there is an event_catcher to call.
Doing so, an empty child would cause an error.
This commit fixes that.
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
Currently, the `moveBefore` method on VNodes assumes that the `other`
VNode it receives is of the same type, and that the entire VNode tree
below that other VNode has the exact same structure. While this is
correct in most cases, it breaks down when there is a VToggler somewhere
in the VNode tree, as the structure below a VToggler can be very
different from the structure below another VToggler that was created
from the same compiled code. For example, two iterations of a t-foreach
that contains a <t t-component="..."/> may spawn different components,
and different components obviously have different structures.
One way to fix this is to remove the assumption that the structure of
the `this` block tree in moveBefore is the same as the structure
of the `other` block tree, and instead, always give the concrete DOM
node before which we want to move the current VNode instead of giving it
a VNode and an afterNode as a fallback. One problem with this solution
is that it degrades performance in the "standard" case, where a
t-foreach contains no VToggler anywhere in its block tree, as retrieving
the first concrete DOM node requires calling firstNode() which
recursively traverses the entire tree.
To avoid this performance penalty in the standard case, we opt to only
go down this route whenever we encounter a VToggler when calling
`moveBefore`. This requires that we maintain two separate methods, one
to move a VNode before another VNode of assumed similar structure, which
is basically the current implementation of `moveBefore` for all VNode
types except VToggler, and one implementation that moves a VNode before
a concrete DOM node. This method needs to be implemented for all VNode
types, as all VNode types can be descendants of a VToggler. This method
will only be called from one place: the `moveBeforeVNode` method of the
toggler, which is the point where we realize that the assumption of
identical structure breaks down.
Co-authored-by: Bruno Boi <boi@odoo.com>
Before this commit, using t-on on t-component was not correctly
implemented by owl: when more than one component shared the same
parentelement and have an handler with the same name, the second handler
would override the first (using an internal key). With this commit, we
simply recreate event handler for each instance of a catcher block. Note
that it means that using t-on on components is slightly slower now.
Also, this commit fixes an additional issue that was noticed: the
catcher block would not update its internal handler properly, so it
would not behave properly after being patched.
closes#1199
Bruno Boi