[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:
```
<hello>
    <world></world>
    <world></world>
    <t t-esc="foo"/>
</hello>
```

Child view:
```
<data>
    <xpath expr="/hello/world[1]" position="replace"/>
</data>
```

=> There are two <world/> in the original view, the first one is removed
   by a child view. The data-oe-xpath set on the remaining <world/>
   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

@@ -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(

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
+            # <world/> elements from the start
+            'arch': """
+                <hello>
+                    <world></world>
+                    <world></world>
+
+                    <t t-esc="foo"/>
+                </hello>
+            """
+        })
+        self.View.create({
+            'name': "Extension",
+            'type': 'qweb',
+            'inherit_id': view1.id,
+            'arch': """
+                <data>
+                    <xpath expr="/hello/world[1]" position="replace"/>
+                </data>
+            """
+        })
+
+        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",

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":