[IMP] IrQweb: refactoring and add technical documentation

Major changes: - Remove some of the recursively when compiling - Compile attributes became a directive - Compile options became a directive - `t-field` compilation now uses the same logic as `t-out` - Simplification of ``t-if`` directive compilation - Improved handling of errors wrapped by QWebException - Constants defined outside the class Odoo ┗━► _render (returns MarkupSafe) ┗━► _compile (returns function) ◄━━━━━━━━━━┓ ┗━► _compile_node (returns code string array) ◄━━━━━━━━┓ ┃ ┃ (skip the current node if found t-qweb-skip) ┃ ┃ ┃ (add technical directives: t-tag-open, t-tag-close, t-inner-content) ┃ ┃ ┃ ┃ ┃ ┣━► _directives_eval_order (defined directive order) ┃ ┃ ┣━► _compile_directives (loop) Consume all remaining directives ◄━━━┓ ┃ ┃ ┃ ┃ (e.g.: to change the indentation) ┃ ┃ ┃ ┃ ┣━► _compile_directive ┃ ┃ ┃ ┃ ┃ ┗━► t-if ━━► _compile_directive_if ━┫ ┃ ┃ ┃ ┃ ┗━► t-foreach ━━► _compile_directive_foreach ━┛ ┃ ┃ ┃ ┃ ┗━► t-inner-content ━━► _compile_directive_inner_content ◄━━━━━┓ ━┛ ┃ ┃ ┃ ┗━► t-options ━━► _compile_directive_options ┃ ┃ ┃ ┃ ┗━► t-call ━━► _compile_directive_call ━┫ ━━━┛ ┃ ┃ ┗━► t-att ━━► _compile_directive_att ┃ ┃ ┃ ┗━► t-tag-open ━━► _compile_directive_open ◄━━┓ ┃ ┃ ┃ ┗━► t-tag-close ━━► _compile_directive_close ◄━━┫ ┃ ┃ ┃ ┗━► t-out ━━► _compile_directive_out ━┛ ━┫ ◄━━┓ ┃ ┃ ┗━► t-field ━━► _compile_directive_field ┃ ━┫ ┃ ┃ ┗━► t-esc ━━► _compile_directive_esc ┃ ━┛ ┃ ┃ ┗━► t-* ━━► ... ┃ ┃ ┃ ┃ ┗━━┻━► _compile_static_node ━┛ closes odoo/odoo#81024 Related: odoo/enterprise#22942 Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
2022-02-03 08:09:31 +00:00
parent 9ce5bc8881
commit e830953570
14 changed files with 1939 additions and 880 deletions
@@ -569,7 +569,7 @@ class IrHttp(models.AbstractModel):
        elif isinstance(exception, QWebException):
            values.update(qweb_exception=exception)

-            if type(exception.error) == exceptions.AccessError:
+            if isinstance(exception.__cause__, exceptions.AccessError):
                code = 403

        elif isinstance(exception, werkzeug.exceptions.HTTPException):
@@ -38,9 +38,9 @@
                </h4>
                <div id="error_qweb" class="collapse show">
                    <div class="card-body">
-                        <p t-if="exception.message">
+                        <p>
                            <strong>Error message:</strong>
-                            <pre t-esc="exception.message"/>
+                            <pre t-esc="exception"/>
                        </p>
                        <p>
                            The error occurred while rendering the template <code t-esc="qweb_exception.name"/>
@@ -1,13 +1,10 @@
 # -*- coding: utf-8 -*-
 from odoo import models

-class QWebCodeFound(Exception):
-    """
-    Exception raised when a qweb compilation encounter dynamic content if the
-    option `raise_on_code` is True.
-    """
-
 class IrQweb(models.AbstractModel):
+    """ Add ``raise_on_code`` option for qweb. When this option is activated
+    then all directives are prohibited.
+    """
    _inherit = 'ir.qweb'

    def _get_template_cache_keys(self):
@@ -15,5 +12,5 @@ class IrQweb(models.AbstractModel):

    def _compile_directives(self, el, options, indent):
        if options.get('raise_on_code'):
-            raise QWebCodeFound()
+            raise PermissionError("This rendering mode prohibits the use of directives.")
        return super()._compile_directives(el, options, indent)
@@ -10,7 +10,6 @@ from lxml import html
 from markupsafe import Markup
 from werkzeug import urls

-from .ir_qweb import QWebCodeFound
 from odoo import _, api, fields, models, tools
 from odoo.addons.base.models.ir_qweb import QWebException
 from odoo.exceptions import UserError, AccessError
@@ -286,11 +285,11 @@ class MailRenderMixin(models.AbstractModel):
                # remove the rendered tag <div> that was added in order to wrap potentially multiples nodes into one.
                render_result = render_result[5:-6]
            except Exception as e:
-                if isinstance(e, QWebException) and isinstance(e.error, QWebCodeFound):
+                if isinstance(e, QWebException) and isinstance(e.__cause__, PermissionError):
                    group = self.env.ref('mail.group_mail_template_editor')
-                    raise AccessError(_('Only users belonging to the "%s" group can modify dynamic templates.', group.name))
+                    raise AccessError(_('Only users belonging to the "%s" group can modify dynamic templates.', group.name)) from e
                _logger.info("Failed to render template : %s", template_src, exc_info=True)
-                raise UserError(_("Failed to render QWeb template : %s)", e))
+                raise UserError(_("Failed to render QWeb template : %s)", e)) from e
            results[record.id] = render_result

        return results
@@ -24,7 +24,7 @@
 <h3 style="font-weight: bolder"><u>Milestones</u></h3>

 <ul class="o_checklist" t-if="milestones['list']">
-<t t-foreach="milestones['list']" t-as="milestone" t-key="milestone['id']">
+<t t-foreach="milestones['list']" t-as="milestone">
 <li t-attf-class="{{milestone['is_reached'] and 'o_checked' or ''}}">
 <t t-esc="milestone['name']"/>
 <span t-if="milestone['is_deadline_future'] and not milestone['is_reached']"><font style="color: rgb(190, 190, 190);"><t t-set="color_level" t-value="64"/><t t-call="project.milestone_deadline"/></font></span>
@@ -38,7 +38,7 @@
 <t t-if="len(milestones['updated']) > 1">the deadline of the following milestones has been updated:</t>
 <t t-else="">the deadline of the following milestone has been updated:</t>
 <ul>
-<t t-foreach="milestones['updated']" t-as="milestone" t-key="milestone['id']">
+<t t-foreach="milestones['updated']" t-as="milestone">
 <li>
 <t t-esc="milestone['name']"/> (<t t-esc="milestone['old_value']" t-options='{"widget": "date"}'/> =&gt; <t t-esc="milestone['new_value']"  t-options='{"widget": "date"}'/>)
 </li>
@@ -50,7 +50,7 @@
 <t t-if="len(milestones['created']) > 1">The following milestones have been added:</t>
 <t t-else="">The following milestone has been added:</t>
 <ul>
-<t t-foreach="milestones['created']" t-as="milestone" t-key="milestone['id']">
+<t t-foreach="milestones['created']" t-as="milestone">
 <li>
 <t t-esc="milestone['name']"/><span t-if="milestone['is_deadline_future'] and not milestone['is_reached']">
 <font style="color: rgb(190, 190, 190);">
@@ -54,7 +54,7 @@ class TestReports(TestReportsCommon):
            'company_id': self.env.company.id,
        })
        report = self.env.ref('stock.label_lot_template')
-        target = b'\n\n^XA\n^FO100,50\n^A0N,44,33^FD[C418]Mellohi^FS\n^FO100,100\n^A0N,44,33^FDLN/SN:Volume-Beta^FS\n^FO100,150^BY3\n^BCN,100,Y,N,N\n^FDVolume-Beta^FS\n^XZ\n\n'
+        target = b'\n\n^XA\n^FO100,50\n^A0N,44,33^FD[C418]Mellohi^FS\n^FO100,100\n^A0N,44,33^FDLN/SN:Volume-Beta^FS\n^FO100,150^BY3\n^BCN,100,Y,N,N\n^FDVolume-Beta^FS\n^XZ\n'

        rendering, qweb_type = report._render_qweb_text(lot1.id)
        self.assertEqual(target, rendering.replace(b' ', b''), 'The rendering is not good')
@@ -1086,16 +1086,17 @@ class Database(http.Controller):
        templates = {}

        with file_open("web/static/src/public/database_manager.qweb.html", "r") as fd:
-            template = fd.read()
+            templates['database_manager'] = fd.read()
        with file_open("web/static/src/public/database_manager.master_input.qweb.html", "r") as fd:
            templates['master_input'] = fd.read()
        with file_open("web/static/src/public/database_manager.create_form.qweb.html", "r") as fd:
            templates['create_form'] = fd.read()

        def load(template_name, options):
-            return (html.fragment_fromstring(templates[template_name]), template_name)
+            fromstring = html.document_fromstring if template_name == 'database_manager' else html.fragment_fromstring
+            return (fromstring(templates[template_name]), template_name)

-        return qweb_render(html.document_fromstring(template), d, load=load)
+        return qweb_render('database_manager', d, load)

    @http.route('/web/database/selector', type='http', auth="none")
    def selector(self, **kw):
@@ -44,8 +44,6 @@ class IrQWeb(models.AbstractModel):
    """
    _inherit = 'ir.qweb'

-    # compile directives
-
    def _compile_node(self, el, options, indent):
        snippet_key = options.get('snippet-key')
        if snippet_key == options['template'] \
@@ -71,10 +69,12 @@ class IrQWeb(models.AbstractModel):

        return super()._compile_node(el, options, indent)

+    # compile directives
+
    def _compile_directive_snippet(self, el, options, indent):
        key = el.attrib.pop('t-snippet')
        el.set('t-call', key)
-        el.set('t-options', "{'snippet-key': '" + key + "'}")
+        el.set('t-options', f"{{'snippet-key': {key!r}}}")
        View = self.env['ir.ui.view'].sudo()
        view_id = View.get_view_id(key)
        name = View.browse(view_id).name
@@ -85,41 +85,42 @@ class IrQWeb(models.AbstractModel):
            escape(pycompat.to_text(view_id)),
            escape(pycompat.to_text(el.findtext('keywords')))
        )
-        self._appendText(div, options)
+        self._append_text(div, options)
        code = self._compile_node(el, options, indent)
-        self._appendText('</div>', options)
+        self._append_text('</div>', options)
        return code

    def _compile_directive_snippet_call(self, el, options, indent):
        key = el.attrib.pop('t-snippet-call')
        el.set('t-call', key)
-        el.set('t-options', "{'snippet-key': '" + key + "'}")
+        el.set('t-options', f"{{'snippet-key': {key!r}}}")
        return self._compile_node(el, options, indent)

    def _compile_directive_install(self, el, options, indent):
+        key = el.attrib.pop('t-install')
+        thumbnail = el.attrib.pop('t-thumbnail', 'oe-thumbnail')
        if self.user_has_groups('base.group_system'):
-            module = self.env['ir.module.module'].search([('name', '=', el.attrib.get('t-install'))])
+            module = self.env['ir.module.module'].search([('name', '=', key)])
            if not module or module.state == 'installed':
                return []
            name = el.attrib.get('string') or 'Snippet'
-            thumbnail = el.attrib.pop('t-thumbnail', 'oe-thumbnail')
            div = '<div name="%s" data-oe-type="snippet" data-module-id="%s" data-oe-thumbnail="%s"><section/></div>' % (
                escape(pycompat.to_text(name)),
                module.id,
                escape(pycompat.to_text(thumbnail))
            )
-            self._appendText(div, options)
+            self._append_text(div, options)
        return []

-    def _compile_directive_tag(self, el, options, indent):
-        if el.get('t-placeholder'):
-            el.set('t-att-placeholder', el.attrib.pop('t-placeholder'))
-        return super(IrQWeb, self)._compile_directive_tag(el, options, indent)
+    def _compile_directive_placeholder(self, el, options, indent):
+        el.set('t-att-placeholder', el.attrib.pop('t-placeholder'))
+        return []

    # order and ignore

    def _directives_eval_order(self):
-        directives = super(IrQWeb, self)._directives_eval_order()
+        directives = super()._directives_eval_order()
+        directives.insert(directives.index('att'), 'placeholder')
        directives.insert(directives.index('call'), 'snippet')
        directives.insert(directives.index('call'), 'snippet-call')
        directives.insert(directives.index('call'), 'install')
@@ -463,25 +463,25 @@ class TestProfiling(TransactionCase):
            'type': 'qweb',
            'arch_db': '''<t t-name="root">
                <t t-foreach="{'a': 3, 'b': 2, 'c': 1}" t-as="item">
-                    [<t t-esc="item_index"/>: <t t-call="base.dummy"/> <t t-esc="item_value"/>]
+                    [<t t-esc="item_index"/>: <t t-call="base.dummy" t-set-record="item"/> <t t-esc="item_value"/>]
                    <b t-esc="add_one_query()"/></t>
            </t>'''
        })
        child_template = self.env['ir.ui.view'].create({
            'name': 'test',
            'type': 'qweb',
-            'arch_db': '<t t-name="dummy"><span><t t-esc="item"/> <t t-esc="add_one_query()"/></span></t>'
+            'arch_db': '<t t-name="dummy"><span t-attf-class="myclass"><t t-esc="record"/> <t t-esc="add_one_query()"/></span></t>'
        })
        self.env.cr.execute("INSERT INTO ir_model_data(name, model, res_id, module)"
                            "VALUES ('dummy', 'ir.ui.view', %s, 'base')", [child_template.id])

        values = {'add_one_query': lambda: self.env.cr.execute('SELECT id FROM ir_ui_view LIMIT 1') or 'query'}
        result = u"""
-                    [0: <span>a query</span> 3]
+                    [0: <span class="myclass">a query</span> 3]
                    <b>query</b>
-                    [1: <span>b query</span> 2]
+                    [1: <span class="myclass">b query</span> 2]
                    <b>query</b>
-                    [2: <span>c query</span> 1]
+                    [2: <span class="myclass">c query</span> 1]
                    <b>query</b>
        """

@@ -510,34 +510,41 @@ class TestProfiling(TransactionCase):
        for data in p.collectors[0].entries[0]['results']['data']:
            data.pop('delay')

+        data = p.collectors[0].entries[0]['results']['data']
        expected = [
            # pylint: disable=bad-whitespace
            # first template and first directive
            {'view_id': template.id,       'xpath': '/t/t',         'directive': """t-foreach="{'a': 3, 'b': 2, 'c': 1}" t-as='item'""", 'query': 0},
            # first pass in the loop
            {'view_id': template.id,       'xpath': '/t/t/t[1]',    'directive': "t-esc='item_index'", 'query': 0},
-            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-call='base.dummy'", 'query': 0}, # the compiled template method is in cache
+            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-call='base.dummy'", 'query': 0}, # 0 because the template is in ir.ui.view cache
+            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-set-record='item'", 'query': 0}, # t-set it's consumed by t-call
            # first pass in the loop: content of the child template
-            {'view_id': child_template.id, 'xpath': '/t/span/t[1]', 'directive': "t-esc='item'", 'query': 0},
+            {'view_id': child_template.id, 'xpath': '/t/span',      'directive': "t-attf-class='myclass'", 'query': 0},
+            {'view_id': child_template.id, 'xpath': '/t/span/t[1]', 'directive': "t-esc='record'", 'query': 0},
            {'view_id': child_template.id, 'xpath': '/t/span/t[2]', 'directive': "t-esc='add_one_query()'", 'query': 1},
            {'view_id': template.id,       'xpath': '/t/t/t[3]',    'directive': "t-esc='item_value'", 'query': 0},
            {'view_id': template.id,       'xpath': '/t/t/b',       'directive': "t-esc='add_one_query()'", 'query':1},
            # second pass in the loop
            {'view_id': template.id,       'xpath': '/t/t/t[1]',    'directive': "t-esc='item_index'", 'query': 0},
-            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-call='base.dummy'", 'query': 0}, # 0 because the template is in cache
-            {'view_id': child_template.id, 'xpath': '/t/span/t[1]', 'directive': "t-esc='item'", 'query': 0},
+            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-call='base.dummy'", 'query': 0},
+            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-set-record='item'", 'query': 0},
+            {'view_id': child_template.id, 'xpath': '/t/span',      'directive': "t-attf-class='myclass'", 'query': 0},
+            {'view_id': child_template.id, 'xpath': '/t/span/t[1]', 'directive': "t-esc='record'", 'query': 0},
            {'view_id': child_template.id, 'xpath': '/t/span/t[2]', 'directive': "t-esc='add_one_query()'", 'query': 1},
            {'view_id': template.id,       'xpath': '/t/t/t[3]',    'directive': "t-esc='item_value'", 'query': 0},
            {'view_id': template.id,       'xpath': '/t/t/b',       'directive': "t-esc='add_one_query()'", 'query':1},
            # third pass in the loop
            {'view_id': template.id,       'xpath': '/t/t/t[1]',    'directive': "t-esc='item_index'", 'query': 0},
            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-call='base.dummy'", 'query': 0},
-            {'view_id': child_template.id, 'xpath': '/t/span/t[1]', 'directive': "t-esc='item'", 'query': 0},
+            {'view_id': template.id,       'xpath': '/t/t/t[2]',    'directive': "t-set-record='item'", 'query': 0},
+            {'view_id': child_template.id, 'xpath': '/t/span',      'directive': "t-attf-class='myclass'", 'query': 0},
+            {'view_id': child_template.id, 'xpath': '/t/span/t[1]', 'directive': "t-esc='record'", 'query': 0},
            {'view_id': child_template.id, 'xpath': '/t/span/t[2]', 'directive': "t-esc='add_one_query()'", 'query': 1},
            {'view_id': template.id,       'xpath': '/t/t/t[3]',    'directive': "t-esc='item_value'", 'query': 0},
            {'view_id': template.id,       'xpath': '/t/t/b',       'directive': "t-esc='add_one_query()'", 'query':1},
        ]
-        self.assertEqual(p.collectors[0].entries[0]['results']['data'], expected)
+        self.assertEqual(data, expected)

    def test_default_recorders(self):
        with Profiler(db=None) as p:
@@ -1056,7 +1056,7 @@ class TestAssetsManifest(AddonManifestPatched):
        with self.assertRaises(Exception) as cm:
            view._render()
        self.assertTrue(
-            "['test_assetsbundle/static/src/js/test_doesntexist.js'] not found" in cm.exception.message
+            "['test_assetsbundle/static/src/js/test_doesntexist.js'] not found" in str(cm.exception)
        )

    def test_09_remove_wholeglob(self):
@@ -1143,10 +1143,11 @@ class TestAssetsManifest(AddonManifestPatched):

        with self.assertRaises(QWebException) as cm:
            view._render()
-        self.assertTrue(cm.exception.error)
-        self.assertFalse(isinstance(cm.exception.error, RecursionError))
+        error = str(cm.exception.__cause__)
+        self.assertTrue(error)
+        self.assertFalse(isinstance(error, RecursionError))
        self.assertTrue(
-            'Circular assets bundle declaration:' in cm.exception.message
+            'Circular assets bundle declaration:' in error
        )

    def test_13_2_include_recursive_sibling(self):
@@ -1681,7 +1682,7 @@ class TestAssetsManifest(AddonManifestPatched):
        with self.assertRaises(Exception) as cm:
            view._render()
        self.assertTrue(
-            "test_assetsbundle/static/src/js/doesnt_exist.js not found" in cm.exception.message
+            "test_assetsbundle/static/src/js/doesnt_exist.js not found" in str(cm.exception)
        )

    def test_29_js_after_js_in_irasset_glob(self):
@@ -1790,7 +1791,7 @@ class TestAssetsManifest(AddonManifestPatched):
        with self.assertRaises(QWebException) as cm:
            view._render()

-        self.assertTrue('Unallowed to fetch files from addon notinstalled_module' in cm.exception.message)
+        self.assertTrue('Unallowed to fetch files from addon notinstalled_module' in str(cm.exception))

    def test_33bis_notinstalled_not_in_manifests(self):
        self.env['ir.asset'].create({
@@ -312,13 +312,12 @@ class QwebTracker():
    @classmethod
    def wrap_compile_directive(cls, method_compile_directive):
        @functools.wraps(method_compile_directive)
-        def _tracked_compile_directive(self, el, options, directive, indent):
-            if not options.get('profile') or directive in ('inner-content', 'tag'):
-                return method_compile_directive(self, el, options, directive, indent)
-
-            enter = self._indent(f"self.env.context['qweb_tracker'].enter_directive({directive!r}, {el.attrib!r}, {options['last_path_node']!r})", indent)
-            leave = self._indent("self.env.context['qweb_tracker'].leave_directive()", indent)
-            code_directive = method_compile_directive(self, el, options, directive, indent)
+        def _tracked_compile_directive(self, el, options, directive, level):
+            if not options.get('profile') or directive in ('inner-content', 'tag-open', 'tag-close'):
+                return method_compile_directive(self, el, options, directive, level)
+            enter = f"{' ' * 4 * level}self.env.context['qweb_tracker'].enter_directive({directive!r}, {el.attrib!r}, {options['last_path_node']!r})"
+            leave = f"{' ' * 4 * level}self.env.context['qweb_tracker'].leave_directive()"
+            code_directive = method_compile_directive(self, el, options, directive, level)
            return [enter, *code_directive, leave] if code_directive else []
        return _tracked_compile_directive

@@ -336,19 +335,30 @@ class QwebTracker():
        execution_context = None
        if self.execution_context_enabled:
            directive_info = {}
-            if directive and ('t-' + directive) in attrib:
+            if ('t-' + directive) in attrib:
                directive_info['t-' + directive] = repr(attrib['t-' + directive])
            if directive == 'set':
                if 't-value' in attrib:
                    directive_info['t-value'] = repr(attrib['t-value'])
                if 't-valuef' in attrib:
                    directive_info['t-valuef'] = repr(attrib['t-valuef'])
+
+                for key in attrib:
+                    if key.startswith('t-set-') or key.startswith('t-setf-'):
+                        directive_info[key] = repr(attrib[key])
            elif directive == 'foreach':
                directive_info['t-as'] = repr(attrib['t-as'])
+            elif directive == 'att':
+                for key in attrib:
+                    if key.startswith('t-att-') or key.startswith('t-attf-'):
+                        directive_info[key] = repr(attrib[key])
            elif directive == 'options':
-                for key in list(attrib):
+                for key in attrib:
                    if key.startswith('t-options-'):
                        directive_info[key] = repr(attrib[key])
+            elif ('t-' + directive) not in attrib:
+                directive_info['t-' + directive] = None
+
            execution_context = tools.profiler.ExecutionContext(**directive_info, xpath=xpath)
            execution_context.__enter__()
            self.context_stack.append(execution_context)
@@ -381,21 +391,36 @@ class QwebCollector(Collector):
    def _get_directive_profiling_name(self, directive, attrib):
        expr = ''
        if directive == 'set':
-            expr = f"t-set={repr(attrib['t-set'])}"
-            if 't-value' in attrib:
-                expr = f"{expr} t-value={repr(attrib['t-value'])}"
-            if 't-valuef' in attrib:
-                expr = f"{expr} t-valuef={repr(attrib['t-valuef'])}"
+            if 't-set' in attrib:
+                expr = f"t-set={repr(attrib['t-set'])}"
+                if 't-value' in attrib:
+                    expr += f" t-value={repr(attrib['t-value'])}"
+                if 't-valuef' in attrib:
+                    expr += f" t-valuef={repr(attrib['t-valuef'])}"
+            for key in attrib:
+                if key.startswith('t-set-') or key.startswith('t-setf-'):
+                    if expr:
+                        expr += ' '
+                    expr += f"{key}={repr(attrib[key])}"
        elif directive == 'foreach':
            expr = f"t-foreach={repr(attrib['t-foreach'])} t-as={repr(attrib['t-as'])}"
        elif directive == 'options':
            if attrib.get('t-options'):
                expr = f"t-options={repr(attrib['t-options'])}"
-            for key in list(attrib):
+            for key in attrib:
                if key.startswith('t-options-'):
                    expr = f"{expr}  {key}={repr(attrib[key])}"
-        elif directive and ('t-' + directive) in attrib:
+        elif directive == 'att':
+            for key in attrib:
+                if key == 't-att' or key.startswith('t-att-') or key.startswith('t-attf-'):
+                    if expr:
+                        expr += ' '
+                    expr += f"{key}={repr(attrib[key])}"
+        elif ('t-' + directive) in attrib:
            expr = f"t-{directive}={repr(attrib['t-' + directive])}"
+        else:
+            expr = f"t-{directive}"
+
        return expr

    def start(self):
@@ -4,11 +4,38 @@ from lxml.builder import E
 import copy
 import itertools
 import logging
+import re

 from odoo.tools.translate import _
 from odoo.tools import SKIPPED_ELEMENT_TYPES, html_escape

 _logger = logging.getLogger(__name__)
+RSTRIP_REGEXP = re.compile(r'\n[ \t]*$')
+
+def add_stripped_items_before(node, spec, extract):
+    text = spec.text or ''
+
+    before_text = ''
+    prev = node.getprevious()
+    if prev is None:
+        parent = node.getparent()
+        result = parent.text and RSTRIP_REGEXP.search(parent.text)
+        before_text = result.group(0) if result else ''
+        parent.text = (parent.text or '').rstrip() + text
+    else:
+        result = prev.tail and RSTRIP_REGEXP.search(prev.tail)
+        before_text = result.group(0) if result else ''
+        prev.tail = (prev.tail or '').rstrip() + text
+
+    if len(spec) > 0:
+        spec[-1].tail = (spec[-1].tail or "").rstrip() + before_text
+    else:
+        spec.text = (spec.text or "").rstrip() + before_text
+
+    for child in spec:
+        if child.get('position') == 'move':
+            child = extract(child)
+        node.addprevious(child)


 def add_text_before(node, text):
@@ -17,22 +44,12 @@ def add_text_before(node, text):
        return
    prev = node.getprevious()
    if prev is not None:
-        prev.tail = (prev.tail or "").rstrip() + text
+        prev.tail = (prev.tail or "") + text
    else:
        parent = node.getparent()
        parent.text = (parent.text or "").rstrip() + text


-def add_text_inside(node, text):
-    """ Add text inside ``node``. """
-    if text is None:
-        return
-    if len(node):
-        node[-1].tail = (node[-1].tail or "").rstrip() + text
-    else:
-        node.text = (node.text or "") + text
-
-
 def remove_element(node):
    """ Remove ``node`` but not its tail, from its XML tree. """
    add_text_before(node, node.tail)
@@ -210,28 +227,22 @@ def apply_inheritance_specs(source, specs_tree, inherit_branding=False, pre_loca
                    elif attribute in node.attrib:
                        del node.attrib[attribute]
            elif pos == 'inside':
-                add_text_inside(node, spec.text)
-                for child in spec:
-                    if child.get('position') == 'move':
-                        child = extract(child)
-                    node.append(child)
+                # add a sentinel element at the end, insert content of spec
+                # before the sentinel, then remove the sentinel element
+                sentinel = E.sentinel()
+                node.append(sentinel)
+                add_stripped_items_before(sentinel, spec, extract)
+                remove_element(sentinel)
            elif pos == 'after':
                # add a sentinel element right after node, insert content of
                # spec before the sentinel, then remove the sentinel element
                sentinel = E.sentinel()
                node.addnext(sentinel)
-                add_text_before(sentinel, spec.text)
-                for child in spec:
-                    if child.get('position') == 'move':
-                        child = extract(child)
-                    sentinel.addprevious(child)
+                add_stripped_items_before(sentinel, spec, extract)
                remove_element(sentinel)
            elif pos == 'before':
-                add_text_before(node, spec.text)
-                for child in spec:
-                    if child.get('position') == 'move':
-                        child = extract(child)
-                    node.addprevious(child)
+                add_stripped_items_before(node, spec, extract)
+
            else:
                raise ValueError(
                    _("Invalid position attribute: '%s'") %