From 2ab5b6371a99361e398c8bb2b359969d6855d1dd Mon Sep 17 00:00:00 2001 From: Antoine Guenet Date: Thu, 22 May 2025 15:50:30 +0200 Subject: [PATCH] [FIX] html_editor: update font size selector on set tag in toolbar In website builder: 1. Select text 2. Change heading type in the toolbar -> the font size selector in the toolbar is blank. This is because the font size input is in an iframe, and a chain of events triggered by changing the heading type (`setTag`) leads to the iframe's document to be re-rendered. This only happens in the website builder because of a call to `updateContainers` as a step_added handler that triggers the `change_current_options_containers_listeners`. These listeners are triggered because `setTag` replaced the block which was the target of the `BuilderOptionsPlugin` with another. We have a method for when such a reference has changed and it bears a risk of breaking the selection: `preserveSelection`'s `remapNode. This binds it to `BuilderOptionsPlugin` so it can update its target in such a case. This way the target is not lost, the iframe is not re-rendered, and the toolbar is intact. Note: broader work might be needed to investigate whether it is normal that the iframe gets re-rendered, and if so, to guarantee that the input never gets lost in the process. --- .../static/src/core/builder_options_plugin.js | 6 +++++ .../static/src/core/clipboard_plugin.js | 10 +++++-- .../html_editor/static/src/core/dom_plugin.js | 26 ++++++++++++++++--- .../static/src/core/selection_plugin.js | 5 ++++ .../static/src/main/list/list_plugin.js | 10 +++---- 5 files changed, 47 insertions(+), 10 deletions(-) diff --git a/addons/html_builder/static/src/core/builder_options_plugin.js b/addons/html_builder/static/src/core/builder_options_plugin.js index 6ca6c7fd4cdbb..3922fc3ef1468 100644 --- a/addons/html_builder/static/src/core/builder_options_plugin.js +++ b/addons/html_builder/static/src/core/builder_options_plugin.js @@ -31,6 +31,7 @@ export class BuilderOptionsPlugin extends Plugin { clean_for_save_handlers: this.cleanForSave.bind(this), post_undo_handlers: this.restoreContainer.bind(this), post_redo_handlers: this.restoreContainer.bind(this), + node_replaced_handlers: this.onNodeReplaced.bind(this), // Resources definitions: remove_disabled_reason_providers: [ // ({ el, reasons }) => { @@ -268,6 +269,11 @@ export class BuilderOptionsPlugin extends Plugin { this.updateContainers(revertedStep.extraStepInfos.optionSelection); } } + onNodeReplaced(node, newNode) { + if (this.target === node) { + this.target = newNode; + } + } getRemoveDisabledReason(el) { const reasons = []; this.dispatchTo("remove_disabled_reason_providers", { el, reasons }); diff --git a/addons/html_editor/static/src/core/clipboard_plugin.js b/addons/html_editor/static/src/core/clipboard_plugin.js index aeefd86bafa4b..6a42b283a63a4 100644 --- a/addons/html_editor/static/src/core/clipboard_plugin.js +++ b/addons/html_editor/static/src/core/clipboard_plugin.js @@ -1,7 +1,13 @@ import { isTextNode, isParagraphRelatedElement } from "../utils/dom_info"; import { Plugin } from "../plugin"; import { closestBlock } from "../utils/blocks"; -import { removeClass, removeStyle, unwrapContents, wrapInlinesInBlocks, splitTextNode } from "../utils/dom"; +import { + removeClass, + removeStyle, + unwrapContents, + wrapInlinesInBlocks, + splitTextNode, +} from "../utils/dom"; import { childNodes, closestElement } from "../utils/dom_traversal"; import { parseHTML } from "../utils/html"; import { @@ -305,7 +311,7 @@ export class ClipboardPlugin extends Plugin { blockBefore.before(div); div.replaceChildren(...childNodes(blockBefore)); blockBefore.remove(); - cursors.remapNode(blockBefore, div).restore(); + this.dependencies.dom.remapNode(blockBefore, div, cursors).restore(); } } } diff --git a/addons/html_editor/static/src/core/dom_plugin.js b/addons/html_editor/static/src/core/dom_plugin.js index 66231e65f0cd9..fdcb83ea97512 100644 --- a/addons/html_editor/static/src/core/dom_plugin.js +++ b/addons/html_editor/static/src/core/dom_plugin.js @@ -60,12 +60,15 @@ function getConnectedParents(nodes) { * @typedef {Object} DomShared * @property { DomPlugin['insert'] } insert * @property { DomPlugin['copyAttributes'] } copyAttributes + * @property { DomPlugin['setTag'] } setTag + * @property { DomPlugin['setTagName'] } setTagName + * @property { DomPlugin['remapNode'] } remapNode */ export class DomPlugin extends Plugin { static id = "dom"; static dependencies = ["baseContainer", "selection", "history", "split", "delete", "lineBreak"]; - static shared = ["insert", "copyAttributes", "setTag", "setTagName"]; + static shared = ["insert", "copyAttributes", "setTag", "setTagName", "remapNode"]; resources = { user_commands: [ { id: "insertFontAwesome", run: this.insertFontAwesome.bind(this) }, @@ -560,7 +563,7 @@ export class DomPlugin extends Plugin { continue; } const newEl = this.setTagName(block, tagName); - cursors.remapNode(block, newEl); + this.remapNode(block, newEl, cursors); // We want to be able to edit the case `

` // but in that case, we want to display "Header 2" and // not "Header 3" as it is more important to display @@ -580,7 +583,7 @@ export class DomPlugin extends Plugin { // into it instead. newCandidate.append(...childNodes(block)); block.append(newCandidate); - cursors.remapNode(block, newCandidate); + this.remapNode(block, newCandidate, cursors); } } cursors.restore(); @@ -597,4 +600,21 @@ export class DomPlugin extends Plugin { } } } + + /** + * Remap the given node to another in the given cursors, if any, and + * dispatch `node` and `newNode` to any subscribing plugin so they can be + * informed that the latter replaces the former. + * + * @param {Node} node + * @param {Node} newNode + * @param {import("./selection_plugin.js").Cursors} [cursors] + * @returns + */ + remapNode(node, newNode, cursors) { + this.dispatchTo("node_replaced_handlers", node, newNode); + if (cursors) { + return cursors.remapNode(node, newNode); + } + } } diff --git a/addons/html_editor/static/src/core/selection_plugin.js b/addons/html_editor/static/src/core/selection_plugin.js index f7e0db306e9fc..01516a326cd75 100644 --- a/addons/html_editor/static/src/core/selection_plugin.js +++ b/addons/html_editor/static/src/core/selection_plugin.js @@ -639,6 +639,11 @@ export class SelectionPlugin extends Plugin { callback(focus); return this; }, + /** + * This should not be called directly but instead should be called + * by `DomPlugin.remapNode`. + * @see DomPlugin.remapNode + */ remapNode(node, newNode) { return this.update((cursor) => { if (cursor.node === node) { diff --git a/addons/html_editor/static/src/main/list/list_plugin.js b/addons/html_editor/static/src/main/list/list_plugin.js index 9ecfbdf02145e..b00dc5508bf9e 100644 --- a/addons/html_editor/static/src/main/list/list_plugin.js +++ b/addons/html_editor/static/src/main/list/list_plugin.js @@ -266,7 +266,7 @@ export class ListPlugin extends Plugin { for (const list of listsToSwitch) { const cursors = this.dependencies.selection.preserveSelection(); const newList = this.switchListMode(list, mode); - cursors.remapNode(list, newList).restore(); + this.dependencies.dom.remapNode(list, newList, cursors).restore(); } for (const block of nonListBlocks) { const list = this.blockToList(block, mode, listStyle); @@ -364,14 +364,14 @@ export class ListPlugin extends Plugin { this.dependencies.dom.copyAttributes(baseContainer, list); this.adjustListPadding(list); baseContainer.remove(); - cursors.remapNode(baseContainer, list.firstChild).restore(); + this.dependencies.dom.remapNode(baseContainer, list.firstChild, cursors).restore(); return list; } blockContentsToList(block, mode) { const cursors = this.dependencies.selection.preserveSelection(); const list = insertListAfter(this.document, block.lastChild, mode, [...block.childNodes]); - cursors.remapNode(block, list.firstChild).restore(); + this.dependencies.dom.remapNode(block, list.firstChild, cursors).restore(); return list; } @@ -658,7 +658,7 @@ export class ListPlugin extends Plugin { const baseContainer = this.dependencies.baseContainer.createBaseContainer(); baseContainer.append(this.document.createElement("br")); li.append(baseContainer); - cursors.remapNode(li, baseContainer); + this.dependencies.dom.remapNode(li, baseContainer, cursors); } // Move LI's children to after UL const blocksToMove = childNodes(li); @@ -675,7 +675,7 @@ export class ListPlugin extends Plugin { const wrapper = this.document.createElement(tag); wrapper.append(...parent.childNodes); parent.replaceChildren(wrapper); - cursors.remapNode(parent, wrapper); + this.dependencies.dom.remapNode(parent, wrapper, cursors); return wrapper; }; for (const block of blocksToMove) {