-
- Downloads
[FIX] web: race condition with x2many control panel
The ControlPanel has been converted in Owl [1], and the code using it has been adapted accordingly. However, in the FieldX2Many, we didn't properly wait for the ControlPanel to be updated (an update of the ControlPanel was synchronous before owl, and is now async, like every Owl renderings, as it waits for the nextAnimationFrame). As a consequence, we might have tricky issues because the mounted hook of the control panel might be called multiple times for a single call to willUnmount later on. In mobile, we bind a global event handler (on scroll) in mounted, and unbind it in willUnmount, so we had leftover event handlers, that crashed when called after the ControlPanel was destroyed. Note that even if the issue popped in mobile, calling mounted on already mounted Components isn't a good idea, and this should be fixed anyway. The issue could be reproduced for instance in FieldService (with collaborative pads activated in Project), in mobile, by opening a task in Edit mode. Then, you might get a traceback by scrolling after having discarded the edition Here is a description of what technically happened: - when clicking on Edit, all widgets (including the FieldX2Many are destroyed and re-instantiated in 'edit' mode). - the pad widget directly triggers a field_changed event which causes a reset of the FieldX2Many (i.e. 'render' is called again) - the FieldX2Many detects that it already has a renderer (and a ControlPanel) so it updates them - it first updates the renderer, and when it's done, it updates the ControlPanel BUT doesn't wait for its promise, so the promise returned by that call to 'render' in FieldX2Many is resolved before the ControlPanel is actually updated - note that at this point, all thoses new widgets are not in the DOM yet - when all widgets are ready, the renderer patches the view (i.e. the former content is removed from the DOM, and the new one is attached into the DOM). As soon as this is done, the renderer calls 'on_attach_callback' on its children, including the FieldX2Many, which leads to a call to 'mounted' on the CP. - then, just before the nextAnimationFrame, Owl complete the rendering of the CP, and detects that it is now in the DOM (it wasn't at the beginning), so 'mounted' is called a second time, will cause the issue described above. This commit fixes the issue by properly waiting for the CP to be rendered in the FieldX2Many. However, this required on cascade changes: - Form view renderings with a FieldX2Many are now *really* async (+- 16ms), meaning that the user can easily trigger concurrent renderings by, e.g. clicking quickly several times on 'Edit', 'Save' or 'Discard'. Concurrent renderings are properly handled so to prevent this from happening, we disable the buttons and re-enable them when the rendering is done (like already done in [2]) - in the FieldX2Many, '_updateControlPanel' was called at several placed, but we never waited for it. As this method was originally sync, its calls have probably been naively adapted, whereas their should have been deeply rethought (for instance, as it is async, and we need to wait for it, we don't want it to be called multiple times sequentially when something happens). This commit does that work, i.e. we clean the places where this function is called such that it is (hopefully) never called sequentially twice. To do so, we changed a bit the spec of the pager in multi page, and we also fixed a paging-related bug. In a few words, here is what we did/do when adding a new row in the bottom of a full page: - before: tweak the count in the data to fool the pager and make it think that no new record has been added (so basically, let it display something wrong) - now: temporarily increase the pager limit so that the new record is displayed on the current pager, and the pager values are correct w.r.t. the displayed records. Some tests needed to be adapted accordingly. - By waiting for the ControlPanel when updating the FieldX2Many, a bunch of QUnit tests failed. Those tests have something in common: they spawn an X2Many (list or kanban theoretically, but always list in practice) containing a FieldBoolean (only field widget of /web converted in owl). When the FieldX2Many is updated, we update the renderer (i.e. re-renderer the FieldBoolean, so we have to wait for the nextAnimationFrame), and when this is done, we update the page (again, we have to wait for the nextAnimationFrame). So basically, we have to wait for two nextAnimationFrames to see the result in the DOM. For this, we added a new test util which basically does a nextTick ('owlCompatibilityNextTick'), and called it everywhere it was necessary. When everything will be written in Owl, we could get rid of this util and its calls. [1] https://github.com/odoo/odoo/commit/fbf347498f1cc7b74ef373179b7bcae201715c24 [2] https://github.com/odoo/odoo/commit/39f08950d6e20460e3a20a5b9c33e4ddf66dce78 closes odoo/odoo#61593 Signed-off-by:Adrien Dieudonné (adr) <adr@odoo.com>
Showing
- addons/web/static/src/js/fields/relational_fields.js 10 additions, 18 deletionsaddons/web/static/src/js/fields/relational_fields.js
- addons/web/static/src/js/views/form/form_controller.js 9 additions, 2 deletionsaddons/web/static/src/js/views/form/form_controller.js
- addons/web/static/tests/fields/relational_fields/field_many2many_tests.js 2 additions, 4 deletions...c/tests/fields/relational_fields/field_many2many_tests.js
- addons/web/static/tests/fields/relational_fields/field_many2one_tests.js 6 additions, 0 deletions...ic/tests/fields/relational_fields/field_many2one_tests.js
- addons/web/static/tests/fields/relational_fields/field_one2many_tests.js 122 additions, 13 deletions...ic/tests/fields/relational_fields/field_one2many_tests.js
- addons/web/static/tests/fields/relational_fields_tests.js 17 additions, 0 deletionsaddons/web/static/tests/fields/relational_fields_tests.js
- addons/web/static/tests/helpers/test_utils.js 15 additions, 0 deletionsaddons/web/static/tests/helpers/test_utils.js
- addons/web/static/tests/views/form_tests.js 8 additions, 1 deletionaddons/web/static/tests/views/form_tests.js
- addons/web/static/tests/views/list_tests.js 2 additions, 0 deletionsaddons/web/static/tests/views/list_tests.js
Loading
Please register or sign in to comment