From 645b6b2c1087ba6fe596fc4f8df4507774b596ad Mon Sep 17 00:00:00 2001 From: Nicolas Bayet Date: Mon, 31 Jul 2023 14:18:05 +0000 Subject: [PATCH] [FIX] web_editor: properly fetching of colorpicker template As described in #129974, the colorpicker cannot cache a promise protected by `useService`. #129974 fixes the issue by through non idiomatic practice (calling a service that is not protected by `useService`). This commit fixes the issue idiomatically. This commit caches the promise directly from a service and removes the caching code from the `ColorPalette`. task-3446691 closes odoo/odoo#130789 X-original-commit: 1d2e54088b0f0e28464ab6aa884fb2e1110e8e04 Signed-off-by: David Monjoie (dmo) --- .../static/tests/core/dialog_service_tests.js | 2 +- addons/web_editor/__manifest__.py | 1 + .../static/src/js/editor/snippets.editor.js | 9 +----- .../static/src/js/editor/snippets.options.js | 1 + .../get_color_picker_template_service.js | 21 ++++++++++++ .../src/js/wysiwyg/widgets/color_palette.js | 32 ++----------------- .../static/src/js/wysiwyg/wysiwyg.js | 18 +++++------ .../web_editor/static/tests/mock_services.js | 16 ++++++++++ 8 files changed, 53 insertions(+), 47 deletions(-) create mode 100644 addons/web_editor/static/src/js/wysiwyg/get_color_picker_template_service.js create mode 100644 addons/web_editor/static/tests/mock_services.js diff --git a/addons/web/static/tests/core/dialog_service_tests.js b/addons/web/static/tests/core/dialog_service_tests.js index d5bcd0e9e12..2685df5de74 100644 --- a/addons/web/static/tests/core/dialog_service_tests.js +++ b/addons/web/static/tests/core/dialog_service_tests.js @@ -230,7 +230,7 @@ QUnit.test("dialog component crashes", async (assert) => { }); const rpc = makeFakeRPCService(); - serviceRegistry.add("rpc", rpc); + serviceRegistry.add("rpc", rpc, { force: true }); serviceRegistry.add("notification", notificationService); serviceRegistry.add("error", errorService); diff --git a/addons/web_editor/__manifest__.py b/addons/web_editor/__manifest__.py index a3fba486027..d8acbdd8a1e 100644 --- a/addons/web_editor/__manifest__.py +++ b/addons/web_editor/__manifest__.py @@ -73,6 +73,7 @@ Odoo Web Editor widget. 'web_editor/static/src/js/wysiwyg/PeerToPeer.js', 'web_editor/static/src/js/wysiwyg/conflict_dialog.js', 'web_editor/static/src/js/wysiwyg/conflict_dialog.xml', + 'web_editor/static/src/js/wysiwyg/get_color_picker_template_service.js', # odoo utils ('include', 'web._assets_helpers'), diff --git a/addons/web_editor/static/src/js/editor/snippets.editor.js b/addons/web_editor/static/src/js/editor/snippets.editor.js index 6a4710a61b5..4e2fab06740 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -7,7 +7,6 @@ import dom from "@web/legacy/js/core/dom"; import { Markup, confine } from "@web/legacy/js/core/utils"; import Widget from "@web/legacy/js/core/widget"; import options from "@web_editor/js/editor/snippets.options"; -import { ColorPalette } from "@web_editor/js/wysiwyg/widgets/color_palette"; import SmoothScrollOnDrag from "@web_editor/js/editor/smooth_scroll_on_drag"; import weUtils from "@web_editor/js/common/utils"; import * as gridUtils from "@web_editor/js/common/grid_layout_utils"; @@ -1774,13 +1773,7 @@ var SnippetsMenu = Widget.extend({ // widget have huge chances of being used by the user (clicking on any // text will load it). The colorpalette itself will do the actual // waiting of the loading completion. - ColorPalette.loadDependencies(() => { - return this._rpc({ - model: 'ir.ui.view', - method: 'render_public_asset', - args: ['web_editor.colorpicker', {}], - }); - }); + this.options.wysiwyg.getColorpickerTemplate(); return this._super(...arguments); }, /** diff --git a/addons/web_editor/static/src/js/editor/snippets.options.js b/addons/web_editor/static/src/js/editor/snippets.options.js index 3e43f6d81ec..aabf96497ef 100644 --- a/addons/web_editor/static/src/js/editor/snippets.options.js +++ b/addons/web_editor/static/src/js/editor/snippets.options.js @@ -1638,6 +1638,7 @@ const ColorpickerUserValueWidget = SelectUserValueWidget.extend({ const wysiwyg = this.getParent().options.wysiwyg; if (wysiwyg) { options.document = this.$target[0].ownerDocument; + options.getTemplate = wysiwyg.getColorpickerTemplate.bind(wysiwyg); } if (this.colorPaletteWrapper) { this.colorPaletteWrapper.destroy(); diff --git a/addons/web_editor/static/src/js/wysiwyg/get_color_picker_template_service.js b/addons/web_editor/static/src/js/wysiwyg/get_color_picker_template_service.js new file mode 100644 index 00000000000..ee7394cd973 --- /dev/null +++ b/addons/web_editor/static/src/js/wysiwyg/get_color_picker_template_service.js @@ -0,0 +1,21 @@ + +/** @odoo-module **/ +import { registry } from "@web/core/registry"; + +let colorPickerTemplatePromise; +export const getColorPickerTemplateService = { + dependencies: ["orm"], + async: true, + start(env, { orm }) { + return () => { + colorPickerTemplatePromise ??= orm.call( + 'ir.ui.view', + 'render_public_asset', + ['web_editor.colorpicker', {}] + ); + return colorPickerTemplatePromise; + }; + }, +}; + +registry.category("services").add("get_color_picker_template", getColorPickerTemplateService); diff --git a/addons/web_editor/static/src/js/wysiwyg/widgets/color_palette.js b/addons/web_editor/static/src/js/wysiwyg/widgets/color_palette.js index 3dc20c3ce32..a6824a9a68f 100644 --- a/addons/web_editor/static/src/js/wysiwyg/widgets/color_palette.js +++ b/addons/web_editor/static/src/js/wysiwyg/widgets/color_palette.js @@ -1,6 +1,5 @@ /** @odoo-module **/ -import session from "web.session"; import { Colorpicker } from "@web/core/colorpicker/colorpicker"; import customColors from "@web_editor/js/editor/custom_colors"; import weUtils from "@web_editor/js/common/utils"; @@ -9,8 +8,6 @@ import { normalizeCSSColor, convertCSSColorToRgba, } from '@web/core/utils/colors'; - - import { Component, useRef, @@ -20,30 +17,7 @@ import { onWillUpdateProps, } from "@odoo/owl"; - - -let colorpickerArch; -let colorpickerTemplateProm; - export class ColorPalette extends Component { - /** - * Load ColorPalette dependencies. This allows to load them without - * instantiating the widget itself. - * - * @static - */ - static async loadDependencies(getTemplate) { - // Public user using the editor may have a colorpalette but with - // the default wysiwyg ones. - if (!session.is_website_user) { - // We can call the colorPalette multiple times but only need 1 rpc - if (!colorpickerTemplateProm && !colorpickerArch) { - colorpickerTemplateProm = getTemplate().then(arch => colorpickerArch = arch); - - } - return colorpickerTemplateProm; - } - } static template = 'web_editor.ColorPalette'; static props = { document: { type: true, optional: true }, @@ -97,7 +71,7 @@ export class ColorPalette extends Component { this.init(); onWillStart(async () => { if (this.props.getTemplate) { - await ColorPalette.loadDependencies(this.props.getTemplate); + this.colorpickerTemplate = await this.props.getTemplate(); } }); onMounted(async () => { @@ -196,8 +170,8 @@ export class ColorPalette extends Component { const switchPaneButtons = this.el.querySelectorAll('.o_we_colorpicker_switch_pane_btn'); let colorpickerEl; - if (colorpickerArch) { - colorpickerEl = $(colorpickerArch)[0]; + if (this.colorpickerTemplate) { + colorpickerEl = $(this.colorpickerTemplate)[0]; } else { colorpickerEl = document.createElement("colorpicker"); const sectionEl = document.createElement('DIV'); diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 348f002d339..b404a62bd8b 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -135,6 +135,7 @@ export class Wysiwyg extends Component { setup() { this.orm = this._useService('orm'); + this.getColorPickerTemplateService = this._useService('get_color_picker_template'); this.notification = this._useService("notification"); this.popover = this._useService("popover"); this.busService = this.env.services.bus_service; @@ -184,7 +185,7 @@ export class Wysiwyg extends Component { $(pickergroup.querySelector('.dropdown-toggle')).dropdown('hide'); }, - getTemplate: this._getColorpickerTemplate.bind(this), + getTemplate: this.getColorpickerTemplate.bind(this), getEditableCustomColors: () => { if (!this.$editable) { return []; @@ -1716,6 +1717,13 @@ export class Wysiwyg extends Component { } return Promise.resolve(); } + getColorpickerTemplate() { + // Public user using the editor may have a colorpalette but with + // the default wysiwyg ones. + if (!session.is_website_user) { + return this.getColorPickerTemplateService(); + } + } //-------------------------------------------------------------------------- // Private @@ -2990,14 +2998,6 @@ export class Wysiwyg extends Component { } } - _getColorpickerTemplate() { - return this.env.services.orm.call( - 'ir.ui.view', - 'render_public_asset', - ['web_editor.colorpicker', {}] - ); - } - // ----------------------------------------------------------------------------- // Legacy compatibility layer // Remove me when all legacy widgets using wysiwyg are converted to OWL. diff --git a/addons/web_editor/static/tests/mock_services.js b/addons/web_editor/static/tests/mock_services.js new file mode 100644 index 00000000000..c55fa5bde76 --- /dev/null +++ b/addons/web_editor/static/tests/mock_services.js @@ -0,0 +1,16 @@ +/** @odoo-module */ + +import { patch } from '@web/core/utils/patch'; +import { registry } from '@web/core/registry'; +import { utils } from '@web/../tests/helpers/mock_env'; +import { getColorPickerTemplateService } from '@web_editor/js/wysiwyg/get_color_picker_template_service'; + +const { prepareRegistriesWithCleanup } = utils; + +const serviceRegistry = registry.category('services'); +patch(utils, { + prepareRegistriesWithCleanup() { + prepareRegistriesWithCleanup(...arguments); + serviceRegistry.add('get_color_picker_template', getColorPickerTemplateService); + }, +});