[FIX] web: prevent command palette deadlock
Impacted versions:
16.0+
How to reproduce:
- create a lot of articles in Knowledge (i.e. 3000+)
- click twice on the `Search Articles` fake search bar in the Knowledge Form
view
Current behavior:
- the command palette does not open
- the command palette is broken and won't open again (even with the `CTRL+K`
shortcut)
Expected behavior:
- the command palette open once and should not break even if the user clicks
multiple times.
Technical explanation:
- The promise given to the `onWillStart` hook of the `command_palette` can
sometimes never be resolved by design (see `KeepLast`). i.e.:
- `setCommandPaletteConfig` is called `onWillStart`
- `setCommandPaletteConfig` is then called as a result of the `SET-CONFIG`
bus event
- the first promise will never be resolved since it uses a `KeepLast`,
therefore `onWillStart` will never be resolved either.
- In `command_service`, the variable `isPaletteOpened` is set to `true` before
the command palette dialog is actually mounted.
- If `onWillStart` is never resolved, the component will never be mounted, and
as such the dialog can never be closed, therefore `isPaletteOpened` stays
`true` forever resulting in a deadlock preventing any further opening of the
command palette.
The issue can be resolved by adding all `KeepLast` promises in a race, which is
what `onWillStart` should actually be waiting for, since it will be resolved as
soon as any `KeepLast` promise is resolved.
task-3554068
closes odoo/odoo#141613
X-original-commit: 6b5fc71e67569b93b699090b225eea147ef463d2
Related: odoo/enterprise#50466
Signed-off-by: Francois Georis (fge) <fge@odoo.com>
Signed-off-by: Damien Abeloos (abd) <abd@odoo.com>
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
import { Dialog } from "@web/core/dialog/dialog";
|
||||
import { useHotkey } from "@web/core/hotkeys/hotkey_hook";
|
||||
import { _t } from "@web/core/l10n/translation";
|
||||
import { KeepLast } from "@web/core/utils/concurrency";
|
||||
import { KeepLast, Race } from "@web/core/utils/concurrency";
|
||||
import { useAutofocus, useService } from "@web/core/utils/hooks";
|
||||
import { scrollTo } from "@web/core/utils/scrolling";
|
||||
import { fuzzyLookup } from "@web/core/utils/search";
|
||||
@@ -107,6 +107,7 @@ export class CommandPalette extends Component {
|
||||
}
|
||||
|
||||
this.keyId = 1;
|
||||
this.race = new Race();
|
||||
this.keepLast = new KeepLast();
|
||||
this._sessionId = CommandPalette.lastSessionId++;
|
||||
this.DefaultCommandItem = DefaultCommandItem;
|
||||
@@ -181,7 +182,7 @@ export class CommandPalette extends Component {
|
||||
|
||||
const { namespace, searchValue } = this.processSearchValue(config.searchValue || "");
|
||||
this.switchNamespace(namespace);
|
||||
await this.search(searchValue);
|
||||
await this.race.add(this.search(searchValue));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
click,
|
||||
destroy,
|
||||
getFixture,
|
||||
makeDeferred,
|
||||
mount,
|
||||
nextTick,
|
||||
patchWithCleanup,
|
||||
@@ -1267,3 +1268,63 @@ QUnit.test("uses openPalette to modify the config used by the command palette",
|
||||
);
|
||||
assert.deepEqual(target.querySelector(".o_command_palette_search input").value, "Command");
|
||||
});
|
||||
|
||||
QUnit.test(
|
||||
"ensure that calling openPalette multiple times successfully loads the last config for the command palette",
|
||||
async (assert) => {
|
||||
const providePromise1 = makeDeferred();
|
||||
const providePromise2 = makeDeferred();
|
||||
const action = () => {};
|
||||
|
||||
await mount(TestComponent, target, { env });
|
||||
|
||||
const provide = [
|
||||
async () => {
|
||||
await providePromise1;
|
||||
return [
|
||||
{
|
||||
name: "Command1",
|
||||
action,
|
||||
},
|
||||
];
|
||||
},
|
||||
async () => {
|
||||
await providePromise2;
|
||||
return [
|
||||
{
|
||||
name: "Command2",
|
||||
action,
|
||||
},
|
||||
];
|
||||
},
|
||||
];
|
||||
const configCustom1 = {
|
||||
searchValue: "Command",
|
||||
providers: [{ provide: provide[0] }],
|
||||
};
|
||||
const configCustom2 = {
|
||||
searchValue: "Command",
|
||||
providers: [{ provide: provide[1] }],
|
||||
};
|
||||
|
||||
env.services.command.openPalette(configCustom1);
|
||||
await nextTick();
|
||||
assert.containsNone(target, ".o_command_palette");
|
||||
env.services.command.openPalette(configCustom2);
|
||||
await nextTick();
|
||||
assert.containsNone(target, ".o_command_palette");
|
||||
providePromise1.resolve();
|
||||
await nextTick();
|
||||
// First config should not be loaded since a second config was sent.
|
||||
assert.containsNone(target, ".o_command_palette");
|
||||
providePromise2.resolve();
|
||||
await nextTick();
|
||||
// Second config should be loaded properly.
|
||||
assert.containsN(target, ".o_command", 1);
|
||||
assert.deepEqual(
|
||||
[...target.querySelectorAll(".o_command span:first-child")].map((el) => el.textContent),
|
||||
["Command2"]
|
||||
);
|
||||
assert.deepEqual(target.querySelector(".o_command_palette_search input").value, "Command");
|
||||
}
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user