From bdabc55280e78025ea29299095cb526b9297c086 Mon Sep 17 00:00:00 2001
From: Samuel Degueldre <sad@odoo.com>
Date: Thu, 18 Aug 2022 05:44:30 +0000
Subject: [PATCH] [FIX] web: fix label with empty string rendering with default
 label

In legacy, when you have a label with a string attribute that is empty,
that label is rendered as empty. In the new form view, when the string
attribute was empty we would fall back to the default label for that
field, which is incorrect.

This commit fixes that by not rendering labels that have an empty string
at all.

closes odoo/odoo#98237

Signed-off-by: Mathieu Duckerts-Antoine <dam@odoo.com>
---
 .../static/src/views/form/form_compiler.js    | 15 +++-
 .../settings_form_compiler.js                 |  2 +-
 .../static/tests/legacy/views/form_tests.js   | 23 ++++++
 .../tests/views/form/form_compiler_tests.js   | 11 +++
 .../tests/views/form/form_view_tests.js       | 73 ++++++++++++++++++-
 .../settings_form_view_tests.js               | 53 ++++++++++++++
 6 files changed, 173 insertions(+), 4 deletions(-)

diff --git a/addons/web/static/src/views/form/form_compiler.js b/addons/web/static/src/views/form/form_compiler.js
index a95ce979535..cfdc71d85dd 100644
--- a/addons/web/static/src/views/form/form_compiler.js
+++ b/addons/web/static/src/views/form/form_compiler.js
@@ -79,6 +79,9 @@ export class FormCompiler extends ViewCompiler {
             fieldInfo: `props.archInfo.fieldNodes['${fieldId}']`,
             className: `"${label.className}"`,
         };
+        if (label.hasAttribute("data-no-label")) {
+            return;
+        }
         let labelText = label.textContent || fieldString;
         labelText = labelText
             ? toStringExpression(labelText)
@@ -173,7 +176,11 @@ export class FormCompiler extends ViewCompiler {
                 label,
                 params
             );
-            label.replaceWith(formLabel);
+            if (formLabel) {
+                label.replaceWith(formLabel);
+            } else {
+                label.remove();
+            }
             return formLabel;
         };
         for (const label of labels) {
@@ -405,6 +412,8 @@ export class FormCompiler extends ViewCompiler {
             const string = el.getAttribute("string");
             if (string) {
                 append(label, createTextNode(string));
+            } else if (string === "") {
+                label.setAttribute("data-no-label", "true");
             }
             if (this.encounteredFields[forAttr]) {
                 label = this.encounteredFields[forAttr](label);
@@ -461,7 +470,9 @@ export class FormCompiler extends ViewCompiler {
 
             for (const anchor of child.querySelectorAll("[href^=\\#]")) {
                 const anchorValue = CSS.escape(anchor.getAttribute("href").substring(1));
-                if (!anchorValue.length) continue;
+                if (!anchorValue.length) {
+                    continue;
+                }
                 pageAnchors.push(anchorValue);
                 noteBookAnchors[anchorValue] = {
                     origin: `'${pageId}'`,
diff --git a/addons/web/static/src/webclient/settings_form_view/settings_form_compiler.js b/addons/web/static/src/webclient/settings_form_view/settings_form_compiler.js
index 3af48890c34..a861bda7262 100644
--- a/addons/web/static/src/webclient/settings_form_view/settings_form_compiler.js
+++ b/addons/web/static/src/webclient/settings_form_view/settings_form_compiler.js
@@ -247,7 +247,7 @@ export class SettingsFormCompiler extends FormCompiler {
             label.textContent = labelweak.textContent;
         }
         const res = super.createLabelFromField(fieldId, fieldName, fieldString, label, params);
-        if (labelweak) {
+        if (labelweak || label.hasAttribute("data-no-label")) {
             // the work of pushing the label in the search structure is already done
             return res;
         }
diff --git a/addons/web/static/tests/legacy/views/form_tests.js b/addons/web/static/tests/legacy/views/form_tests.js
index 9480ebef5ec..d3504e29266 100644
--- a/addons/web/static/tests/legacy/views/form_tests.js
+++ b/addons/web/static/tests/legacy/views/form_tests.js
@@ -1419,6 +1419,29 @@ QUnit.module('LegacyViews', {
         form.destroy();
     });
 
+    QUnit.test("label ignores the content of the label when present", async function (assert) {
+        await createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: `
+                <form>
+                    <sheet>
+                        <group>
+                            <label for="bar">customstring</label>
+                            <div>
+                                <field name="bar"/>
+                            </div>
+                        </group>
+                    </sheet>
+                </form>`,
+            res_id: 2,
+        });
+
+        assert.containsOnce(target, "label.o_form_label");
+        assert.strictEqual(target.querySelector("label.o_form_label").textContent, "Bar");
+    });
+
     QUnit.test('input ids for multiple occurrences of fields in form view', async function (assert) {
         // A same field can occur several times in the view, but its id must be
         // unique by occurrence, otherwise there is a warning in the console (in
diff --git a/addons/web/static/tests/views/form/form_compiler_tests.js b/addons/web/static/tests/views/form/form_compiler_tests.js
index 7f4c7b4c00a..3f4a09d60fe 100644
--- a/addons/web/static/tests/views/form/form_compiler_tests.js
+++ b/addons/web/static/tests/views/form/form_compiler_tests.js
@@ -41,6 +41,17 @@ QUnit.module("Form Compiler", () => {
         assert.areEquivalent(compileTemplate(arch), expected);
     });
 
+    QUnit.test("label with empty string is not rendered", async (assert) => {
+        const arch = /*xml*/ `<form><field name="test"/><label for="test" string=""/></form>`;
+        const expected = /*xml*/ `
+            <t>
+                <div t-att-class="props.class" t-attf-class="{{props.record.isInEdition ? 'o_form_editable' : 'o_form_readonly'}} d-block" class="o_form_nosheet" t-ref="compiled_view_root">
+                    <Field id="'test'" name="'test'" record="props.record" fieldInfo="props.archInfo.fieldNodes['test']"/>
+                </div>
+            </t>`;
+        assert.areEquivalent(compileTemplate(arch), expected);
+    });
+
     QUnit.test("properly compile simple div with field", async (assert) => {
         const arch = /*xml*/ `<form><div class="someClass">lol<field name="display_name"/></div></form>`;
         const expected = /*xml*/ `
diff --git a/addons/web/static/tests/views/form/form_view_tests.js b/addons/web/static/tests/views/form/form_view_tests.js
index 9745c07d87d..3fc69349bfe 100644
--- a/addons/web/static/tests/views/form/form_view_tests.js
+++ b/addons/web/static/tests/views/form/form_view_tests.js
@@ -1617,7 +1617,33 @@ QUnit.module("Views", (hooks) => {
         );
     });
 
-    QUnit.test("label uses the string attribute", async function (assert) {
+    QUnit.test(
+        "label with no string attribute gets the default label for the corresponding field",
+        async function (assert) {
+            await makeView({
+                type: "form",
+                resModel: "partner",
+                serverData,
+                arch: `
+                <form>
+                    <sheet>
+                        <group>
+                            <label for="bar"/>
+                            <div>
+                                <field name="bar"/>
+                            </div>
+                        </group>
+                    </sheet>
+                </form>`,
+                resId: 2,
+            });
+
+            assert.containsOnce(target, "label.o_form_label");
+            assert.strictEqual(target.querySelector("label.o_form_label").textContent, "Bar");
+        }
+    );
+
+    QUnit.test("label uses the string attribute when present", async function (assert) {
         await makeView({
             type: "form",
             resModel: "partner",
@@ -1640,6 +1666,51 @@ QUnit.module("Views", (hooks) => {
         assert.strictEqual(target.querySelector("label.o_form_label").textContent, "customstring");
     });
 
+    QUnit.test("label ignores the content of the label when present", async function (assert) {
+        await makeView({
+            type: "form",
+            resModel: "partner",
+            serverData,
+            arch: `
+                <form>
+                    <sheet>
+                        <group>
+                            <label for="bar">customstring</label>
+                            <div>
+                                <field name="bar"/>
+                            </div>
+                        </group>
+                    </sheet>
+                </form>`,
+            resId: 2,
+        });
+
+        assert.containsOnce(target, "label.o_form_label");
+        assert.strictEqual(target.querySelector("label.o_form_label").textContent, "Bar");
+    });
+
+    QUnit.test("label with empty string attribute doesn't get rendered", async function (assert) {
+        await makeView({
+            type: "form",
+            resModel: "partner",
+            serverData,
+            arch: `
+                <form>
+                    <sheet>
+                        <group>
+                            <label for="bar" string=""/>
+                            <div>
+                                <field name="bar"/>
+                            </div>
+                        </group>
+                    </sheet>
+                </form>`,
+            resId: 2,
+        });
+
+        assert.containsNone(target, "label.o_form_label");
+    });
+
     QUnit.test(
         "label is not rendered when invisible and not at top-level in a group",
         async function (assert) {
diff --git a/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js b/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js
index febe8d2cead..952f9af8bf7 100644
--- a/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js
+++ b/addons/web/static/tests/webclient/settings_form_view/settings_form_view_tests.js
@@ -554,6 +554,59 @@ QUnit.module("SettingsFormView", (hooks) => {
         }
     );
 
+    QUnit.test("search for default label when label has empty string", async function (assert) {
+        serverData.actions = {
+            1: {
+                id: 1,
+                name: "Settings view",
+                res_model: "res.config.settings",
+                type: "ir.actions.act_window",
+                views: [[1, "form"]],
+            },
+            4: {
+                id: 4,
+                name: "Other action",
+                res_model: "task",
+                type: "ir.actions.act_window",
+                views: [[2, "list"]],
+            },
+        };
+
+        serverData.views = {
+            "res.config.settings,1,form": `
+                    <form string="Settings" js_class="base_settings">
+                        <div class="settings">
+                            <div class="app_settings_block" string="CRM" data-key="crm">
+                                <div class="row mt16 o_settings_container">
+                                    <div class="col-12 col-lg-6 o_setting_box">
+                                        <div class="o_setting_left_pane">
+                                            <label for="foo" string=""/>
+                                            <field name="foo"/>
+                                        </div>
+                                    </div>
+                                </div>
+                            </div>
+                        </div>
+                    </form>`,
+            "task,2,list": `
+                    <tree>
+                        <field name="display_name"/>
+                    </tree>`,
+            "res.config.settings,false,search": "<search></search>",
+            "task,false,search": "<search></search>",
+        };
+
+        const webClient = await createWebClient({ serverData });
+
+        await doAction(webClient, 1);
+        assert.containsNone(target, ".o_form_label");
+        assert.containsNone(target, ".settingSearchHeader");
+        await editSearch(target, "Fo");
+        await execTimeouts();
+        assert.containsNone(target, ".o_form_label");
+        assert.containsNone(target, ".settingSearchHeader");
+    });
+
     QUnit.test(
         "clicking on any button in setting should show discard warning if setting form is dirty",
         async function (assert) {