pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web_editor: prevent async code on link tools update

Browse Source 
Starting from [1], the `_updateColorpicker` method was added on the
"link tools" widget to update colorpickers with the selected link colors
(and it will create the colorpicker for each CSS property on widget
`start`).

Following this update, the code on [2] and [3] added some fixes to
handle the async parts of `_updateColorpicker`. [3] was done very
recently, more than one year later after the introduction of [1]. It
turned a synchronous method into an asynchronous one (rightfully
considering the async calls to `_updateColorpicker` that were inside).
As a fix, this commit will try to re-made it a synchronous method, and
other parts related to `_updateColorpicker` too.

The goal of this commit is to separate the link tools colorpicker
"creation" code (`_addColorPicker` now) from colorpicker's "update"
code. These changes are supposed to fix a race condition issue that
appeared on 16.0, but since the code is the same on 15.0 and subject to
the same issues, we target 15.0 with this fix. Hopefully, this prepares
for an easier refactoring in master.

[1]: https://github.com/odoo/odoo/commit/26d37812f0217ae913c8ddf761bcb26b5c44bff5
[2]: https://github.com/odoo/odoo/pull/78301/commits/6f1a6b76642db2e4f81cf3474ad0ec745a4bb3c9
[3]: https://github.com/odoo/odoo/commit/d15fdca644ef9ed1dfb011d3790f403e0157f55b

runbot-5753

closes odoo/odoo#113510

X-original-commit: 2d4a3efe0d7c9618c388575b4bb652a26913fe68
Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
Co-authored-by: qsm-odoo <qsm@odoo.com>
This commit is contained in:
xO-Tx
2023-02-24 12:14:35 +01:00
co-authored by qsm-odoo
parent 385721586e
commit 1096ed7b5a
3 changed files with 125 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web_editor/static/src/js/editor/snippets.editor.js
+27 -5
View File
@@ -2054,6 +2054,15 @@ var SnippetsMenu = Widget.extend({
* - Remove the 'contentEditable' attributes
*/
cleanForSave: async function () {
// Wait for snippet post-drop code here, since sometimes we save very
// quickly after a snippet drop during automated testing, which breaks
// some options code (executed while destroying the editor).
// TODO we should find a better way, by better locking the drag and drop
// code inside the edition mutex... which unfortunately cannot be done
// given the state of the code, as internal operations of that drag and
// drop code need to use the mutex themselves.
await this.postSnippetDropPromise;
// First disable the snippet selection, calling options onBlur, closing
// widgets, etc. Then wait for full resolution of the mutex as widgets
// may have triggered some final edition requests that need to be
@@ -2157,6 +2166,10 @@ var SnippetsMenu = Widget.extend({
* @returns {Promise}
*/
callPostSnippetDrop: async function ($target) {
this.postSnippetDropPromise = new Promise(resolve => {
this._postSnippetDropResolver = resolve;
});
// First call the onBuilt of all options of each item in the snippet
// (and so build their editor instance first).
await this._callForEachChildSnippet($target, function (editor, $snippet) {
@@ -2180,6 +2193,8 @@ var SnippetsMenu = Widget.extend({
// Lastly, ensure that the snippets or its related parts are added to
// the invisible DOM list if needed.
await this._updateInvisibleDOM();
this._postSnippetDropResolver();
},
/**
* Public implementation of _execWithLoadingEffect.
@@ -2532,13 +2547,20 @@ var SnippetsMenu = Widget.extend({
}
return editorToEnable;
}).then(editor => {
}).then(async editor => {
// If a link was clicked, the linktools should be focused after
// the right panel is shown to the user.
if (this._currentTab === this.tabs.OPTIONS
&& this.options.wysiwyg.linkTools
&& !this.options.wysiwyg.linkTools.noFocusUrl) {
this.options.wysiwyg.linkTools.focusUrl();
// TODO: this should be reviewed to be done another way: we
// should avoid focusing something here while it is being
// rendered elsewhere.
const linkTools = this.options.wysiwyg.linkTools;
if (linkTools && this._currentTab === this.tabs.OPTIONS
&& !linkTools.noFocusUrl) {
// Wait for `linkTools` potential in-progress rendering
// before focusing the URL input on `snippetsMenu` (this
// prevents race condition for automated testing).
await linkTools.renderingPromise;
linkTools.focusUrl();
}
return editor;
});
addons/web_editor/static/src/js/wysiwyg/widgets/link.js
+17 -1
View File
@@ -125,6 +125,8 @@ const Link = Widget.extend({
if (/(?:s_website_form_send|o_submit)/.test(this.data.className)) {
this.isButton = true;
}
this.renderingPromise = new Promise(resolve => this._renderingResolver = resolve);
},
/**
* @override
@@ -152,7 +154,7 @@ const Link = Widget.extend({
const _super = this._super.bind(this);
await this._updateOptionsUI();
this._updateOptionsUI();
if (this.data.url) {
var match = /mailto:(.+)/.exec(this.data.url);
@@ -167,6 +169,20 @@ const Link = Widget.extend({
return _super(...arguments);
},
/**
* @private
*/
async _widgetRenderAndInsert() {
const res = await this._super(...arguments);
// TODO find a better solution than this during the upcoming refactoring
// of the link tools / link dialog.
if (this._renderingResolver) {
this._renderingResolver();
}
return res;
},
/**
* @override
*/
addons/web_editor/static/src/js/wysiwyg/widgets/link_tools.js
+81 -58
View File
@@ -31,7 +31,8 @@ const LinkTools = Link.extend({
*/
init: function (parent, options, editable, data, $button, link) {
this._link = link;
this._observer = new MutationObserver(() =>{
this._observer = new MutationObserver(async () => {
await this.renderingPromise;
this._updateLabelInput();
});
this._observer.observe(this._link, {subtree: true, childList: true, characterData: true});
@@ -39,7 +40,19 @@ const LinkTools = Link.extend({
// Keep track of each selected custom color and colorpicker.
this.customColors = {};
this.colorpickers = {};
this.colorpickersPromises = {};
this.COLORPICKER_CSS_PROPERTIES = ['color', 'background-color', 'border-color'];
this.PREFIXES = {
'color': 'text-',
'background-color': 'bg-',
};
},
/**
* @override
*/
willStart: async function () {
const _super = this._super.bind(this);
await Promise.all(this.COLORPICKER_CSS_PROPERTIES.map(cssProperty => this._addColorPicker(cssProperty)));
return _super(...arguments);
},
/**
* @override
@@ -48,12 +61,24 @@ const LinkTools = Link.extend({
this._addHintClasses();
const ret = await this._super(...arguments);
const link = this.$link[0];
const colorpickerLocations = {
'color': '.link-custom-color-text .dropdown-menu',
'background-color': '.link-custom-color-fill .dropdown-menu',
'border-color': '.link-custom-color-border .o_we_so_color_palette .dropdown-menu',
};
for (const cssProperty of this.COLORPICKER_CSS_PROPERTIES) {
// Colorpickers were created into fragments before any UI or event
// handler of this main widget was built. This just moves those
// colorpickers at their rightful position, synchronously.
const locationEl = this.el.querySelector(colorpickerLocations[cssProperty]);
this.colorpickers[cssProperty].$el.appendTo(locationEl);
}
const customStyleProps = ['color', 'background-color', 'background-image', 'border-width', 'border-style', 'border-color'];
if (customStyleProps.some(s => link.style[s])) {
// Force custom style if style exists on the link.
const customOption = this.el.querySelector('[name="link_style_color"] we-button[data-value="custom"]');
this._setSelectOption($(customOption), true);
await this._updateOptionsUI();
this._updateOptionsUI();
}
return ret;
},
@@ -226,7 +251,7 @@ const LinkTools = Link.extend({
/**
* @override
*/
_updateOptionsUI: async function () {
_updateOptionsUI: function () {
const el = this.el.querySelector('[name="link_style_color"] we-button.active');
if (el) {
this.colorCombinationClass = el.dataset.value;
@@ -235,9 +260,12 @@ const LinkTools = Link.extend({
// Show custom colors only for Custom style.
this.$('.link-custom-color').toggleClass('d-none', el.dataset.value !== 'custom');
await this._updateColorpicker('color');
await this._updateColorpicker('background-color');
await this._updateColorpicker('border-color');
// Note: the _updateColorpicker method is supposedly async but can
// be used synchronously given the fact that _addColorPicker was
// called during this widget initialization.
this._updateColorpicker('color');
this._updateColorpicker('background-color');
this._updateColorpicker('border-color');
const borderWidth = this.linkEl.style['border-width'];
const numberAndUnit = getNumericAndUnit(borderWidth);
@@ -261,58 +289,8 @@ const LinkTools = Link.extend({
* @param {string} cssProperty
*/
_updateColorpicker: async function (cssProperty) {
const prefix = {
'color': 'text-',
'background-color': 'bg-',
}[cssProperty];
const prefix = this.PREFIXES[cssProperty];
let colorpicker = this.colorpickers[cssProperty];
await this.colorpickersPromises[cssProperty];
if (!colorpicker) {
colorpicker = new ColorPaletteWidget(this, {
excluded: ['transparent_grayscale'],
$editable: $(this.options.wysiwyg.odooEditor.editable),
withGradients: cssProperty === 'background-color',
});
this.colorpickers[cssProperty] = colorpicker;
const target = this.el.querySelector({
'color': '.link-custom-color-text .dropdown-menu',
'background-color': '.link-custom-color-fill .dropdown-menu',
'border-color': '.link-custom-color-border .o_we_so_color_palette .dropdown-menu',
}[cssProperty]);
this.colorpickersPromises[cssProperty] = colorpicker.appendTo($(target));
await this.colorpickersPromises[cssProperty];
colorpicker.on('custom_color_picked color_picked color_hover color_leave', this, (ev) => {
// Reset color styles in link content to make sure new color is not hidden.
// Only done when applied to avoid losing state during preview.
if (['custom_color_picked', 'color_picked'].includes(ev.name)) {
const selection = window.getSelection();
const range = document.createRange();
range.selectNodeContents(this.linkEl);
selection.removeAllRanges();
selection.addRange(range);
this.options.wysiwyg.odooEditor.execCommand('applyColor', '', 'color');
this.options.wysiwyg.odooEditor.execCommand('applyColor', '', 'backgroundColor');
}
let color = ev.data.color;
const colorNames = colorpicker.getColorNames();
const colorClasses = prefix ? computeColorClasses(colorNames, prefix) : [];
const colorClass = `${prefix}${color}`;
if (colorClasses.includes(colorClass)) {
color = colorClass;
} else if (colorNames.includes(color)) {
// Store as color value.
color = getCSSVariableValue(color);
}
this.customColors[cssProperty] = color;
this.applyLinkToDom(this._getData());
if (['custom_color_picked', 'color_picked'].includes(ev.name)) {
this.options.wysiwyg.odooEditor.historyStep();
this._updateOptionsUI();
}
});
}
// Update selected color.
const colorNames = colorpicker.getColorNames();
@@ -349,6 +327,51 @@ const LinkTools = Link.extend({
$colorPreview.css('background-image', isColorGradient(color) ? color : '');
}
},
/**
* @private
* @param {string} cssProperty
*/
async _addColorPicker(cssProperty) {
const prefix = this.PREFIXES[cssProperty];
const colorpicker = new ColorPaletteWidget(this, {
excluded: ['transparent_grayscale'],
$editable: $(this.options.wysiwyg.odooEditor.editable),
withGradients: cssProperty === 'background-color',
});
this.colorpickers[cssProperty] = colorpicker;
await colorpicker.appendTo(document.createDocumentFragment());
colorpicker.on('custom_color_picked color_picked color_hover color_leave', this, (ev) => {
// Reset color styles in link content to make sure new color is not hidden.
// Only done when applied to avoid losing state during preview.
if (['custom_color_picked', 'color_picked'].includes(ev.name)) {
const selection = window.getSelection();
const range = document.createRange();
range.selectNodeContents(this.linkEl);
selection.removeAllRanges();
selection.addRange(range);
this.options.wysiwyg.odooEditor.execCommand('applyColor', '', 'color');
this.options.wysiwyg.odooEditor.execCommand('applyColor', '', 'backgroundColor');
}
let color = ev.data.color;
const colorNames = colorpicker.getColorNames();
const colorClasses = prefix ? computeColorClasses(colorNames, prefix) : [];
const colorClass = `${prefix}${color}`;
if (colorClasses.includes(colorClass)) {
color = colorClass;
} else if (colorNames.includes(color)) {
// Store as color value.
color = getCSSVariableValue(color);
}
this.customColors[cssProperty] = color;
this.applyLinkToDom(this._getData());
if (['custom_color_picked', 'color_picked'].includes(ev.name)) {
this.options.wysiwyg.odooEditor.historyStep();
this._updateOptionsUI();
}
});
return colorpicker;
},
/**
* Add hint to the classes of the link and button.
*/