[FIX] web: prevent crash in some cases in list editable

Before this commit, when we moved from one line to the next, we did not
wait for the unselectRow to end.  This means that if the read (from the
unselect row) completes after the default_get, the list renderer was not
in a coherent state (currentRow was set to null), which could (and did)
cause a crash.

This was found by pressing the TAB key a few times, and editing some
values.

Also, we slightly improved the logging in the mock server.  Before this,
the responses from the mock server did not show from which route it came
from.  But in this test, we specifically make sure that the rpcs
complete in a different order, so it was not really optimal.
This commit is contained in:
Géry Debongnie
2017-05-02 09:12:41 +02:00
parent 5a58cb2817
commit 6f0be0dfca
3 changed files with 55 additions and 3 deletions
@@ -256,8 +256,9 @@ ListRenderer.include({
if (this.currentRow < this.state.data.length - 1) {
this._selectCell(this.currentRow + 1, 0);
} else {
this._unselectRow();
this.trigger_up('add_record');
this._unselectRow().then(
this.trigger_up.bind(this, 'add_record')
);
}
},
/**
@@ -106,7 +106,7 @@ var MockServer = Class.extend({
if (logLevel === 1) {
console.log('Mock: ' + route, JSON.parse(resultString));
} else if (logLevel === 2) {
console.log('%c[rpc] response:', 'color: blue; font-weight: bold;', JSON.parse(resultString));
console.log('%c[rpc] response' + route, 'color: blue; font-weight: bold;', JSON.parse(resultString));
}
return JSON.parse(resultString);
});
@@ -1617,6 +1617,57 @@ QUnit.module('Views', {
list.destroy();
});
QUnit.test('navigation with tab and read completes after default_get', function (assert) {
assert.expect(8);
var defaultGetDef = $.Deferred();
var readDef = $.Deferred();
var list = createView({
View: ListView,
model: 'foo',
data: this.data,
arch: '<tree editable="bottom"><field name="foo"/><field name="int_field"/></tree>',
mockRPC: function (route, args) {
if (args.method) {
assert.step(args.method);
}
var result = this._super.apply(this, arguments);
if (args.method === 'read') {
return readDef.then(_.constant(result));
}
if (args.method === 'default_get') {
return defaultGetDef.then(_.constant(result));
}
return result;
},
});
list.$('td:contains(-4)').last().click();
list.$('tr.o_selected_row input[name="int_field"]').val('1234').trigger('input');
list.$('tr.o_selected_row input[name="int_field"]').trigger({type: 'keydown', which: 9}); // tab
defaultGetDef.resolve();
assert.strictEqual(list.$('tbody tr.o_data_row').length, 4,
"should have 4 data rows");
readDef.resolve();
assert.strictEqual(list.$('tbody tr.o_data_row').length, 5,
"should have 5 data rows");
assert.strictEqual(list.$('td:contains(1234)').length, 1,
"should have a cell with new value");
// we trigger a tab to move to the second cell in the current row. this
// operation requires that this.currentRow is properly set in the
// list editable renderer.
list.$('tr.o_selected_row input[name="foo"]').trigger({type: 'keydown', which: 9}); // tab
assert.ok(list.$('tr.o_data_row:eq(4)').hasClass('o_selected_row'),
"5th row should be selected");
assert.verifySteps(['write', 'read', 'default_get']);
list.destroy();
});
});
});