diff --git a/addons/web_editor/static/src/js/common/utils.js b/addons/web_editor/static/src/js/common/utils.js index 53707e352ac..c96c832fcc5 100644 --- a/addons/web_editor/static/src/js/common/utils.js +++ b/addons/web_editor/static/src/js/common/utils.js @@ -435,6 +435,32 @@ function _addBackgroundImageAttributes(...newAttributes) { function _isBackgroundImageAttribute(attribute) { return BACKGROUND_IMAGE_ATTRIBUTES.has(attribute); } +/** + * Checks if an element supposedly marked with the o_editable_media class should + * in fact be editable (checks if its environment looks like a non editable + * environment whose media should be editable). + * + * TODO: the name of this function is voluntarily bad to reflect the fact that + * this system should be improved. The combination of o_not_editable, + * o_editable, getContentEditableAreas, getReadOnlyAreas and other concepts + * related to what should be editable or not should be reviewed. + * + * @returns {boolean} + */ +function _shouldEditableMediaBeEditable(mediaEl) { + // Some sections of the DOM are contenteditable="false" (for + // example with the help of the o_not_editable class) but have + // inner media that should be editable (the fact the container + // is not is to prevent adding text in between those medias). + // This case is complex and the solution to support it is not + // perfect: we mark those media with a class and check that the + // first non editable ancestor is in fact in an editable parent. + const parentEl = mediaEl.parentElement; + const nonEditableAncestorRootEl = parentEl && parentEl.closest('[contenteditable="false"]'); + return nonEditableAncestorRootEl + && nonEditableAncestorRootEl.parentElement + && nonEditableAncestorRootEl.parentElement.isContentEditable; +} export default { COLOR_PALETTE_COMPATIBILITY_COLOR_NAMES: COLOR_PALETTE_COMPATIBILITY_COLOR_NAMES, @@ -460,4 +486,5 @@ export default { setEditableWindow: _setEditableWindow, addBackgroundImageAttributes: _addBackgroundImageAttributes, isBackgroundImageAttribute: _isBackgroundImageAttribute, + shouldEditableMediaBeEditable: _shouldEditableMediaBeEditable, }; diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js index f688230294f..6661daab2a3 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js @@ -2388,10 +2388,14 @@ export class OdooEditor extends EventTarget { this.observerUnactive('_activateContenteditable'); this.editable.setAttribute('contenteditable', this.options.isRootEditable); - const editableAreas = this.options.getContentEditableAreas(this).filter(node => !isArtificialVoidElement(node)); + const editableAreas = this.options.getContentEditableAreas(this); for (const node of editableAreas) { if (!node.isContentEditable) { - node.setAttribute('contenteditable', true); + if (isArtificialVoidElement(node) || node.nodeName === 'IMG') { + node.classList.add('o_editable_media'); + } else { + node.setAttribute('contenteditable', true); + } } } for (const node of this.options.getReadOnlyAreas()) { 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 8d6a3857218..6a4710a61b5 100644 --- a/addons/web_editor/static/src/js/editor/snippets.editor.js +++ b/addons/web_editor/static/src/js/editor/snippets.editor.js @@ -2769,10 +2769,12 @@ var SnippetsMenu = Widget.extend({ _computeSelectorFunctions: function (selector, exclude, target, noCheck, isChildren, excludeParent) { var self = this; - // TODO the `:not([contenteditable="true"])` part is designed to make - // images with such attribute editable even when they are in an - // environment where editing is not normally possible. This should be - // reviewed if we are to handle more hierarchy of editable nodes being + // The `:not(.o_editable_media)` part is handled outside of the selector + // (see filterFunc). + // Note: the `:not([contenteditable="true"])` part was there for that + // same purpose before the implementation of the o_editable_media class. + // It still make sense for potential editable areas though. Although it + // should be reviewed if we are to handle more hierarchy of nodes being // editable despite their non editable environment. // Without the `:not(.s_social_media)`, it is no longer possible to edit // icons in the social media snippet. This should be fixed in a more @@ -2780,7 +2782,13 @@ var SnippetsMenu = Widget.extend({ exclude += `${exclude && ', '}.o_snippet_not_selectable, .o_not_editable:not(.s_social_media) :not([contenteditable="true"])`; let filterFunc = function () { - return !$(this).is(exclude); + if (!$(this).is(exclude)) { + return true; + } + if (this.classList.contains('o_editable_media')) { + return weUtils.shouldEditableMediaBeEditable(this); + } + return false; }; if (target) { const oldFilter = filterFunc; diff --git a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js index 317e2cf98d3..348f002d339 100644 --- a/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js +++ b/addons/web_editor/static/src/js/wysiwyg/wysiwyg.js @@ -487,7 +487,21 @@ export class Wysiwyg extends Component { this.showTooltip = true; this.$editable.on('dblclick', mediaSelector, ev => { const targetEl = ev.currentTarget; - if (targetEl.isContentEditable || (targetEl.parentElement && targetEl.parentElement.isContentEditable)) { + let isEditable = + // TODO that first check is probably useless/wrong: checking if + // the media itself has editable content should not be relevant. + // In fact the content of all media should be marked as non + // editable anyway. + targetEl.isContentEditable || + // For a media to be editable, the base case is to be in a + // container whose content is editable. + (targetEl.parentElement && targetEl.parentElement.isContentEditable); + + if (!isEditable && targetEl.classList.contains('o_editable_media')) { + isEditable = weUtils.shouldEditableMediaBeEditable(targetEl); + } + + if (isEditable) { this.showTooltip = false; if (!isProtected(this.odooEditor.document.getSelection().anchorNode)) { diff --git a/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js b/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js index b69782631fe..1378ebc0f5d 100644 --- a/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js +++ b/addons/website/static/src/components/wysiwyg_adapter/wysiwyg_adapter.js @@ -7,6 +7,7 @@ import { useHotkey } from '@web/core/hotkeys/hotkey_hook'; import { Wysiwyg } from "@web_editor/js/wysiwyg/wysiwyg"; import { requireWysiwygLegacyModule } from "@web_editor/js/frontend/loader"; import weUtils from '@web_editor/js/common/utils'; +import { isMediaElement } from '@web_editor/js/editor/odoo-editor/src/utils/utils'; import { EditMenuDialog, MenuDialog } from "../dialog/edit_menu"; import { WebsiteDialog } from '../dialog/dialog'; @@ -474,12 +475,15 @@ export class WysiwygAdapterComponent extends Wysiwyg { return !$(el).closest('.o_not_editable').length; }); - // TODO review in master. This stable fix restores the possibility to + // TODO migrate in master. This stable fix restores the possibility to // edit the company team snippet images on subsequent editions. Indeed - // this badly relies on the contenteditable="true" attribute being on - // those images but it is rightfully lost after the first save. - // grep: COMPANY_TEAM_CONTENTEDITABLE - let $extraEditableZones = $editableSavableZones.find('.s_company_team .o_not_editable img'); + // this badly relied on the contenteditable="true" attribute being on + // those images but it is rightfully lost after the first save. Later, + // the o_editable_media class system was implemented and the class was + // added in the snippet template but this did not solve existing + // snippets in user databases. + let $extraEditableZones = $editableSavableZones.find('.s_company_team .o_not_editable *') + .filter((i, el) => isMediaElement(el) || el.tagName === 'IMG'); // To make sure the selection remains bounded to the active tab, // each tab is made non editable while keeping its nested diff --git a/addons/website/views/snippets/s_company_team.xml b/addons/website/views/snippets/s_company_team.xml index 2d4ff7c8fb0..924f62dd3a6 100644 --- a/addons/website/views/snippets/s_company_team.xml +++ b/addons/website/views/snippets/s_company_team.xml @@ -1,13 +1,6 @@ -