Commit Graph

84 Commits

Author SHA1 Message Date
Géry Debongnie 6e185f987b [FIX] qweb: do not leak values in global context in rare cases
In some very rare cases (such as the use of the t-foreach directive),
Owl did leak the values in the render context in the global context.
This was due to the fact that the compiled template looked like this:

let _3 = _4 = _5;

instead of

let _3 = _5;
let _4 = _5;
2021-10-19 16:41:45 +02:00
Géry Debongnie 17ae1d06c4 [IMP] component: better detection for dynamic component change
Before this commit, Owl could not detect that the underlying component
in a template such as <t t-component="{{state.child}}"/> was changing,
if the two components have the same tag as root element.

This is because the reconciliation is done at the vdom level, which does
not know about components.  To solve this, one could use a t-key to make
sure owl can make the difference.

With this commit, we can simply use our knowledge of the fact that we are
dealing with a dynamic component and autogenerate a suitable key.

closes #623
2021-07-05 10:09:23 +02:00
Géry Debongnie caf842c482 [FIX] component: fix scoping issue in nested loops
The templates contained in a slot have to be rendered with the current
scope, but during the rendering of the sub component, which happens
later (after the willStart for the sub component). Therefore, we need to
save the scope that should be used for the slots, so we can access the
proper variables. This was done using an Object.assign() statement,
which is actually only a shallowclone for the own properties.

In this commit, we properly copy all the properties for the current
scope, even those contained in the prototype.

closes #855
2021-07-05 10:09:23 +02:00
Michael Mattiello (mcm) d08ea63565 [IMP] directive: make props override t-props
Before this commit, props and t-props were computed like
`Object.assign(props, t_props)`.
Now, it computes like `Object.assign(t_props, props)` so props will
override t-props.

closes https://github.com/odoo/owl/issues/886
2021-07-02 11:50:23 +02:00
Sébastien Theys 7ebe0da962 [IMP] component: add support for callable expression in event handler 2021-07-01 12:26:23 +02:00
Lucas Perais (lpe) d67d295eaa [FIX] qweb, t-call: t-call nested in t-slot
Have a t-call within a t-set-slot of a component.
The called template has a t-component directive.

It should be like:

```xml
<t t-name="Zero">
  <Slotted>
    <t t-call="someTemplate" />
  </Slotted>
</t>

<t t-name="someTemplate">
  <SomeComponent />
</t>
```

Before this commit, the parent of SomeComponent was the Zero component

After this commit, the parent of SomeComponent is the Slotted Component as it should be

closes #862
2021-06-10 10:00:22 +02:00
Samuel Degueldre e646eb697e [FIX] qweb: fix order of lookup in t-component directive
Previously, the order of resolution was:
- components defined on the class of the current component
- components defined on the QWeb instance
- the current instance's context

This is unnatural because it doesn't go from most specific to least, as
the current instance's context is the most specific. This is also
fragile, as adding components to the QWeb instance can break unrelated
components.

This commit fixes that by making the lookup start with the current
component's context instead
2021-05-18 09:58:30 +02:00
Géry Debongnie 0290f63ba3 [FIX] component: various issues while mounting manually components
The initial problem solved by this commit is that it was possible to get
into a situation where a mounting/rendering was started, then the component was
updated, but then another mounting operation begins, and it tries to
reuse the previous rendering operation, which is no longer uptodate.

The underlying issue is that Owl did not track properly the various
internal state change of a component.  These issue should be solved by
the introduction of the status enum, which currently tracks 6 possible
states:

- CREATED
- WILLSTARTED
- RENDERED
- MOUNTED
- UNMOUNTED
- DESTROYED

This status number replaces the isMounted and isDestroyed boolean flags.
It has the advantage of making sure that the component is in a
consistent state (it is no longer possible to be destroyed and mounted,
for example)

Another advantage is that it gives us an easy way to track the fact that
a component has been rendered, but is not in the DOM.  This is a subtle
situation where some various events can happen, and we need to be able
to react to that case.

Note that there is a change of behaviour: if a component is mounted in a
specific target, then before the mounting is complete, the component is
mounted in another target, we no longer reject the first mounting
operation.
2021-02-08 10:59:28 +01:00
Géry Debongnie 398f9f4e53 [IMP] component: can trigger event handlers even if unmounted
From the beginning, Owl was designed to only call event handler on
components that are mounted in the DOM.  The main benefit is that if a
component is destroyed, we are guaranteed to not execute any useless (or
potentially dangerous code).

However, there is one downside: if a component tree is being mounted,
and a child component trigger an event in its mounted hook, then it
cannot be caught by the parent, since the parent is technically not yet
mounted.

This may not be a good situation, but the point is that Owl
unnecessarily prevent the handler to be called.

We can fix this issue by simply checking if the component is not destroyed
instead of checking that it is mounted.

closes #809
2021-02-03 14:46:18 +01:00
Géry Debongnie a28ce440dc [REF] qweb: simplify _compile method arguments
Strangely, the _compile method required a CompilationContext whenever it
was compiled as a sub template, but this parent context was actually not
really needed.  I guess that it was the case in the past, but this was
changed at some point.

This commit makes another significant change: the xml element is no
longer mandatory.  It is actually only required for slots (because the
template is not registered to qweb).

Finally, the interface for the whole method has been changed to use an
option object, which makes more sense with 3 optional paremeters.
2020-10-19 16:27:38 +02:00
Géry Debongnie 38c7ad9629 [FIX] t-model: support expressions with [ ]
Before this commit, the t-model directive worked well with expressions
such as "state.value", but not with bracketed expression: "state[value]"
(it generated invalid code).

This commit make the t-model smarter by detecting this case, and
properly capturing the base expression and key variable.

closes #694
2020-09-18 14:58:48 +02:00
Géry Debongnie 06d852fcf9 [FIX] component: allow using vars with body as props
Consider this scenario:

- a variable v (with a body) is defined in a template
- it is then passed to a sub component as a prop
- and now, it is t-esc-ed.

Before this commit, the displayed value was [object object], because the
value actually passed to the sub component was a VDomArray (internal
structure used to represent nodelists)

This issue is actually quite a problem in practice, because values in a
templates are translated, but not in attributes.  Therefore, using a
t-set directive with a body text content is the proper way to have
translated values at runtime.

We override in this commit the method toString of VDomArray to make sure
it is properly displayed.

Note that we considered changing the way props were generated (by trying
to detect VDomArray, then calling vDomToString), but then the value
would not be able to be used in a t-raw.  Also, it is quite elegant to
be able to format the VDomArray only at the end.

closes #670
2020-09-17 09:44:08 +02:00
Géry Debongnie e5e7790530 [FIX] qweb: handle input value attribute as a property
Sometimes, HTML is slightly more subtle than what I initially expect.
Rendering some html is simple, we have tags and attributes.  However,
once we add behaviour, then the situation is more complex:

<input value="abc"/>

is an input with an INITIAL value of "abc", but the attribute does not
actually represent the CURRENT value of the input, which may be
different if the user did change it.

This is basically the difference between "attribute" and "property".

So, when rendering html with owl, we sometimes want to actually set
the property (current value), instead of the html attribute.

This commit make sure that this is the case for inputs with the "value"
attribute.

closes #722
2020-09-17 08:32:02 +02:00
Lucas Perais (lpe) 8e03f9cd9c [FIX] qweb: t-call should protect scope and let it accessible
Have a t-call nested in a t-foreach nested in a t-foreach

```xml
<t t-name="template">
  <t t-foreach="..." t-as="a">
    <t t-foreach="..." t-as="b">
      <t-call="templateCalled" />
    </t>
  </t>
</t>
```

Before this commit, the `a` variable was not accessible within the t-call.
That was because the way t-call protected its scope by hiding other protected scope
in this case, the first protected scope for the first `t-foreach` was hidden

After this commit, `a` and `b` are accessible in the t-call, whether the t-call
defines its own variables by `t-set` or not.
Also, as expected from other fixes, there is no leaks of variables defined within a `t-call`

fixes #695
2020-07-28 08:56:00 +02:00
Géry Debongnie 85318b3ae6 [FIX] qweb: properly handle subtemplates in shared templates
With QWeb, we can register globally templates (using the xml tag or
the registerTemplate function). However, these templates, once
compiled, can generate sub template compiled functions. Before this
commit, these sub functions were local to a specific instance.

This means that creating a new QWeb instance and rendering a global
parent template would crash, since it was unable to find the actual sub
function.

This commit fixes the issue: the sub functions are now shared
statically, but with a unique ID, so we do not have issues with sub
functions having a same name in different QWeb instance.

closes #701
2020-05-25 11:43:24 +02:00
lucas c469cac315 [FIX] qweb: protect scope in t-call writing recursively
Have something like
```xml
 <div t-name="Parent">
   <t t-call="nodeTemplate">
     <t t-set="recursive_idx" t-value="1"/>
     <t t-set="node" t-value="root"/>
   </t>
 </div>

 <div t-name="nodeTemplate">
   <t t-set="recursive_idx" t-value="recursive_idx + 1"/>
   <p><t t-esc="node.val"/> <t t-esc="recursive_idx"/></p>
   <t t-foreach="node.children or []" t-as="subtree">
     <t t-call="nodeTemplate">
       <t t-set="node" t-value="subtree"/>
     </t>
   </t>
 </div>
```

Where we want to propagate a recursion index through recursive t-calls

Before this commit, it did not work as we protected the scope in order
to not leak, in the wronf manner. Namely the protected scope only took
firt level prototype properties of the original scope.

After this commit, this case works as we mark the scope as read only

solves #672
2020-03-12 13:15:28 +01:00
Géry Debongnie b00188c1ce [IMP] qweb: add support for event capture
In this commit, we uses the "!" as suffix to designate an event that
should be captured.  This is inspired by the Vue source code.

This solves the issue of communicating additional information to the
underlying virtual dom.  However, this will most likely disappear when
we rewrite the vdom as a virtual block system.

closes #650
2020-02-21 09:22:43 +01:00
Géry Debongnie 20eb848262 [FIX] component: merge hooks properly
Before this commit, creating a sub component with
t-att-style/t-att-class attributes or with t-on- event handlers would
override the *hook* object rendered by the sub component.

This is an issue for some directives, such as t-ref, which defines
hook functions.

This commit fixes the issue by checking for an override, and wrapping
the hooks in a function that calls each defined hook.

closes #638
2020-02-17 15:50:53 +01:00
Géry Debongnie f9cac94dc7 [REF] tests: split component.test into multiple files 2020-01-21 09:26:38 +01:00
Géry Debongnie 49931104a1 [CLEANUP] update snapshots 2020-01-08 16:05:22 +01:00
Lucas Perais (lpe) 21d1306ec2 [FIX] qweb: t-foreach needs to hold on scope
Have a t-on within a t-foreach
the t-on has an expression

Before this commit, when triggering the t-on, the expression was falsely evaluated
In details, the expression took scope from outside the foreach loop
as we protect it to have similar results than an actual for loop

But, at triggering time, the scope protection had already been terminated
meaning the info of the iteration of the loop the handler has been built in
had already disappeared

This commit fixes that

closes #594
2020-01-08 15:45:44 +01:00
Lucas Perais (lpe) 21737d33fa [FIX] qweb: variables set outside foreach must be altered by inloop t-tset
In a template, have t-set t-value outside a t-foreach
in the t-foreach, alter that variable by resetting it (as for a incrementation variable)

Before this commit, when printing the variable when the loop had finished
its value was the one set in the first place

After this commit, the value becomes the one altered by the loop iterations

closes #598
2020-01-08 15:45:44 +01:00
Géry Debongnie 23012f3e7c [FIX] component: multiple t-calls
Before this commit, owl could crash in some specific situations:
multiple t-calls with sub components, outside a loop.  The reason is
that the t-call did not generate a key, so from the point of view of the
children component, it had the same key, even though it was at a
different place in a template.

Also, with this fix, we can fix the playground responsive example.

closes #602
2020-01-08 15:24:43 +01:00
Géry Debongnie 4f61d9f1e0 [FIX] component: no crash if empty props
The expression parser properly format an empty string into an empty
string. But then, this empty string was injected in the props object of
a component, and the compiled code looked like this:

let props5 = { val: };

So, in this case, we simply put undefined, since there are no props
value.

closes #587
2019-12-19 14:32:10 +01:00
Géry Debongnie 6465665550 [FIX] qweb/component: handle nested t-foreach and components
The way keys were handled in QWeb was mostly ok, but a little naive. It
is fine when we deal with a list of dom nodes, since the reconciliation
algorithm need only to be able to differentiate/reconcile nodes in that
list, but it is an issue with components, which needs to globally be
able to find themselves in their parent's children map.

Because of the way this was handled, there were situations were Owl
internal virtual dom would crash, since components wrongly assumed that
they were already rendered in a different place.

To fix this, we generalize the way keys are generated, by concatenating
all sub keys coming from each iteration loops.

closes #584
2019-12-17 15:23:24 +01:00
Géry Debongnie 9216c5c24b [FIX] qweb: t-call properly transfer key to sub components
Since t-call is now a function call, we need to properly handle the
internal key used to find previous components.  If a t-call is inside a
t-foreach, then we need to transfer the key to the sub template.
Otherwise, each component in the subtemplate will be associated to the
same key, which means that it will lead to big issues: components are
destroyed and reused...

closes #581
2019-12-16 20:30:34 +01:00
Lucas Perais (lpe) 5e7f1d5e6d [FIX] qweb: t-call must pass extra parent
Before this commit, when template was t-call'ed and defined
t-component directive within it, there was a crash because extra.parent was never defined

After this commit, this use case works
2019-12-13 20:28:49 +01:00
Géry Debongnie 29a80d0b28 [FIX] qweb: properly display falsy values
While it is not tested, nor documented, the reference QWeb
implementation display the `false` value as "false". So, we have to
adapt the Owl implementation to match that behaviour.

At the same time, this commit uses 'let' instead of 'var' in various places,
to make the compiled code more consistant.
2019-12-13 14:48:19 +01:00
Géry Debongnie 0931a4dc5b [FIX] qweb: bind handlers to component
Before this commit, the handlers were bound to the current context,
which is often the component but not necessarily.  In some cases, a sub
scope can be created (with t-foreach, or slots, or t-call), and the
context is actually an object with the actual component instance it its
prototype chain, but not the component.
2019-12-12 14:32:43 +01:00
Géry Debongnie b283e65ad4 [REF] qweb: improve t-on generated code
and deduplicate logic between directive t-on applied on a node and on a
component
2019-12-12 14:32:43 +01:00
Géry Debongnie f517d44e32 [IMP] qweb: slightly simplify generated templates
- In some cases, we can simply inline a template key instead of assigning
  it to a variable and then using that variable
- Add component name in generated code for components
2019-12-12 09:39:26 +01:00
Géry Debongnie 97d8b3ed8c [REF] tests: remove Widget common class
Bad name, and bad practice to use it. It makes tests less standalone.
2019-12-11 12:25:05 +01:00
Géry Debongnie 9921ae07b1 [REF] tests: move slots tests out of main component file 2019-12-11 12:25:05 +01:00
Géry Debongnie bc455be720 [IMP] components: slots can now define default content
closes #554
2019-12-11 10:11:47 +01:00
Lucas Perais (lpe) 9145799ae9 [FIX] qweb: evaluate body of t-set immediately 2019-12-11 10:07:41 +01:00
Géry Debongnie ce9c2f8613 [FIX] qweb/component: refactoring of scoping/variables/slots
This commit is a significant refactoring of the internal of QWeb. It
simplifies the way variables/scoping and slots interact together.  The
main idea is that we use a simple scope object instead of a
context/var/scope object.

This commit also implement the actual correct QWeb semantic for the
t-call directive with a sub body.  Before, we simply extracted the
variables from the body and injected them at the top of the sub
template.  We now simply compile the body before the sub template.

This is a joint work with Lucas (lpe).

closes #541
closes #544
closes #545

closes #557
closes #556
2019-12-11 10:07:41 +01:00
Géry Debongnie b890e7ceae [IMP] qweb: add template name in generated compiled code 2019-12-10 16:14:09 +01:00
Géry Debongnie a0bfbf6dd0 [FIX] qweb/component: do not call handlers for unmounted components
Unmounted components should be considered inactive, at least from the
perspective of Owl itself.

closes #543
2019-12-06 09:58:12 +01:00
Géry Debongnie b67cb71048 [FIX] slots: propagate vars to correct function
closes #529
2019-12-06 09:53:48 +01:00
Géry Debongnie ed4ab51c17 [FIX] component: tricky concurrency issue
This is a really interesting problem: there was a situation where a
component would not have an event handler properly bound. The reason was
that we were in a situation where the scheduler task queue was flushed
at an unfortunate timing:

- parent component template is rendered
- subcomponent is prepared:
   - sub component is willStarted
   - it is rendered (so, fiber counter is set to 0)
- scheduler task queue is flushed => we patch the DOM
- the code registered in the __prepare.then(handler) is executed, which
  add the createHook to the vnode (but after the dom is patched)

Usually, the last two steps happen in a different order, but in an
environment with connected components, we sometimes flush the task queue
at various moments, so some code could be executed between the end of
__prepare and the registered handler.

The fix is interesting: we give a callback to the __prepare function
instead of registering a .then handler to the deferred.  This callback
is then guaranteed to be called at exactly the proper timing.

Thanks seb for the hard work of finding the root cause of this issue

closes #520
2019-12-03 08:23:10 +01:00
Géry Debongnie 8c17bb0411 [FIX] component: reorganize fiber update algorithm
Closes #473
Closes #484
Closes #489
Closes #492
Closes #512

Co-authored-by: Aaron Bohy <aab@odoo.com>
2019-11-29 15:13:37 +01:00
Aaron Bohy 85d4393242 [FIX] t-slot: correct parented relationship with nested slots
Closes #506
2019-11-28 10:15:57 +01:00
Aaron Bohy 6c8b401092 [FIX] component: slots: multiple slots with components
Closes #508
2019-11-27 15:47:55 +01:00
Aaron Bohy 12be815342 [REF] qweb: use global nextID
and reset it to 1 before each test s.t. snapshots are deterministic.
2019-11-27 15:47:55 +01:00
Géry Debongnie 6c753bb49f [FIX] qweb: scope t-key directive to node and subnodes
not to siblings

closes #503
2019-11-27 13:31:40 +01:00
Aaron Bohy 2e3e8cd603 [FIX] component: destroy not yet mounted components (2)
Following 2922cee6ea

Previous commit was not correct: used children with a cancelled
fiber (for instance, with a new currentFiber) could be destroyed.
This commit fixes the issue differently, by directly detecting
children that are not used anymore (and not mounted), and destroy
them directly (no need to wait for willStart promise to be resolved
anymore).
2019-11-20 14:31:03 +01:00
Aaron Bohy 2922cee6ea [FIX] component: can destroy not yet mounted components
Sub-issue of #476
2019-11-20 11:43:14 +01:00
Géry Debongnie c7773bfd2a [FIX] qweb/component: fix scoping issue with t-model and t-foreach
Without this fix, the handler set by t-model did not capture properly
the expression that needs to be updated.

closes #474
2019-11-19 13:15:34 +01:00
Aaron Bohy e5940b4b6b [FIX] component: concurrent calls to mount and render
Closes #450
2019-11-14 15:12:31 +01:00
Aaron Bohy e47f604449 [FIX] component: concurrent rendering issue
Resolved rendering with cancelled fiber for (not yet) destroyed
component -> Cannot read property 'sel' of null

Closes #421
2019-11-12 10:01:12 +01:00