From 89f9bb887abd8eb5c7ff2ebaa3ff282e4db06795 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Wed, 11 May 2022 08:32:30 +0000 Subject: [PATCH] [FIX] base, tools: properly distribute branding around removed elements When branding was added on items which follow an element that is removed by an inheriting view, the branding was incorrect. Indeed, it supposed the removed element does not exist in the original view. E.g.: Parent view: ``` ``` Child view: ``` ``` => There are two in the original view, the first one is removed by a child view. The data-oe-xpath set on the remaining should still be /hello/world[2] and not /hello/world[1] to target the right element in the parent view. This of course induced edition problems where a saved area was not saved inside the right element of the parent view or, more likely in normally complex arch, just crashed on save. As an example, with 14.0 enterprise: - Install website_appointment - Go to a page with a calendar to schedule an appointment - Enter edit mode, try to add something in the area *below* the calendar - Save => crash Commit [1] already fixed similar problems when an element was *replaced* by something (especially, when replaced by an element with the same tag name). This commit actually reviews what was done to fix both problems (replacement and removal) at the same time. It also makes it so the xml that `apply_inheritance_specs` produces has no "Element" part impacted when used with "inherit_branding=True", which seems better... although the notion of inheriting branding should probably be independant from this function (maybe something to do in master). [1]: https://github.com/odoo/odoo/commit/c077ef05575d9677bce284195683f96c68386788 opw-2811674 X-original-commit: 4ab569933464617444f6150793876ff4eba11690 Part-of: odoo/odoo#91991 --- odoo/addons/base/models/ir_ui_view.py | 14 +++++----- odoo/addons/base/tests/test_views.py | 38 +++++++++++++++++++++++++++ odoo/tools/template_inheritance.py | 20 +++++++------- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py index 094de41a9e7..fefd9d9d2a3 100644 --- a/odoo/addons/base/models/ir_ui_view.py +++ b/odoo/addons/base/models/ir_ui_view.py @@ -1956,16 +1956,18 @@ actual arch. # TODO: collections.Counter if remove p2.6 compat # running index by tag type, for XPath query generation indexes = collections.defaultdict(lambda: 0) - for child in e.iterchildren(tag=etree.Element): + for child in e.iterchildren(etree.Element, etree.ProcessingInstruction): if child.get('data-oe-xpath'): # injected by view inheritance, skip otherwise # generated xpath is incorrect - # Also, if a node is known to have been replaced during applying xpath - # increment its index to compute an accurate xpath for susequent nodes - replaced_node_tag = child.attrib.pop('meta-oe-xpath-replacing', None) - if replaced_node_tag: - indexes[replaced_node_tag] += 1 self.distribute_branding(child) + elif child.tag is etree.ProcessingInstruction: + # If a node is known to have been replaced during + # applying an inheritance, increment its index to + # compute an accurate xpath for subsequent nodes + if child.target == 'apply-inheritance-specs-node-removal': + indexes[child.text] += 1 + e.remove(child) else: indexes[child.tag] += 1 self.distribute_branding( diff --git a/odoo/addons/base/tests/test_views.py b/odoo/addons/base/tests/test_views.py index cb5b91298e2..7ad8a1f9442 100644 --- a/odoo/addons/base/tests/test_views.py +++ b/odoo/addons/base/tests/test_views.py @@ -870,6 +870,44 @@ class TestTemplating(ViewCase): initial.get('data-oe-xpath'), "The node's xpath position should be correct") + def test_branding_inherit_remove_node(self): + view1 = self.View.create({ + 'name': "Base view", + 'type': 'qweb', + # The t-esc node is to ensure branding is distributed to both + # elements from the start + 'arch': """ + + + + + + + """ + }) + self.View.create({ + 'name': "Extension", + 'type': 'qweb', + 'inherit_id': view1.id, + 'arch': """ + + + + """ + }) + + arch_string = view1.with_context(inherit_branding=True).get_combined_arch() + + arch = etree.fromstring(arch_string) + self.View.distribute_branding(arch) + + # Only remaining world but still the second in original view + [initial] = arch.xpath('/hello[1]/world[1]') + self.assertEqual( + '/hello[1]/world[2]', + initial.get('data-oe-xpath'), + "The node's xpath position should be correct") + def test_branding_inherit_top_t_field(self): view1 = self.View.create({ 'name': "Base view", diff --git a/odoo/tools/template_inheritance.py b/odoo/tools/template_inheritance.py index e6e62b78d88..45e7d7f96f5 100644 --- a/odoo/tools/template_inheritance.py +++ b/odoo/tools/template_inheritance.py @@ -176,19 +176,19 @@ def apply_inheritance_specs(source, specs_tree, inherit_branding=False, pre_loca comment.tail = text source.insert(0, comment) else: - replaced_node_tag = None + # TODO ideally the notion of 'inherit_branding' should + # not exist in this function. Given the current state of + # the code, it is however necessary to know where nodes + # were removed when distributing branding. As a stable + # fix, this solution was chosen: the location is marked + # with a "ProcessingInstruction" which will not impact + # the "Element" structure of the resulting tree. + if inherit_branding: + node.addprevious(etree.ProcessingInstruction('apply-inheritance-specs-node-removal', node.tag)) + for child in spec: if child.get('position') == 'move': child = extract(child) - if inherit_branding and not replaced_node_tag and child.tag is not etree.Comment: - # To make a correct branding, we need to - # - know exactly which node has been replaced - # - store it before anything else has altered the Tree - # Do it exactly here :D - child.set('meta-oe-xpath-replacing', node.tag) - # We just store the replaced node tag on the first - # child of the xpath replacing it - replaced_node_tag = node.tag node.addprevious(child) node.getparent().remove(node) elif mode == "inner":