From 2d75013ff098c7faa9ec2dee93b1099e3bd35de5 Mon Sep 17 00:00:00 2001
From: qsm-odoo <qsm@odoo.com>
Date: Tue, 24 May 2022 17:14:44 +0000
Subject: [PATCH] [FIX] base: remove "removal branding" from the <head/> of
 rendered pages

Since [1], during the call of `apply_inheritance_specs` in charge of
resolving all the xpath of inheriting views and building the whole
primary view, processing instructions were added to mark the locations
where nodes were removed. Before that commit, another system was in
place where the following node next to the removal was marked via a
meta-oe-xpath-replacing attribute.

The problem with that change is that some of those processing
instructions were actually not removed once the branding was distributed
before the view was served... indeed all processing instructions which
were in the `<head/>` or a t-ignore area were ignored instead of removed.

This was actually already the case with the old system of the attribute
"meta-oe-xpath-replacing" but it was never noticed (as either the
attribute was there but had no effect or was indirectly gone if it
landed on a `<t>` node).

After [1], a bug was reported in 15.0 with the website configurator
where the processing instruction actually made some nonsense appear on
top of the page (at least without the work being discussed at [2] which
makes it so processing instructions act as HTML comments as it should
normally be the case).

[1]: https://github.com/odoo/odoo/commit/f67832a3ae0d9a3b5b53129132762e6bc1aed874
[2]: https://github.com/odoo/odoo/pull/92213

closes odoo/odoo#92316

X-original-commit: 4310d012a16f982c2336d084254b5c5617794ee2
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
---
 odoo/addons/base/models/ir_ui_view.py |  7 ++-
 odoo/addons/base/tests/test_views.py  | 67 +++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/odoo/addons/base/models/ir_ui_view.py b/odoo/addons/base/models/ir_ui_view.py
index b337f6a509b..c69f3f71bb3 100644
--- a/odoo/addons/base/models/ir_ui_view.py
+++ b/odoo/addons/base/models/ir_ui_view.py
@@ -1927,7 +1927,12 @@ actual arch.
                 if not attrs.intersection(descendant.attrib):
                     continue
                 self._pop_view_branding(descendant)
-            # TODO: find a better name and check if we have a string to boolean helper
+
+            # Remove the processing instructions indicating where nodes were
+            # removed (see apply_inheritance_specs)
+            for descendant in e.iterdescendants(tag=etree.ProcessingInstruction):
+                if descendant.target == 'apply-inheritance-specs-node-removal':
+                    descendant.getparent().remove(descendant)
             return
 
         node_path = e.get('data-oe-xpath')
diff --git a/odoo/addons/base/tests/test_views.py b/odoo/addons/base/tests/test_views.py
index c1579f15dd5..71fbb699e13 100644
--- a/odoo/addons/base/tests/test_views.py
+++ b/odoo/addons/base/tests/test_views.py
@@ -950,6 +950,73 @@ class TestTemplating(ViewCase):
             initial.get('data-oe-xpath'),
             "The node's xpath position should be correct")
 
+    def test_branding_inherit_remove_node_processing_instruction(self):
+        view1 = self.View.create({
+            'name': "Base view",
+            'type': 'qweb',
+            'arch': """
+                <html>
+                    <head>
+                        <hello></hello>
+                    </head>
+                    <body>
+                        <world></world>
+                    </body>
+                </html>
+            """
+        })
+        self.View.create({
+            'name': "Extension",
+            'type': 'qweb',
+            'inherit_id': view1.id,
+            'arch': """
+                <data>
+                    <xpath expr="//hello" position="replace"/>
+                    <xpath expr="//world" position="replace"/>
+                </data>
+            """
+        })
+
+        arch_string = view1.with_context(inherit_branding=True).get_combined_arch()
+        arch = etree.fromstring(arch_string)
+
+        head = arch.xpath('//head')[0]
+        head_child = head[0]
+        self.assertEqual(
+            head_child.target,
+            'apply-inheritance-specs-node-removal',
+            "A node was removed at the start of the <head>, a processing instruction should exist as first child node")
+        self.assertEqual(
+            head_child.text,
+            'hello',
+            "The processing instruction should mention the tag of the node that was removed")
+
+        body = arch.xpath('//body')[0]
+        body_child = body[0]
+        self.assertEqual(
+            body_child.target,
+            'apply-inheritance-specs-node-removal',
+            "A node was removed at the start of the <body>, a processing instruction should exist as first child node")
+        self.assertEqual(
+            body_child.text,
+            'world',
+            "The processing instruction should mention the tag of the node that was removed")
+
+        self.View.distribute_branding(arch)
+
+        # Test that both head and body have their processing instruction
+        # 'apply-inheritance-specs-node-removal' removed after branding
+        # distribution. Note: test head and body separately as the code in
+        # charge of the removal is different in each case.
+        self.assertEqual(
+            len(head),
+            0,
+            "The processing instruction of the <head> should have been removed")
+        self.assertEqual(
+            len(body),
+            0,
+            "The processing instruction of the <body> should have been removed")
+
     def test_branding_inherit_top_t_field(self):
         view1 = self.View.create({
             'name': "Base view",