From 40ca30a12e8866de3d9f6c827d2b71035557dd05 Mon Sep 17 00:00:00 2001 From: qsm-odoo Date: Sat, 8 Oct 2022 09:43:10 +0000 Subject: [PATCH] [FIX] website: hide potential issues of save after drag&drop in tours The website edition system is based on a main mutex to guarantee the order of user manipulations. Meaning that if an user follows any flow really quickly, each part of the flow should be guaranteed to be done in the order the user asked for, independently from the duration of each part. The mutex system to ensure that is not robust yet though, or at least it is not easy to use and should probably be replaced by something more easier to use in the future. Indeed, some operations are badly implemented around this mutex. This is the case of the drag and drop of a snippet into the page. Drag and dropping a snippet is a long series of operations, to name a few: 1. The user mousedown on a snippet thumbnail 2. The user drags the snippet around 3. The user drops the snippet in a dropzone 4. We scroll the page to the position of the snippet 5. The snippet is built (options' `onBuilt` methods) 6. The snippet public widgets are started That whole series of operation has to be reduced to an atomic work to be added to the mutex queue. Meaning that for example, if the user manages to click on the save button while he is dragging a snippet around... the editor should only save the page after the whole series of operations of the drag and drop has completed. This is not the case right now: the mutex lock is released between (4) and (5) because we have too: (5) and (6) use the mutex themselves and would thus induce a deadlock if we only release the mutex after (6) as it should be. This means that during the scroll to the snippet (4), if the user manages to click on the save button, the save operation will be scheduled to be done before (5) and (6). It is thus technically possible to save a snippet which is not built** (even if it would not be critical most of the time). Worse, before building a snippet, its options have to be instantiated... and that operation is not mutex protected so this is probably (not 100% sure) possible that this happens: a. Drag and drop a snippet. During the scrolling, click on save. b. End of scrolling, the mutex lock is released, we create the first option and its widgets as part of the onBuilt process. Creating each widget is async, once the first one is created, the saving process starts as it was scheduled in (a). c. Saving the content, destroying all snippet options and their widgets. d. Continuing building snippet options widgets started at (b)... but previously-made ones were destroyed at (c)... possible crashes***. Fixing all of this probably requires a change in the system, maybe the use of multiple mutexes, maybe the saving operation can always be delayed at the end of the queue somehow, ... but this requires some more thinking and probably not possible in stable. In stable, it could be enough to disable the saving entirely during drag and drop (like drag and drop of another snippet is currently disabled during another drag and drop). This will be done in another commit if this is decided. Meanwhile, this commit just changes the behavior in tours for now: waiting for drag&drop to fully complete before clicking on save, like an human would. ** It is possible to reproduce by using the ALT + S shortcut: start dragging a snippet which has a onBuilt method in its options, hover the dropzone where you will drop, ALT + S to save (the editor waits you drop your snippet), drop your snippet -> the snippet is saved and the edit mode is left... but the onBuilt was not executed. *** a crash was indeed found during a specific theme tour (out of ~30 Odoo themes because of this after the other commit of this current PR). After those other commits especially, it is also possible to reproduce with the ALT + S trick, although not consistently. It may be linked to the specific last snippet added by that theme tour instantiating specific options. But as explained saving just after drop is an issue independently from this potential crash problem (snippet not built, etc). X-original-commit: e96167ae20d2a143d7def4347166b37e158003b0 Part-of: odoo/odoo#102800 --- addons/website/static/src/js/tours/tour_utils.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/addons/website/static/src/js/tours/tour_utils.js b/addons/website/static/src/js/tours/tour_utils.js index 0864ba9350a..ee35fdf14ef 100644 --- a/addons/website/static/src/js/tours/tour_utils.js +++ b/addons/website/static/src/js/tours/tour_utils.js @@ -184,7 +184,14 @@ function clickOnSnippet(snippet, position = "bottom") { function clickOnSave(position = "bottom") { return [{ trigger: "div:not(.o_loading_dummy) > #oe_snippets button[data-action=\"save\"]:not([disabled])", - extra_trigger: "body:not(:has(.o_dialog))", + // TODO this should not be needed but for now it better simulates what + // an human does. By the time this was added, it's technically possible + // to drag and drop a snippet then immediately click on save and have + // some problem. Worst case probably is a traceback during the redirect + // after save though so it's not that big of an issue. The problem will + // of course be solved (or at least prevented in stable). More details + // in related commit message. + extra_trigger: "body:not(:has(.o_dialog)) #oe_snippets:not(:has(.o_we_already_dragging))", in_modal: false, content: Markup(_t("Good job! It's time to Save your work.")), position: position,