From 544afd6de917c806ea0107a00e1c9a87ba4e9345 Mon Sep 17 00:00:00 2001 From: Nicolas Bayet Date: Wed, 7 Dec 2022 10:18:46 +0000 Subject: [PATCH] [FIX] web_editor: prevent error on font-size change Before this commit, changing the font size of an element with one ancestor having the same font size could trigger a traceback. For fixing that issue some code has to be refactored to handle the setFontSize case in formatSelection. As a side effect of this refactoring, nested font-size is now properly handled (see added tests). task-3086301 closes odoo/odoo#108633 X-original-commit: d003b82043468bcf609006c5910ac8fc9520e136 Signed-off-by: David Monjoie (dmo) --- .../js/editor/odoo-editor/src/utils/utils.js | 101 ++++++------------ .../odoo-editor/test/spec/format.test.js | 83 ++++++++++++-- 2 files changed, 107 insertions(+), 77 deletions(-) diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js index 49f1fad98bd..189c2580473 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js @@ -914,7 +914,7 @@ const formatsSpecs = { }, fontSize: { isFormatted: isFontSize, - hasStyle: (node, props) => node.style && node.style['font-size'] === props.size, + hasStyle: (node) => node.style && node.style['font-size'], addStyle: (node, props) => node.style['font-size'] = props.size, removeStyle: (node) => removeStyle(node, 'font-size'), }, @@ -954,7 +954,7 @@ const removeFormat = (node, formatSpec) => { } } - if (formatSpec.isTag(node)) { + if (formatSpec.isTag && formatSpec.isTag(node)) { const attributesNames = node.getAttributeNames().filter((name)=> { return name !== 'data-oe-zws-empty-inline'; }); @@ -1008,79 +1008,48 @@ export const formatSelection = (editor, formatName, {applyStyle, formatProps} = const formatSpec = formatsSpecs[formatName]; for (const selectedTextNode of selectedTextNodes) { - // Compute inline ancestors until finding one which is a block or has a class. const inlineAncestors = []; - let node = selectedTextNode; - while ((node = node.parentElement) && (!isBlock(node) && !(node.classList && node.classList.length))) { - inlineAncestors.unshift(node); - } - const blockOrInlineClass = node; + let currentNode = selectedTextNode; + let parentNode = selectedTextNode.parentElement; - const firstBlockOrClassHasFormat = formatSpec.isFormatted(blockOrInlineClass, formatProps); + // Remove the format on all inline ancestors until a block or an element + // with a class (in case the formating comes from the class). + while (parentNode && (!isBlock(parentNode) && !(parentNode.classList && parentNode.classList.length))) { + const isUselessZws = parentNode.tagName === 'SPAN' && + parentNode.hasAttribute('data-oe-zws-empty-inline') && + parentNode.getAttributeNames().length === 1; - let previousAncestorHasFormat = firstBlockOrClassHasFormat; - let lastAncestorInlineFormat; - - for (const ancestor of inlineAncestors) { - const ancestorIsTag = formatSpec.isTag && formatSpec.isTag(ancestor); - const ancestorHasStyle = formatSpec.hasStyle && formatSpec.hasStyle(ancestor, formatProps); - const ancestorTagIsFormated = ancestorIsTag || ancestorHasStyle; - const isUselessZwsSpan = - ancestor.tagName === 'SPAN' && - ancestor.hasAttribute('data-oe-zws-empty-inline') && - ancestor.getAttributeNames().length === 1; - const ancestorIsFormatted = formatSpec.isFormatted(ancestor, formatProps); - - if (ancestorTagIsFormated && previousAncestorHasFormat !== ancestorIsFormatted) { - if (lastAncestorInlineFormat) { - // Remove format on two node that switch the format to flatten the DOM. - const newLastAncestorInlineFormat = splitAroundUntil(ancestor, lastAncestorInlineFormat); - removeFormat(newLastAncestorInlineFormat, formatSpec); - removeFormat(ancestor, formatSpec); - lastAncestorInlineFormat = undefined; - } else { - lastAncestorInlineFormat = ancestor; + if (isUselessZws) { + unwrapContents(parentNode); + } else { + const newLastAncestorInlineFormat = splitAroundUntil(currentNode, parentNode); + removeFormat(newLastAncestorInlineFormat, formatSpec); + if (newLastAncestorInlineFormat.isConnected) { + inlineAncestors.push(newLastAncestorInlineFormat); + currentNode = newLastAncestorInlineFormat; } - // if there is two consecutive format that are the same, remove one. - } else if ( - (ancestorIsTag || ancestorHasStyle) && - ( - previousAncestorHasFormat || - (!previousAncestorHasFormat && !ancestorIsFormatted) - ) - ) { - removeFormat(ancestor, formatSpec); - } else if (isUselessZwsSpan) { - unwrapContents(ancestor); } - previousAncestorHasFormat = ancestor.parentElement && formatSpec.isFormatted(ancestor, formatProps); + + parentNode = currentNode.parentElement; } - if (selectedTextNode.nodeType === Node.TEXT_NODE) { - const isLeafNodeFormated = formatSpec.isFormatted(selectedTextNode, formatProps); - if (isLeafNodeFormated !== applyStyle) { - if (lastAncestorInlineFormat) { - const splitNode = splitAroundUntil(selectedTextNode, lastAncestorInlineFormat); - removeFormat(splitNode, formatSpec); - } else { - if (firstBlockOrClassHasFormat && !applyStyle) { - formatSpec.addNeutralStyle && formatSpec.addNeutralStyle(getOrCreateSpan(selectedTextNode, inlineAncestors)); - } else if (!firstBlockOrClassHasFormat && applyStyle) { - const tag = formatSpec.tagName && document.createElement(formatSpec.tagName); - if (tag) { - selectedTextNode.after(tag); - tag.append(selectedTextNode); + const firstBlockOrClassHasFormat = formatSpec.isFormatted(parentNode, formatProps); - if (!formatSpec.isFormatted(tag, formatProps)) { - tag.after(selectedTextNode); - tag.remove(); - formatSpec.addStyle(getOrCreateSpan(selectedTextNode, inlineAncestors), formatProps); - } - } else { - formatSpec.addStyle(getOrCreateSpan(selectedTextNode, inlineAncestors), formatProps); - } - } + if (firstBlockOrClassHasFormat && !applyStyle) { + formatSpec.addNeutralStyle && formatSpec.addNeutralStyle(getOrCreateSpan(selectedTextNode, inlineAncestors)); + } else if (!firstBlockOrClassHasFormat && applyStyle) { + const tag = formatSpec.tagName && document.createElement(formatSpec.tagName); + if (tag) { + selectedTextNode.after(tag); + tag.append(selectedTextNode); + + if (!formatSpec.isFormatted(tag, formatProps)) { + tag.after(selectedTextNode); + tag.remove(); + formatSpec.addStyle(getOrCreateSpan(selectedTextNode, inlineAncestors), formatProps); } + } else { + formatSpec.addStyle(getOrCreateSpan(selectedTextNode, inlineAncestors), formatProps); } } } diff --git a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js index 4aeae1f0d4f..30646c2c185 100644 --- a/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js +++ b/addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js @@ -1,5 +1,5 @@ import { isSelectionFormat } from '../../src/utils/utils.js'; -import { BasicEditor, testEditor, setTestSelection, Direction } from '../utils.js'; +import { BasicEditor, testEditor, setTestSelection, Direction, unformat } from '../utils.js'; const bold = async editor => { await editor.execCommand('bold'); @@ -139,9 +139,23 @@ describe('Format', () => { }); it('should remove a bold tag that was redondant with different tags while performing the command', async () => { await testEditor(BasicEditor, { - contentBefore: `

a${spanBold(`b${strong(`c${b(`[d]`)}e`)}f`)}g

`, + contentBefore: unformat(`

+ a + + b + c[d]e + f + + g +

`), stepFunction: bold, - contentAfter: `

a${spanBold(`bc`)}[d]${spanBold(`ef`)}g

`, + contentAfter: unformat(`

+ a + bc + [d] + ef + g +

`), }); }); @@ -535,12 +549,12 @@ describe('Format', () => { await testEditor(BasicEditor, { contentBefore: `

ab${u(s(`cd`))}${s(u(`ghi[]`))}${u(s(`ef`))}

`, stepFunction: underline, - contentAfterEdit: `

ab${u(s(`cd`))}${s(`${u(`ghi`)}${span(`[]\u200b`, 'first')}`)}${u(s(`ef`))}

`, - contentAfter: `

ab${u(s(`cd`))}${s(u(`ghi`) + `[]`)}${u(s(`ef`))}

`, + contentAfterEdit: `

ab${u(s(`cd`))}${s(`${u(`ghi`)}`)}${s(`[]\u200b`, 'first')}${u(s(`ef`))}

`, + // The reason the cursor is after the tag is because when the editor get's cleaned, the zws tag gets deleted. + contentAfter: `

ab${u(s(`cd`))}${s(u(`ghi`))}[]${u(s(`ef`))}

`, }); }); it('should remove underline, write, restore underline, write, remove underline again, write (collapsed, strikeThrough)', async () => { - const uselessSpan = content => `${content}`; // TODO: clean const uselessU = u(''); // TODO: clean await testEditor(BasicEditor, { contentBefore: `

ab${u(s(`cd[]ef`))}

`, @@ -552,7 +566,7 @@ describe('Format', () => { await editor.execCommand('underline'); await editor.execCommand('insert', 'C'); }, - contentAfterEdit: `

ab${u(s(`cd`))}${s(`A${u(`B`)}${uselessSpan(`C[]`)}${uselessU}`)}${u(s(`ef`))}

`, + contentAfterEdit: `

ab${u(s(`cd`))}${s(`A${u(`B`)}C[]${uselessU}`)}${u(s(`ef`))}

`, }); }); it('should remove only underline decoration on a span', async () => { @@ -639,12 +653,12 @@ describe('Format', () => { await testEditor(BasicEditor, { contentBefore: `

ab${u(em(`cd`))}${em(u(`ghi[]`))}${u(em(`ef`))}

`, stepFunction: underline, - contentAfterEdit: `

ab${u(em(`cd`))}${em(u(`ghi`) + `[]\u200b`)}${u(em(`ef`))}

`, - contentAfter: `

ab${u(em(`cd`))}${em(u(`ghi`) + `[]`)}${u(em(`ef`))}

`, + contentAfterEdit: `

ab${u(em(`cd`))}${em(u(`ghi`))}[]\u200b${u(em(`ef`))}

`, + // The reason the cursor is after the tag is because when the editor get's cleaned, the zws tag gets deleted. + contentAfter: `

ab${u(em(`cd`))}${em(u(`ghi`))}[]${u(em(`ef`))}

`, }); }); it('should remove underline, write, restore underline, write, remove underline again, write (collapsed, italic)', async () => { - const uselessSpan = content => `${content}`; const uselessU = u(''); // TODO: clean await testEditor(BasicEditor, { contentBefore: `

ab${u(em(`cd[]ef`))}

`, @@ -656,7 +670,7 @@ describe('Format', () => { await editor.execCommand('underline'); await editor.execCommand('insert', 'C'); }, - contentAfter: `

ab${u(em(`cd`))}${em(`A${u(`B`)}${uselessSpan(`C[]`)}${uselessU}`)}${u(em(`ef`))}

`, + contentAfter: `

ab${u(em(`cd`))}${em(`A${u(`B`)}C[]${uselessU}`)}${u(em(`ef`))}

`, }); }); }); @@ -684,6 +698,53 @@ describe('Format', () => { contentAfter: '

ab[]cd

', }); }); + it('should change the font-size for a character in an inline that has a font-size', async () => { + await testEditor(BasicEditor, { + contentBefore: `

ab[c]de

`, + stepFunction: setFontSize('20px'), + contentAfter: unformat(`

+ a + b + [c] + d + e +

`), + }); + }); + it('should change the font-size of a character with multiples inline ancestors having a font-size', async () => { + await testEditor(BasicEditor, { + contentBefore: unformat(`

+ a + + b + c[d]e + f + + g +

`), + stepFunction: setFontSize('30px'), + contentAfter: unformat(`

+ a + + b + c + + [d] + + e + f + + g +

`), + }); + }); + it('should remove a redundant font-size', async () => { + await testEditor(BasicEditor, { + contentBefore: '

b[c]d

', + stepFunction: setFontSize('10px'), + contentAfter: '

b[c]d

', + }); + }); }); it('should add style to a span parent of an inline', async () => {