From 93151fb73b527608e545c0cbda8fb3ee6b5af8e0 Mon Sep 17 00:00:00 2001 From: Antoine Guenet Date: Mon, 3 Jan 2022 12:17:29 +0000 Subject: [PATCH] [FIX] web_editor: properly apply color in nested inline contexts The colorpicker failed in cases like: ```xml abc ``` where we select the letter "b". This was because `applyColor` only considered direct children of the `font` element. This extends it to deal with further descendants of it. task-2623347 closes odoo/odoo#83254 X-original-commit: 5d8d98812a4f351abd27389c4992bf9e753833b7 Signed-off-by: David Monjoie (dmo) --- .../lib/odoo-editor/src/commands/commands.js | 12 ++-- .../static/lib/odoo-editor/src/utils/utils.js | 52 ++++++++++++++ .../lib/odoo-editor/test/spec/editor.test.js | 12 ++++ .../lib/odoo-editor/test/spec/utils.test.js | 71 +++++++++++++++++++ 4 files changed, 142 insertions(+), 5 deletions(-) diff --git a/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js b/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js index d67cf0a2dda..c4a4b4de663 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js +++ b/addons/web_editor/static/lib/odoo-editor/src/commands/commands.js @@ -1,6 +1,7 @@ /** @odoo-module **/ import { ancestors, + descendants, childNodeIndex, closestBlock, closestElement, @@ -29,6 +30,7 @@ import { setSelection, setCursorStart, setTagName, + splitAroundUntil, splitElement, splitTextNode, startPos, @@ -505,15 +507,15 @@ export const editorCommands = { const selectedNodes = getSelectedNodes(editor.editable); const fonts = selectedNodes.flatMap(node => { let font = closestElement(node, 'font'); - const children = font && [...font.childNodes]; + const children = font && descendants(font); if (font && font.nodeName === 'FONT') { // Partially selected : split it. const selectedChildren = children.filter(child => selectedNodes.includes(child)); if (selectedChildren.length) { - const after = selectedChildren[selectedChildren.length - 1].nextSibling; - font = after ? splitElement(font, childNodeIndex(after))[0] : font; - const before = selectedChildren[0].previousSibling; - font = before ? splitElement(font, childNodeIndex(before) + 1)[1] : font; + const splitResult = splitAroundUntil(selectedChildren, font); + font = splitResult[0] || splitResult[1] || font; + } else { + font = []; } } else if (node.nodeType === Node.TEXT_NODE && isVisibleStr(node)) { // Node is a visible text node: wrap it in a . diff --git a/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js b/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js index 630dc25dcae..0bc25ea635a 100644 --- a/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js +++ b/addons/web_editor/static/lib/odoo-editor/src/utils/utils.js @@ -261,6 +261,20 @@ export function ancestors(node, editable) { return [node.parentElement, ...ancestors(node.parentElement, editable)]; } +/** + * Take a node, return all of its descendants, in depth-first order. + * + * @param {Node} node + * @returns {Node[]} + */ +export function descendants(node) { + const posterity = []; + for (const child of (node.childNodes || [])) { + posterity.push(child, ...descendants(child)); + } + return posterity; +} + export function closestBlock(node) { return findNode(closestPath(node), node => isBlock(node)); } @@ -1320,6 +1334,44 @@ export function splitElement(element, offset) { return [before, after]; } +/** + * Split around the given elements, until a given ancestor (included). Elements + * will be removed in the process so caution is advised in dealing with their + * references. Returns a tuple containing the new elements on both sides of the + * split. + * + * @see splitElement + * @param {Node[] | Node} elements + * @param {Node} limitAncestor + * @returns {[Node, Node]} + */ +export function splitAroundUntil(elements, limitAncestor) { + elements = Array.isArray(elements) ? elements : [elements]; + let after = elements[elements.length - 1].nextSibling; + let newUntil = limitAncestor; + let beforeSplit, afterSplit; + // Split up ancestors up to font + while (after && after.parentElement !== limitAncestor) { + afterSplit = splitElement(after.parentElement, childNodeIndex(after))[0]; + newUntil = afterSplit; + after = newUntil.nextSibling; + } + if (after) { + afterSplit = splitElement(limitAncestor, childNodeIndex(after))[0]; + limitAncestor = afterSplit; + } + let before = elements[0].previousSibling; + while (before && before.parentElement !== limitAncestor) { + beforeSplit = splitElement(before.parentElement, childNodeIndex(before) + 1)[1]; + newUntil = beforeSplit; + before = newUntil.previousSibling; + } + if (before) { + beforeSplit = splitElement(limitAncestor, childNodeIndex(before) + 1)[1]; + } + return [beforeSplit, afterSplit]; +} + export function insertText(sel, content) { if (sel.anchorNode.nodeType == Node.TEXT_NODE) { const pos = [sel.anchorNode.parentElement, splitTextNode(sel.anchorNode, sel.anchorOffset)]; diff --git a/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js b/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js index 9b5fd8c7892..84bee154759 100644 --- a/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js +++ b/addons/web_editor/static/lib/odoo-editor/test/spec/editor.test.js @@ -3117,4 +3117,16 @@ X[] }); }); }); + + describe('applyColor', () => { + it('should apply a color to a slice of text in a span in a font', async () => { + await testEditor(BasicEditor, { + contentBefore: '

abc[def]ghi

', + stepFunction: editor => editor.execCommand('applyColor', 'rgb(255, 0, 0)', 'color'), + contentAfter: '

abc' + + '[def]' + + 'ghi

', + }); + }); + }); }); diff --git a/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js b/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js index 0c4b7c594eb..70c3acf3236 100644 --- a/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js +++ b/addons/web_editor/static/lib/odoo-editor/test/spec/utils.test.js @@ -5,6 +5,7 @@ import { childNodeIndex, closestBlock, closestElement, + descendants, endPos, firstLeaf, getAdjacentPreviousSiblings, @@ -26,6 +27,8 @@ import { setSelection, setCursorStart, setCursorEnd, + splitAroundUntil, + splitTextNode, getCursorDirection, DIRECTIONS, isBlock, @@ -351,6 +354,22 @@ describe('Utils', () => { window.chai.expect(result).to.eql([editable]); }); }); + describe('descendants', () => { + it('should find all the descendants of a div in depth-first order', () => { + const [div] = insertTestHtml( + '

abc

def

', + ); + window.chai.expect(descendants(div)).to.eql([ + div.firstChild, //
... + div.firstChild.firstChild, //
... + div.firstChild.firstChild.firstChild, //

abc

+ div.firstChild.firstChild.firstChild.firstChild, // "abc" + div.firstChild.firstChild.childNodes[1], //

def

+ div.firstChild.firstChild.childNodes[1].firstChild, //

def

+ div.firstChild.firstChild.childNodes[1].firstChild.firstChild, // "def" + ]); + }); + }); describe('closestBlock', () => { it('should find the closest block of a deeply nested text node', () => { const [div] = insertTestHtml( @@ -1393,4 +1412,56 @@ describe('Utils', () => { }); }); }); + + //-------------------------------------------------------------------------- + // DOM Modification + //-------------------------------------------------------------------------- + + describe('splitAroundUntil', () => { + it('should split a slice of text from its inline ancestry', () => { + const [p] = insertTestHtml('

abcdefg

'); + const cde = p.childNodes[1].childNodes[1].firstChild; + // We want to test with "cde" being three separate text nodes. + splitTextNode(cde, 2); + const cd = cde.previousSibling; + splitTextNode(cd, 1); + const d = cd; + const result = splitAroundUntil(d, p.childNodes[1]); + window.chai.expect(result.every(node => node.tagName === 'FONT')).to.be.ok; + window.chai.expect(p.outerHTML).to.eql( + '

abcdefg

' + ); + window.chai.expect(result[0]).to.eql(p.childNodes[2]); + // "before" split happened so "after" split is not in the DOM anymore: + window.chai.expect(result[1].parentElement).to.eql(null); + }); + it('should split a slice of text from its inline ancestry', () => { + const [p] = insertTestHtml('

abcdefghi

'); + const cdefg = p.childNodes[1].childNodes[1].firstChild; + // We want to test with "cdefg" being five separate text nodes. + splitTextNode(cdefg, 4); + const cdef = cdefg.previousSibling; + splitTextNode(cdef, 3); + const cde = cdef.previousSibling; + splitTextNode(cde, 2); + const cd = cde.previousSibling; + splitTextNode(cd, 1); + const d = cd; + const result = splitAroundUntil([d, d.nextSibling.nextSibling], p.childNodes[1]); + window.chai.expect(result.every(node => node.tagName === 'FONT')).to.be.ok; + window.chai.expect(p.outerHTML).to.eql( + '

abcdefghi

' + ); + window.chai.expect(result[0]).to.eql(p.childNodes[2]); + // "before" split happened so "after" split is not in the DOM anymore: + window.chai.expect(result[1].parentElement).to.eql(null); + }); + it('should not do anything (nothing to split)', () => { + const [p] = insertTestHtml('

abcdefg

'); + const cde = p.childNodes[1].childNodes[1].firstChild; + const result = splitAroundUntil(cde, p.childNodes[1]); + window.chai.expect(result.every(node => node === undefined)).to.be.ok; + window.chai.expect(p.outerHTML).to.eql('

abcdefg

'); + }); + }); });