[FIX] web, *: use correct DOM for m2m tags

* mail, calendar

- Fix traceback when clicking on a tag without color field (introduced
  in recent changes)
- Properly allow to delete tags on firefox (a button obviously cannot
  contain another button)

Closes https://github.com/odoo/odoo/pull/26547
Thanks to @hbh-odoo

4 files changed, 71 insertions, 55 deletions

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index 7683cd6db94..9f958ec57e0 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -1841,6 +1841,7 @@ var FieldMany2ManyTags = AbstractField.extend({
      * @param {MouseEvent} event
      */
     _onDeleteTag: function (event) {
+        event.preventDefault();
         event.stopPropagation();
         this._removeTag($(event.target).parent().data('id'));
     },
@@ -1886,7 +1887,7 @@ var FieldMany2ManyTags = AbstractField.extend({
 
 var FormFieldMany2ManyTags = FieldMany2ManyTags.extend({
     events: _.extend({}, FieldMany2ManyTags.prototype.events, {
-        'click .badge': '_onOpenColorPicker',
+        'click .dropdown-toggle': '_onOpenColorPicker',
         'mousedown .o_colorpicker a': '_onUpdateColor',
         'mousedown .o_colorpicker .o_hide_in_kanban': '_onUpdateColor',
         'focusout .o_colorpicker': '_onCloseColorPicker',
@@ -1907,8 +1908,9 @@ var FormFieldMany2ManyTags = FieldMany2ManyTags.extend({
      * @param {MouseEvent} ev
      */
     _onOpenColorPicker: function (ev) {
-        var tagID = $(ev.currentTarget).data('id');
-        var tagColor = $(ev.currentTarget).data('color');
+        ev.preventDefault();
+        var tagID = $(ev.currentTarget).parent().data('id');
+        var tagColor = $(ev.currentTarget).parent().data('color');
         var tag = _.findWhere(this.value.data, { res_id: tagID });
         if (tag && this.colorField in tag.data) { // if there is a color field on the related model
             this.$color_picker = $(qweb.render('FieldMany2ManyTag.colorpicker', {
@@ -1937,8 +1939,8 @@ var FormFieldMany2ManyTags = FieldMany2ManyTags.extend({
         var $target = $(ev.currentTarget);
         var color = $target.data('color');
         var id = $target.data('id');
-        var tag = this.$("button.badge[data-id='" + id + "']");
-        var currentColor = tag.data('color');
+        var $tag = this.$(".badge[data-id='" + id + "']");
+        var currentColor = $tag.data('color');
         var changes = {};
 
         if ($target.is('.o_hide_in_kanban')) {
diff --git a/addons/web/static/src/scss/fields.scss b/addons/web/static/src/scss/fields.scss
index 3681c38eb47..f85fd04d505 100644
--- a/addons/web/static/src/scss/fields.scss
+++ b/addons/web/static/src/scss/fields.scss
@@ -99,15 +99,21 @@
             flex: 0 0 auto;
             border: 0;
             font-size: 12px;
-            color: white;
             cursor: pointer;
+            user-select: none;
+
+            a {
+                color: inherit;
+            }
 
-            > .o_badge_text {
+            .o_badge_text {
                 @include o-text-overflow(inline-block);
                 max-width: 200px;
+                color: inherit;
             }
 
             .o_delete {
+                color: inherit;
                 cursor: pointer;
             }
         }
@@ -168,13 +174,15 @@
                 } @else {
                     &, &::after {
                         background-color: nth($o-colors, $size);
+                        color: $white;
                     }
                 }
             }
         }        
-        > .o_tag_error{ 
+        .o_tag_error {
             box-shadow: inset 0 0 0 2px #FF0000;
-            > .o_delete {
+
+            .o_delete {
                 color: #FF0000;
             }
         }
diff --git a/addons/web/static/src/xml/base.xml b/addons/web/static/src/xml/base.xml
index 9b6ef0099d5..e1c74db4645 100644
--- a/addons/web/static/src/xml/base.xml
+++ b/addons/web/static/src/xml/base.xml
@@ -833,11 +833,19 @@
     <t t-foreach="elements" t-as="el">
         <t t-set="color" t-value="el[colorField] || 0"/>
         <t t-set="colornames" t-value="['No color', 'Red', 'Orange', 'Yellow', 'Light blue', 'Dark purple', 'Salmon pink', 'Medium blue', 'Dark blue', 'Fushia', 'Green', 'Purple']"/>
-        <div class="dropdown">
-            <button t-attf-class="badge badge-pill dropdown-toggle o-no-caret o_tag_color_#{color}" data-toggle="dropdown" t-att-data-color="color" t-att-data-index="el_index" t-att-data-id="el.id" t-attf-title="Tag color: #{colornames[color]}">
+        <div t-attf-class="badge badge-pill dropdown o_tag_color_#{color}" t-att-data-color="color" t-att-data-index="el_index" t-att-data-id="el.id" t-attf-title="Tag color: #{colornames[color]}">
+            <t t-set="_badge_text">
                 <span class="o_badge_text" t-att-title="el.display_name"><span role="img" t-attf-aria-label="Tag color: #{colornames[color]}"/><t t-esc="el.display_name"/></span>
-                <a t-if="!readonly" class="fa fa-times o_delete" title="Delete" aria-label="Delete"/>
-            </button>
+            </t>
+            <t t-if="colorField">
+                <a href="#" class="dropdown-toggle o-no-caret" data-toggle="dropdown">
+                    <t t-raw="_badge_text"/>
+                </a>
+            </t>
+            <t t-else="">
+                <t t-raw="_badge_text"/>
+            </t>
+            <a t-if="!readonly" href="#" class="fa fa-times o_delete" title="Delete" aria-label="Delete"/>
         </div>
     </t>
 </t>
diff --git a/addons/web/static/tests/fields/relational_fields_tests.js b/addons/web/static/tests/fields/relational_fields_tests.js
index 77b2c403b8c..390190ca02c 100644
--- a/addons/web/static/tests/fields/relational_fields_tests.js
+++ b/addons/web/static/tests/fields/relational_fields_tests.js
@@ -11738,9 +11738,9 @@ QUnit.module('relational_fields', {
         assert.strictEqual($input.autocomplete('widget').find('li').length, 3,
             "autocomplete dropdown should have 3 entries (2 values + 'Search and Edit...')");
         $input.autocomplete('widget').find('li:first()').click(); // adds a tag
-        assert.strictEqual(form.$('.o_field_many2manytags[name="timmy"] > .dropdown > button').length, 1,
+        assert.strictEqual(form.$('.o_field_many2manytags[name="timmy"] .badge').length, 1,
             "should contain 1 tag");
-        assert.ok(form.$('.o_field_many2manytags[name="timmy"] > .dropdown > button:contains("gold")').length,
+        assert.ok(form.$('.o_field_many2manytags[name="timmy"] .badge:contains("gold")').length,
             "should contain newly added tag 'gold'");
 
         form.destroy();
@@ -11773,18 +11773,18 @@ QUnit.module('relational_fields', {
                 return this._super.apply(this, arguments);
             },
         });
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 2,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge .dropdown-toggle').length, 2,
             "should contain 2 tags");
-        assert.ok(form.$('button:contains(gold)').length,
+        assert.ok(form.$('.badge .dropdown-toggle:contains(gold)').length,
             'should have fetched and rendered gold partner tag');
-        assert.ok(form.$('button:contains(silver)').length,
+        assert.ok(form.$('.badge .dropdown-toggle:contains(silver)').length,
             'should have fetched and rendered silver partner tag');
-        assert.strictEqual(form.$('button:first()').data('color'), 2,
+        assert.strictEqual(form.$('.badge:first()').data('color'), 2,
             'should have correctly fetched the color');
 
         form.$buttons.find('.o_form_button_edit').click();
 
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 2,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge .dropdown-toggle').length, 2,
             "should still contain 2 tags in edit mode");
         assert.ok(form.$('.o_tag_color_2 .o_badge_text:contains(gold)').length,
             'first tag should still contain "gold" and be color 2 in edit mode');
@@ -11799,46 +11799,44 @@ QUnit.module('relational_fields', {
         assert.strictEqual($input.autocomplete('widget').find('li a:contains("red")').length, 1,
             "autocomplete dropdown should contain 'red'");
         $input.autocomplete('widget').find('li').click(); // add 'red'
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 3,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge .dropdown-toggle').length, 3,
             "should contain 3 tags");
-        assert.ok(form.$('.o_field_many2manytags > .dropdown > button:contains("red")').length,
+        assert.ok(form.$('.o_field_many2manytags .badge .dropdown-toggle:contains("red")').length,
             "should contain newly added tag 'red'");
-        assert.ok(form.$('.o_field_many2manytags > .dropdown > button[data-color=8]:contains("red")').length,
+        assert.ok(form.$('.o_field_many2manytags .badge[data-color=8] .dropdown-toggle:contains("red")').length,
             "should have fetched the color of added tag");
 
         // remove tag with id 14
-        form.$('.o_field_many2manytags button[data-id=14] .o_delete').click();
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 2,
+        form.$('.o_field_many2manytags .badge[data-id=14] .o_delete').click();
+        assert.strictEqual(form.$('.o_field_many2manytags .badge .dropdown-toggle').length, 2,
             "should contain 2 tags");
-        assert.ok(!form.$('.o_field_many2manytags > .dropdown > button:contains("silver")').length,
+        assert.ok(!form.$('.o_field_many2manytags .badge .dropdown-toggle:contains("silver")').length,
             "should not contain tag 'silver' anymore");
 
         // save the record (should do the write RPC with the correct commands)
         form.$buttons.find('.o_form_button_save').click();
 
         // checkbox 'Hide in Kanban'
-        $input = form.$('.o_field_many2manytags button[data-id=13]'); // selects 'red' tag
+        $input = form.$('.o_field_many2manytags .badge[data-id=13] .dropdown-toggle'); // selects 'red' tag
         $input.click(); // opens the colorpicker dropdown
-        assert.ok(form.$('.o_field_many2manytags button[data-id=13] .o_colorpicker .custom-checkbox'), "should have a checkbox in the colorpicker dropdown menu");
-
-        $input.click();
-        var $checkBox = form.$('.o_field_many2manytags button[data-id=13]').next().find('.custom-checkbox input');
+        var $checkBox = form.$('.o_field_many2manytags .badge[data-id=13] .custom-checkbox input');
+        assert.strictEqual($checkBox.length, 1, "should have a checkbox in the colorpicker dropdown menu");
         assert.notOk($checkBox.is(':checked'), "should have unticked checkbox in colorpicker dropdown menu");
 
-        $checkBox.mousedown();
-        $input = form.$('.o_field_many2manytags button[data-id=13]'); // refresh
-        assert.equal($input.data('color'), "0", "should become transparent when toggling on checkbox");
+        $checkBox.mousedown().click();
 
+        $input = form.$('.o_field_many2manytags .badge[data-id=13] .dropdown-toggle'); // refresh
         $input.click();
-        $checkBox = form.$('.o_field_many2manytags button[data-id=13]').next().find('.custom-checkbox input'); // refresh
+        $checkBox = form.$('.o_field_many2manytags .badge[data-id=13] .custom-checkbox input'); // refresh
+        assert.equal($input.parent().data('color'), "0", "should become transparent when toggling on checkbox");
         assert.ok($checkBox.is(':checked'), "should have a ticked checkbox in colorpicker dropdown menu after mousedown");
 
-        $checkBox.mousedown();
-        $input = form.$('.o_field_many2manytags button[data-id=13]'); // refresh
-        assert.equal($input.data('color'), "8", "should revert to old color when toggling off checkbox");
+        $checkBox.mousedown().click();
 
+        $input = form.$('.o_field_many2manytags .badge[data-id=13] .dropdown-toggle'); // refresh
         $input.click();
-        $checkBox = form.$('.o_field_many2manytags button[data-id=13]').next().find('.custom-checkbox input'); // refresh
+        $checkBox = form.$('.o_field_many2manytags .badge[data-id=13] .custom-checkbox input'); // refresh
+        assert.equal($input.parent().data('color'), "8", "should revert to old color when toggling off checkbox");
         assert.notOk($checkBox.is(':checked'), "should have an unticked checkbox in colorpicker dropdown menu after 2nd click");
 
         // TODO: it would be nice to test the behaviors of the autocomplete dropdown
@@ -11871,9 +11869,9 @@ QUnit.module('relational_fields', {
                 return this._super.apply(this, arguments);
             }
         });
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 1,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 1,
             "should contain 1 tag");
-        assert.ok(form.$('button:contains(gold)').length,
+        assert.ok(form.$('.badge:contains(gold)').length,
             'should have fetched and rendered gold partner tag');
 
         form.$buttons.find('.o_form_button_edit').click();
@@ -11886,9 +11884,9 @@ QUnit.module('relational_fields', {
         assert.strictEqual($input.autocomplete('widget').find('li a:contains("silver")').length, 1,
             "autocomplete dropdown should contain 'silver'");
         $input.autocomplete('widget').find('li').click(); // add 'silver'
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 2,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 2,
             "should contain 2 tags");
-        assert.ok(form.$('.o_field_many2manytags > .dropdown > button:contains("silver")').length,
+        assert.ok(form.$('.o_field_many2manytags .badge:contains("silver")').length,
             "should contain newly added tag 'silver'");
 
         form.destroy();
@@ -11921,9 +11919,9 @@ QUnit.module('relational_fields', {
         assert.strictEqual($input.autocomplete('widget').find('li').length, 3,
             "autocomplete dropdown should have 3 entries (2 values + 'Search and Edit...')");
         $input.autocomplete('widget').find('li:first()').click(); // adds a tag
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 1,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 1,
             "should contain 1 tag");
-        assert.ok(form.$('.o_field_many2manytags > .dropdown > button:contains("gold")').length,
+        assert.ok(form.$('.o_field_many2manytags .badge:contains("gold")').length,
             "should contain newly added tag 'gold'");
 
         // save the record (should do the write RPC with the correct commands)
@@ -11948,20 +11946,20 @@ QUnit.module('relational_fields', {
         });
 
         // First checks that default color 0 is rendered as 0 color
-        assert.ok(form.$('button:first()').is('.o_tag_color_0'),
+        assert.ok(form.$('.badge.dropdown:first()').is('.o_tag_color_0'),
             'first tag color should be 0');
 
         // Update the color in readonly
-        form.$('button:first()').click();
+        form.$('.badge:first() .dropdown-toggle').click();
         $('.o_colorpicker a[data-color="1"]').trigger('mousedown'); // choose color 1
-        assert.strictEqual(form.$('button:first()').data('color'), 1,
+        assert.strictEqual(form.$('.badge:first()').data('color'), 1,
             'should have correctly updated the color (in readonly)');
 
         // Update the color in edit
         form.$buttons.find('.o_form_button_edit').click();
-        form.$('button:first()').click();
+        form.$('.badge:first() .dropdown-toggle').click();
         $('.o_colorpicker a[data-color="6"]').trigger('mousedown'); // choose color 6
-        assert.strictEqual(form.$('button:first()').data('color'), 6,
+        assert.strictEqual(form.$('.badge:first()').data('color'), 6,
             'should have correctly updated the color (in edit)');
         form.destroy();
     });
@@ -12099,7 +12097,7 @@ QUnit.module('relational_fields', {
                 '</form>',
             res_id: 1,
         });
-        assert.strictEqual(form.$('.o_field_widget[name="partner_ids"] > .dropdown > button').length, 100,
+        assert.strictEqual(form.$('.o_field_widget[name="partner_ids"] .badge').length, 100,
             'should have rendered 100 tags');
         form.destroy();
     });
@@ -12124,14 +12122,14 @@ QUnit.module('relational_fields', {
         });
 
         form.$buttons.find('.o_form_button_edit').click();
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 1,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 1,
             "should contain one tag");
 
         // update foo, which will trigger an onchange and update timmy
         // -> m2mtags input should not have taken the focus
         form.$('input:first').focus();
         form.$('input:first').val('trigger onchange').trigger('input');
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 0,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 0,
             "should contain no tags");
         assert.strictEqual(form.$('input:first').get(0), document.activeElement,
             "foo input should have kept the focus");
@@ -12139,14 +12137,14 @@ QUnit.module('relational_fields', {
         // add a tag -> m2mtags input should still have the focus
         form.$('.o_field_many2manytags input').click(); // opens the dropdown
         form.$('.o_field_many2manytags input').autocomplete('widget').find('li:first').click();
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 1,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 1,
             "should contain a tag");
         assert.strictEqual(form.$('.o_field_many2manytags input').get(0), document.activeElement,
             "m2m tags input should have kept the focus");
 
         // remove a tag -> m2mtags input should still have the focus
         form.$('.o_field_many2manytags .o_delete').click();
-        assert.strictEqual(form.$('.o_field_many2manytags > .dropdown > button').length, 0,
+        assert.strictEqual(form.$('.o_field_many2manytags .badge').length, 0,
             "should contain no tags");
         assert.strictEqual(form.$('.o_field_many2manytags input').get(0), document.activeElement,
             "m2m tags input should have kept the focus");