From 2689915e3f47686f69729b95216acf9e930740fb Mon Sep 17 00:00:00 2001
From: ijas ahammed <ija@odoo.com>
Date: Fri, 6 Nov 2020 10:04:01 +0000
Subject: [PATCH] [IMP] web,sms: limit the clickable area for email/phone/url
 widgets

The 'o_text_overflow' class whlie provided on any field, is used to
have classic ellipsis (...) for very long string. However, it is
implemented the way that it occupies the available width for the field
tag it even if the the string is small.

The same class is also utilized in some field widgets like  email, phone,
URL etc. But it has a small side-effect due to full width occupancy.

Because the above widgets rendered an anchor tag, the clickable area is on
the whole available width instead of simply on the content provided in anchor
tag. So even if user clicks on the empty area of that field, the click
action is performed (for example, in email field widget, default mail client
pops-up) which should not happen. It should behave like clickable m2o fields
where the action is performed only when clicking the content and not on the
empty area.

With this commit, we wrap the anchor tag, within a container div tag. Here,
the overflow class will be on the container div which will do it's job to
prevent the long strings from breaking the UI, and the anchor tag being its
child, will not be the full width, thus limiting the clickable area. This
commit also makes the related test cases more robust by checking proper
classes for particular widgets.

TaskId - 2345974

closes odoo/odoo#63735

Related: odoo/enterprise#15777
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
---
 .../sms/static/src/js/fields_phone_widget.js  |  2 +-
 addons/sms/static/tests/sms_widget_test.js    |  4 +-
 .../web/static/src/js/fields/basic_fields.js  | 30 ++++++-----
 .../tests/fields/basic_fields_mobile_tests.js |  8 +--
 .../static/tests/fields/basic_fields_tests.js | 52 +++++++++----------
 addons/web/static/tests/views/form_tests.js   |  2 +-
 6 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/addons/sms/static/src/js/fields_phone_widget.js b/addons/sms/static/src/js/fields_phone_widget.js
index 82b8e5b3f7f..8e76124f164 100644
--- a/addons/sms/static/src/js/fields_phone_widget.js
+++ b/addons/sms/static/src/js/fields_phone_widget.js
@@ -33,7 +33,7 @@ Phone.include({
      */
     getFocusableElement() {
         if (this.enableSMS && this.mode === 'readonly') {
-            return this.$el.filter('.' + this.className);
+            return this.$el.filter('.' + this.className).find('a');
         }
         return this._super.apply(this, arguments);
     },
diff --git a/addons/sms/static/tests/sms_widget_test.js b/addons/sms/static/tests/sms_widget_test.js
index 467cd0770de..8d27965b9c1 100644
--- a/addons/sms/static/tests/sms_widget_test.js
+++ b/addons/sms/static/tests/sms_widget_test.js
@@ -148,7 +148,7 @@ QUnit.module('fields', {
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector)').first().text(), 'yopSMS',
             "value should be displayed properly with a link to send SMS");
 
-        assert.containsN(list, 'a.o_field_widget.o_form_uri', 2,
+        assert.containsN(list, 'div.o_field_widget.o_form_uri.o_field_phone > a', 2,
             "should have the correct classnames");
 
         // Edit a line and check the result
@@ -165,7 +165,7 @@ QUnit.module('fields', {
         assert.doesNotHaveClass($cell.parent(), 'o_selected_row', 'should not be in edit mode anymore');
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector)').first().text(), 'newSMS',
             "value should be properly updated");
-        assert.containsN(list, 'a.o_field_widget.o_form_uri', 2,
+        assert.containsN(list, 'div.o_field_widget.o_form_uri.o_field_phone > a', 2,
             "should still have links with correct classes");
 
         await testUtils.dom.click(list.$('tbody td:not(.o_list_record_selector) .o_field_phone_sms').first());
diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js
index 2f20b3cde46..93b82312b89 100644
--- a/addons/web/static/src/js/fields/basic_fields.js
+++ b/addons/web/static/src/js/fields/basic_fields.js
@@ -1560,7 +1560,7 @@ var FieldEmail = InputField.extend({
      */
     init: function () {
         this._super.apply(this, arguments);
-        this.tagName = this.mode === 'readonly' ? 'a' : 'input';
+        this.tagName = this.mode === 'readonly' ? 'div' : 'input';
     },
 
     //--------------------------------------------------------------------------
@@ -1573,7 +1573,7 @@ var FieldEmail = InputField.extend({
      * @override
      */
     getFocusableElement: function () {
-        return this.mode === 'readonly' ? this.$el : this._super.apply(this, arguments);
+        return this.mode === 'readonly' ? this.$el.find('a') : this._super.apply(this, arguments);
     },
 
     //--------------------------------------------------------------------------
@@ -1588,11 +1588,12 @@ var FieldEmail = InputField.extend({
      */
     _renderReadonly: function () {
         if (this.value) {
-            this.$el.text(this.value)
-                .addClass('o_form_uri o_text_overflow')
-                .attr('href', this.prefix + ':' + this.value);
-        } else {
-            this.$el.text('');
+            this.el.classList.add("o_form_uri", "o_text_overflow");
+            const anchorEl = Object.assign(document.createElement('a'), {
+                text: this.value,
+                href: `${this.prefix}:${this.value}`,
+            });
+            this.el.appendChild(anchorEl);
         }
     },
     /**
@@ -1664,7 +1665,7 @@ var UrlWidget = InputField.extend({
      */
     init: function () {
         this._super.apply(this, arguments);
-        this.tagName = this.mode === 'readonly' ? 'a' : 'input';
+        this.tagName = this.mode === 'readonly' ? 'div' : 'input';
         this.websitePath = this.nodeOptions.website_path || false;
     },
 
@@ -1678,7 +1679,7 @@ var UrlWidget = InputField.extend({
      * @override
      */
     getFocusableElement: function () {
-        return this.mode === 'readonly' ? this.$el : this._super.apply(this, arguments);
+        return this.mode === 'readonly' ? this.$el.find('a') : this._super.apply(this, arguments);
     },
 
     //--------------------------------------------------------------------------
@@ -1698,10 +1699,13 @@ var UrlWidget = InputField.extend({
             const regex = /^(?:[fF]|[hH][tT])[tT][pP][sS]?:\/\//;
             href = !regex.test(this.value) ? `http://${href}` : href;
         }
-        this.$el.text(this.attrs.text || this.value)
-            .addClass('o_form_uri o_text_overflow')
-            .attr('target', '_blank')
-            .attr('href', href);
+        this.el.classList.add("o_form_uri", "o_text_overflow");
+        const anchorEl = Object.assign(document.createElement('a'), {
+            text: this.attrs.text || this.value,
+            href: href,
+            target: '_blank',
+        });
+        this.el.appendChild(anchorEl);
     },
 
     //--------------------------------------------------------------------------
diff --git a/addons/web/static/tests/fields/basic_fields_mobile_tests.js b/addons/web/static/tests/fields/basic_fields_mobile_tests.js
index 4b8b435378d..c6d91ab8461 100644
--- a/addons/web/static/tests/fields/basic_fields_mobile_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_mobile_tests.js
@@ -70,7 +70,7 @@ QUnit.module('basic_fields', {
             res_id: 1,
         });
 
-        var $phoneLink = form.$('a.o_form_uri.o_field_widget');
+        var $phoneLink = form.$('div.o_form_uri.o_field_widget.o_field_phone > a');
         assert.strictEqual($phoneLink.length, 1,
             "should have a anchor with correct classes");
         assert.strictEqual($phoneLink.text(), 'yop',
@@ -90,7 +90,7 @@ QUnit.module('basic_fields', {
 
         // save
         await testUtils.form.clickSave(form);
-        $phoneLink = form.$('a.o_form_uri.o_field_widget');
+        $phoneLink = form.$('div.o_form_uri.o_field_widget.o_field_phone > a');
         assert.strictEqual($phoneLink.text(), 'new',
             "new value should be displayed properly");
         assert.hasAttrValue($phoneLink, 'href', 'tel:new',
@@ -114,7 +114,7 @@ QUnit.module('basic_fields', {
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector) a').first().text(), 'yop',
             "value should be displayed properly");
 
-        var $phoneLink = list.$('a.o_form_uri.o_field_widget');
+        var $phoneLink = list.$('div.o_form_uri.o_field_widget.o_field_phone > a');
         assert.strictEqual($phoneLink.length, 3,
             "should have anchors with correct classes");
         assert.hasAttrValue($phoneLink.first(), 'href', 'tel:yop',
@@ -134,7 +134,7 @@ QUnit.module('basic_fields', {
         assert.doesNotHaveClass($cell.parent(), 'o_selected_row', 'should not be in edit mode anymore');
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector) a').first().text(), 'new',
             "value should be properly updated");
-        $phoneLink = list.$('a.o_form_uri.o_field_widget');
+        $phoneLink = list.$('div.o_form_uri.o_field_widget.o_field_phone > a');
         assert.strictEqual($phoneLink.length, 3,
             "should still have anchors with correct classes");
         assert.hasAttrValue($phoneLink.first(), 'href', 'tel:new',
diff --git a/addons/web/static/tests/fields/basic_fields_tests.js b/addons/web/static/tests/fields/basic_fields_tests.js
index 05dab6201ad..768ba833547 100644
--- a/addons/web/static/tests/fields/basic_fields_tests.js
+++ b/addons/web/static/tests/fields/basic_fields_tests.js
@@ -1101,7 +1101,7 @@ QUnit.module('basic_fields', {
             res_id: 1,
         });
 
-        var $mailtoLink = form.$('a.o_form_uri.o_field_widget.o_text_overflow');
+        var $mailtoLink = form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_email > a');
         assert.strictEqual($mailtoLink.length, 1,
             "should have a anchor with correct classes");
         assert.strictEqual($mailtoLink.text(), 'yop',
@@ -1121,7 +1121,7 @@ QUnit.module('basic_fields', {
 
         // save
         await testUtils.form.clickSave(form);
-        $mailtoLink = form.$('a.o_form_uri.o_field_widget.o_text_overflow');
+        $mailtoLink = form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_email > a');
         assert.strictEqual($mailtoLink.text(), 'new',
             "new value should be displayed properly");
         assert.hasAttrValue($mailtoLink, 'href', 'mailto:new',
@@ -1145,7 +1145,7 @@ QUnit.module('basic_fields', {
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector)').first().text(), 'yop',
             "value should be displayed properly as text");
 
-        var $mailtoLink = list.$('a.o_form_uri.o_field_widget.o_text_overflow');
+        var $mailtoLink = list.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_email > a');
         assert.strictEqual($mailtoLink.length, 5,
             "should have anchors with correct classes");
         assert.hasAttrValue($mailtoLink.first(), 'href', 'mailto:yop',
@@ -1165,7 +1165,7 @@ QUnit.module('basic_fields', {
         assert.doesNotHaveClass($cell.parent(), 'o_selected_row', 'should not be in edit mode anymore');
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector)').first().text(), 'new',
             "value should be properly updated");
-        $mailtoLink = list.$('a.o_form_uri.o_field_widget.o_text_overflow');
+        $mailtoLink = list.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_email > a');
         assert.strictEqual($mailtoLink.length, 5,
             "should still have anchors with correct classes");
         assert.hasAttrValue($mailtoLink.first(), 'href', 'mailto:new',
@@ -1191,7 +1191,7 @@ QUnit.module('basic_fields', {
             res_id: 1,
         });
 
-        var $mailtoLink = form.$('a.o_form_uri.o_field_widget.o_text_overflow');
+        var $mailtoLink = form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_email > a');
         assert.strictEqual($mailtoLink.text(), '',
             "the value should be displayed properly");
 
@@ -1938,13 +1938,13 @@ QUnit.module('basic_fields', {
             res_id: 1,
         });
 
-        assert.containsOnce(form, 'a.o_form_uri.o_field_widget.o_text_overflow',
+        assert.containsOnce(form, 'div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a',
             "should have a anchor with correct classes");
-        assert.hasAttrValue(form.$('a.o_form_uri.o_field_widget.o_text_overflow'), 'href', 'http://yop',
+        assert.hasAttrValue(form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a'), 'href', 'http://yop',
             "should have proper href link");
-        assert.hasAttrValue(form.$('a.o_form_uri.o_field_widget.o_text_overflow'), 'target', '_blank',
+        assert.hasAttrValue(form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a'), 'target', '_blank',
             "should have target attribute set to _blank");
-        assert.strictEqual(form.$('a.o_form_uri.o_field_widget.o_text_overflow').text(), 'yop',
+        assert.strictEqual(form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a').text(), 'yop',
             "the value should be displayed properly");
 
         // switch to edit mode and check the result
@@ -1959,11 +1959,11 @@ QUnit.module('basic_fields', {
 
         // save
         await testUtils.form.clickSave(form);
-        assert.containsOnce(form, 'a.o_form_uri.o_field_widget.o_text_overflow',
+        assert.containsOnce(form, 'div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a',
             "should still have a anchor with correct classes");
-        assert.hasAttrValue(form.$('a.o_form_uri.o_field_widget.o_text_overflow'), 'href', 'http://limbo',
+        assert.hasAttrValue(form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a'), 'href', 'http://limbo',
             "should have proper new href link");
-        assert.strictEqual(form.$('a.o_form_uri.o_field_widget.o_text_overflow').text(), 'limbo',
+        assert.strictEqual(form.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a').text(), 'limbo',
             'the new value should be displayed');
 
         form.destroy();
@@ -1982,7 +1982,7 @@ QUnit.module('basic_fields', {
             res_id: 1,
         });
 
-        assert.strictEqual(form.$('a[name="foo"]').text(), 'kebeclibre',
+        assert.strictEqual(form.$('div[name="foo"].o_field_url > a').text(), 'kebeclibre',
             "url text should come from the text attribute");
         form.destroy();
     });
@@ -2009,10 +2009,10 @@ QUnit.module('basic_fields', {
             res_id: 1,
         });
 
-        assert.strictEqual(form.$('a[name="url1"]').attr('href'), 'http://www.url1.com');
-        assert.strictEqual(form.$('a[name="url2"]').attr('href'), 'www.url2.com');
-        assert.strictEqual(form.$('a[name="url3"]').attr('href'), 'http://www.url3.com');
-        assert.strictEqual(form.$('a[name="url4"]').attr('href'), 'https://url4.com');
+        assert.strictEqual(form.$('div[name="url1"].o_field_url > a').attr('href'), 'http://www.url1.com');
+        assert.strictEqual(form.$('div[name="url2"].o_field_url > a').attr('href'), 'www.url2.com');
+        assert.strictEqual(form.$('div[name="url3"].o_field_url > a').attr('href'), 'http://www.url3.com');
+        assert.strictEqual(form.$('div[name="url4"].o_field_url > a').attr('href'), 'https://url4.com');
 
         form.destroy();
     });
@@ -2029,9 +2029,9 @@ QUnit.module('basic_fields', {
 
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector)').length, 5,
             "should have 5 cells");
-        assert.containsN(list, 'a.o_form_uri.o_field_widget.o_text_overflow', 5,
+        assert.containsN(list, 'div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a', 5,
             "should have 5 anchors with correct classes");
-        assert.hasAttrValue(list.$('a.o_form_uri.o_field_widget.o_text_overflow').first(), 'href', 'http://yop',
+        assert.hasAttrValue(list.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a').first(), 'href', 'http://yop',
             "should have proper href link");
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector)').first().text(), 'yop',
             "value should be displayed properly as text");
@@ -2048,11 +2048,11 @@ QUnit.module('basic_fields', {
         await testUtils.dom.click(list.$buttons.find('.o_list_button_save'));
         $cell = list.$('tbody td:not(.o_list_record_selector)').first();
         assert.doesNotHaveClass($cell.parent(), 'o_selected_row', 'should not be in edit mode anymore');
-        assert.containsN(list, 'a.o_form_uri.o_field_widget.o_text_overflow', 5,
+        assert.containsN(list, 'div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a', 5,
             "should still have 5 anchors with correct classes");
-        assert.hasAttrValue(list.$('a.o_form_uri.o_field_widget.o_text_overflow').first(), 'href', 'http://brolo',
+        assert.hasAttrValue(list.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a').first(), 'href', 'http://brolo',
             "should have proper new href link");
-        assert.strictEqual(list.$('a.o_form_uri.o_field_widget.o_text_overflow').first().text(), 'brolo',
+        assert.strictEqual(list.$('div.o_form_uri.o_field_widget.o_text_overflow.o_field_url > a').first().text(), 'brolo',
             "value should be properly updated");
 
         list.destroy();
@@ -5735,7 +5735,7 @@ QUnit.module('basic_fields', {
             },
         });
 
-        var $phone = form.$('a.o_field_widget.o_form_uri');
+        var $phone = form.$('div.o_field_widget.o_form_uri.o_field_phone > a');
         assert.strictEqual($phone.length, 1,
             "should have rendered the phone number as a link with correct classes");
         assert.strictEqual($phone.text(), 'yop',
@@ -5753,7 +5753,7 @@ QUnit.module('basic_fields', {
 
         // save
         await testUtils.form.clickSave(form);
-        assert.strictEqual(form.$('a.o_field_widget.o_form_uri').text(), 'new',
+        assert.strictEqual(form.$('div.o_field_widget.o_form_uri.o_field_phone > a').text(), 'new',
             "new value should be displayed properly");
 
         form.destroy();
@@ -5779,7 +5779,7 @@ QUnit.module('basic_fields', {
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector) a').first().text(), 'yop',
             "value should be displayed properly with a link to send SMS");
 
-        assert.containsN(list, 'a.o_field_widget.o_form_uri', 5,
+        assert.containsN(list, 'div.o_field_widget.o_form_uri.o_field_phone > a', 5,
             "should have the correct classnames");
 
         // Edit a line and check the result
@@ -5796,7 +5796,7 @@ QUnit.module('basic_fields', {
         assert.doesNotHaveClass($cell.parent(), 'o_selected_row', 'should not be in edit mode anymore');
         assert.strictEqual(list.$('tbody td:not(.o_list_record_selector) a').first().text(), 'new',
             "value should be properly updated");
-        assert.containsN(list, 'a.o_field_widget.o_form_uri', 5,
+        assert.containsN(list, 'div.o_field_widget.o_form_uri.o_field_phone > a', 5,
             "should still have links with correct classes");
 
         list.destroy();
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index 66d7d6bf594..570fc238f5a 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -4772,7 +4772,7 @@ QUnit.module('Views', {
             "product_id should be focused");
         form.$('[name="product_id"]').trigger($.Event('keydown', {which: $.ui.keyCode.TAB}));
         form.$('[name="foo"]:eq(1)').trigger($.Event('keydown', {which: $.ui.keyCode.TAB}));
-        assert.strictEqual(form.$('[name="display_name"]')[0], document.activeElement,
+        assert.strictEqual(form.$('div[name="display_name"].o_field_url > a')[0], document.activeElement,
             "display_name should be focused");
 
         // simulate shift+tab on active element