From fb06850e492fdb4f1f4d7a03cae09abed506b89a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= <ged@odoo.com>
Date: Thu, 20 Apr 2017 20:12:13 +0200
Subject: [PATCH] [FIX] web: prevent onchanges with invalid values

Before this commit, when a field was changed, it always triggered a
field_change event, with the new value, even if that value is not valid.
As a result, it could happen that an onchange was sent to the server
with a string as the value of a float field.

What we do in this commit is making sure that invalid values do not
escape the confine of a field, by returning immediately after detecting
that a value is not valid.
---
 .../static/src/js/fields/abstract_field.js    |  1 +
 addons/web/static/tests/views/form_tests.js   | 44 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/addons/web/static/src/js/fields/abstract_field.js b/addons/web/static/src/js/fields/abstract_field.js
index 0c7b6d71908..d2a317bd3b2 100644
--- a/addons/web/static/src/js/fields/abstract_field.js
+++ b/addons/web/static/src/js/fields/abstract_field.js
@@ -338,6 +338,7 @@ var AbstractField = Widget.extend({
             this._isValid = true;
         } catch (e) {
             this._isValid = false;
+            return;
         }
         var changes = {};
         changes[this.name] = value;
diff --git a/addons/web/static/tests/views/form_tests.js b/addons/web/static/tests/views/form_tests.js
index eacedbb4331..2d71552b119 100644
--- a/addons/web/static/tests/views/form_tests.js
+++ b/addons/web/static/tests/views/form_tests.js
@@ -2371,6 +2371,50 @@ QUnit.module('Views', {
         }
     });
 
+    QUnit.test('onchanges are not sent for invalid values', function (assert) {
+        assert.expect(6);
+
+        this.data.partner.onchanges = {
+            int_field: function (obj) {
+                obj.foo = String(obj.int_field);
+            },
+        };
+        var form = createView({
+            View: FormView,
+            model: 'partner',
+            data: this.data,
+            arch: '<form>' +
+                    '<group><field name="foo"/><field name="int_field"/></group>' +
+                '</form>',
+            res_id: 2,
+            mockRPC: function (route, args) {
+                assert.step(args.method);
+                return this._super.apply(this, arguments);
+            },
+        });
+
+        form.$buttons.find('.o_form_button_edit').click();
+
+        // edit int_field, and check that an onchange has been applied
+        form.$('input[name="int_field"]').val("123").trigger('input');
+        assert.strictEqual(form.$('input[name="foo"]').val(), "123",
+            "the onchange has been applied");
+
+        // enter an invalid value in a float, and check that no onchange has
+        // been applied
+        form.$('input[name="int_field"]').val("123a").trigger('input');
+        assert.strictEqual(form.$('input[name="foo"]').val(), "123",
+            "the onchange has not been applied");
+
+        // save, and check that the int_field input is marked as invalid
+        form.$buttons.find('.o_form_button_save').click();
+        assert.ok(form.$('input[name="int_field"]').hasClass('o_form_invalid'),
+            "input int_field is marked as invalid");
+
+        assert.verifySteps(['read', 'onchange']);
+        form.destroy();
+    });
+
     QUnit.test('rpc complete after destroying parent', function (assert) {
         // We just test that there is no crash in this situation
         assert.expect(0);