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

[FIX] web_editor: preserve leading spaces on delete range

Browse Source 
When pressing delete on "[a] b c" the result is " b c" instead of
" b c". As result, text is rendered as "[]b c". The cause lies in
`deleteRange`: it ensures trailing spaces remains visible, but does not
take care of eventual invisible leading spaces. This adapts it to make
use of `prepareUpdate` which is made to handle this sort of issues. In
so doing we need to adapt `getState` and `prepareUpdate` to handle the
special cases brought by `deleteRange`. An option is introduced in
`prepareUpdate` to prevent its reentry until the update is complete (as
otherwise we may restore invalid intermediary states in the middle of
the deletion process).

task-3143042

X-original-commit: 8fd69afa591d11afe07da803ec550f052402713f
Part-of: odoo/odoo#138549
This commit is contained in:
Antoine Guenet
2023-10-13 07:33:55 +00:00
parent 8c759e2277
commit 0c95529e0f
5 changed files with 94 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web_editor/static/src/js/editor/odoo-editor/src/OdooEditor.js
+31 -29
View File
@@ -54,7 +54,6 @@ import {
rightPos,
getAdjacentPreviousSiblings,
getAdjacentNextSiblings,
rightLeafOnlyNotBlockPath,
isBlock,
getTraversedNodes,
getSelectedNodes,
@@ -77,6 +76,8 @@ import {
isNotAllowedContent,
childNodeIndex,
EMAIL_REGEX,
prepareUpdate,
boundariesOut,
} from './utils/utils.js';
import { editorCommands } from './commands/commands.js';
import { Powerbox } from './powerbox/Powerbox.js';
@@ -272,6 +273,8 @@ export class OdooEditor extends EventTarget {
this.isMobile = matchMedia('(max-width: 767px)').matches;
this.isFirefox = navigator.userAgent.toLowerCase().indexOf('firefox') > -1;
this.isPrepareUpdateLocked = false;
// Keyboard type detection, happens only at the first keydown event.
this.keyboardType = KEYBOARD_TYPES.UNKNOWN;
@@ -2062,17 +2065,28 @@ export class OdooEditor extends EventTarget {
range.startContainer.before(zws);
insertedZws = zws;
}
let start = range.startContainer;
const startBlock = closestBlock(start);
let end = range.endContainer;
const endBlock = closestBlock(end);
// Let the DOM split and delete the range.
let { startContainer: start, startOffset, endContainer: end, endOffset } = range;
const [startBlock, endBlock] = [closestBlock(start), closestBlock(end)];
const doJoin =
(closestBlock(start) !== closestBlock(range.commonAncestorContainer) ||
closestBlock(end) !== closestBlock(range.commonAncestorContainer))
&& (closestBlock(start).tagName !== 'TD' && closestBlock(end).tagName !== 'TD');
(startBlock !== closestBlock(range.commonAncestorContainer) ||
endBlock !== closestBlock(range.commonAncestorContainer))
&& (startBlock.tagName !== 'TD' && endBlock.tagName !== 'TD');
let next = nextLeaf(end, this.editable);
// Get the boundaries of the range so as to get the state to restore.
if (end.nodeType === Node.TEXT_NODE) {
splitTextNode(end, endOffset);
endOffset = nodeSize(end);
}
if (start.nodeType === Node.TEXT_NODE) {
splitTextNode(start, startOffset);
startOffset = 0;
}
const restoreUpdate = prepareUpdate(...boundariesOut(start).slice(0, 2), ...boundariesOut(end).slice(2, 4), { allowReenter: false });
// Let the DOM split and delete the range.
const contents = range.extractContents();
setSelection(start, nodeSize(start));
range = getDeepRange(this.editable, { sel });
// Restore unremovables removed by extractContents.
@@ -2105,16 +2119,7 @@ export class OdooEditor extends EventTarget {
fillEmpty(closestBlock(start));
}
fillEmpty(closestBlock(range.endContainer));
// Ensure trailing space remains visible.
const joinWith = range.endContainer;
const oldText = joinWith.textContent;
const rightLeaf = rightLeafOnlyNotBlockPath(range.endContainer).next().value;
const hasSpaceAfter = !rightLeaf || rightLeaf.textContent.startsWith(' ');
const shouldPreserveSpace = (doJoin || hasSpaceAfter) && joinWith && oldText.endsWith(' ');
if (shouldPreserveSpace) {
joinWith.textContent = oldText.replace(/ $/, '\u00A0');
setSelection(joinWith, nodeSize(joinWith));
}
// Rejoin blocks that extractContents may have split in two.
while (
doJoin &&
@@ -2137,9 +2142,9 @@ export class OdooEditor extends EventTarget {
break;
}
}
// If the oDeleteBackward loop have emptied the start block and the
// range end in another element (rangeStart != rangeEnd), we delete
// the start block and move the cursor to the end block.
// If the oDeleteBackward loop emptied the start block and the range
// ends in another element (rangeStart !== rangeEnd), we delete the
// start block and move the cursor to the end block.
if (
startBlock &&
startBlock.textContent === '\u200B' &&
@@ -2157,21 +2162,18 @@ export class OdooEditor extends EventTarget {
// parent styles, then call `fillEmpty` to properly add a flagged
// zws if still needed.
const el = closestElement(insertedZws);
const next = insertedZws.nextSibling;
insertedZws.remove();
el && fillEmpty(el);
}
next = range.endContainer && rightLeafOnlyNotBlockPath(range.endContainer).next().value;
if (
shouldPreserveSpace && next && !(next && next.nodeType === Node.TEXT_NODE && next.textContent.startsWith(' '))
) {
// Restore the text we modified in order to preserve trailing space.
joinWith.textContent = oldText;
setSelection(joinWith, nodeSize(joinWith));
setSelection(next, 0);
}
if (joinWith) {
const el = closestElement(joinWith);
el && fillEmpty(el);
}
const restoreCursor = preserveCursor(this.document);
restoreUpdate();
restoreCursor();
}
/**
addons/web_editor/static/src/js/editor/odoo-editor/src/utils/utils.js
+33 -7
View File
@@ -1724,7 +1724,7 @@ export function toggleClass(node, className) {
* @returns {boolean}
*/
export function isFakeLineBreak(brEl) {
return !(getState(...rightPos(brEl), DIRECTIONS.RIGHT).cType & (CTGROUPS.INLINE | CTGROUPS.BR));
return !(getState(...rightPos(brEl), DIRECTIONS.RIGHT).cType & (CTYPES.CONTENT | CTGROUPS.BR));
}
/**
* Checks whether or not the given block has any visible content, except for
@@ -2058,6 +2058,7 @@ export function resetOuids(node) {
// Prepare / Save / Restore state utilities
//------------------------------------------------------------------------------
const prepareUpdateLockedEditables = new Set();
/**
* Any editor command is applied to a selection (collapsed or not). After the
* command, the content type on the selection boundaries, in both direction,
@@ -2076,9 +2077,24 @@ export function resetOuids(node) {
* @param {...(HTMLElement|number)} args - argument 1 and 2 can be repeated for
* multiple preparations with only one restore callback returned. Note: in
* that case, the positions should be given in the document node order.
* @param {Object} [options]
* @param {boolean} [options.allowReenter = true] - if false, all calls to
* prepareUpdate before this one gets restored will be ignored.
* @returns {function}
*/
export function prepareUpdate(...args) {
const closestRoot = args.length && ancestors(args[0]).find(ancestor => ancestor.oid === 'root');
const isPrepareUpdateLocked = closestRoot && prepareUpdateLockedEditables.has(closestRoot);
const options = {
allowReenter: true,
...(args.length && args[args.length - 1] instanceof Object ? args.pop() : {}),
};
if (isPrepareUpdateLocked) {
return () => {};
}
if (!options.allowReenter && closestRoot) {
prepareUpdateLockedEditables.add(closestRoot);
}
const positions = [...args];
// Check the state in each direction starting from each position.
@@ -2100,6 +2116,9 @@ export function prepareUpdate(...args) {
for (const data of restoreData) {
restoreState(data);
}
if (!options.allowReenter && closestRoot) {
prepareUpdateLockedEditables.delete(closestRoot);
}
};
}
/**
@@ -2123,16 +2142,15 @@ export function getState(el, offset, direction, leftCType) {
let domPath;
let inverseDOMPath;
let whitespaceAtEdgeRegex;
const whitespaceAtStartRegex = new RegExp('^' + whitespace + '+');
const whitespaceAtEndRegex = new RegExp(whitespace + '+$');
const reasons = [];
if (direction === DIRECTIONS.LEFT) {
domPath = leftDOMPath(el, offset, reasons);
inverseDOMPath = rightDOMPath(el, offset);
whitespaceAtEdgeRegex = new RegExp(whitespace + '+$');
} else {
domPath = rightDOMPath(el, offset, reasons);
inverseDOMPath = leftDOMPath(el, offset);
whitespaceAtEdgeRegex = new RegExp('^' + whitespace + '+');
}
// TODO I think sometimes, the node we have to consider as the
@@ -2157,7 +2175,13 @@ export function getState(el, offset, direction, leftCType) {
// visible if we have content backwards.
if (direction === DIRECTIONS.LEFT) {
if (!isWhitespace(value)) {
cType = lastSpace || whitespaceAtEdgeRegex.test(value) ? CTYPES.SPACE : CTYPES.CONTENT;
if (lastSpace) {
cType = CTYPES.SPACE;
} else {
const rightLeaf = rightLeafOnlyNotBlockPath(node).next().value;
const hasContentRight = rightLeaf && !whitespaceAtStartRegex.test(rightLeaf.textContent);
cType = !hasContentRight && whitespaceAtEndRegex.test(node.textContent) ? CTYPES.SPACE : CTYPES.CONTENT;
}
break;
}
if (value.length) {
@@ -2165,12 +2189,14 @@ export function getState(el, offset, direction, leftCType) {
}
} else {
leftCType = leftCType || getState(el, offset, DIRECTIONS.LEFT).cType;
if (whitespaceAtEdgeRegex.test(value)) {
if (whitespaceAtStartRegex.test(value)) {
const leftLeaf = leftLeafOnlyNotBlockPath(node).next().value;
const hasContentLeft = leftLeaf && !whitespaceAtEndRegex.test(leftLeaf.textContent);
const rct = !isWhitespace(value)
? CTYPES.CONTENT
: getState(...rightPos(node), DIRECTIONS.RIGHT).cType;
cType =
leftCType & CTYPES.CONTENT && rct & (CTYPES.CONTENT | CTYPES.BR)
leftCType & CTYPES.CONTENT && rct & (CTYPES.CONTENT | CTYPES.BR) && !hasContentLeft
? CTYPES.SPACE
: rct;
break;
addons/web_editor/static/src/js/editor/odoo-editor/test/spec/editor.test.js
+27 -3
View File
@@ -1239,7 +1239,7 @@ X[]
stepFunction: deleteForward,
contentAfter: '<h1>ab []gh</h1>',
});
// // Backward selection
// Backward selection
await testEditor(BasicEditor, {
contentBefore: '<h1>ab ]cd</h1><p>ef[gh</p>',
stepFunction: deleteForward,
@@ -2637,7 +2637,19 @@ X[]
contentAfter: '<p>ab[]de</p>',
});
});
it('should delete a one letter word', async () => {
it('should delete a one letter word followed by visible space (start of block)', async () => {
await testEditor(BasicEditor, {
contentBefore: '<p>a[] b</p>',
stepFunction: deleteBackward,
contentAfter: '<p>[]&nbsp;b</p>',
});
await testEditor(BasicEditor, {
contentBefore: '<p>[a] b</p>',
stepFunction: deleteBackward,
contentAfter: '<p>[]&nbsp;b</p>',
});
});
it('should delete a one letter word surrounded by visible space', async () => {
await testEditor(BasicEditor, {
contentBefore: '<p>ab c[] de</p>',
stepFunction: deleteBackward,
@@ -2646,7 +2658,19 @@ X[]
await testEditor(BasicEditor, {
contentBefore: '<p>ab [c] de</p>',
stepFunction: deleteBackward,
contentAfter: '<p>ab&nbsp;[] de</p>',
contentAfter: '<p>ab []&nbsp;de</p>',
});
});
it('should delete a one letter word preceded by visible space (end of block)', async () => {
await testEditor(BasicEditor, {
contentBefore: '<p>a b[]</p>',
stepFunction: deleteBackward,
contentAfter: '<p>a&nbsp;[]</p>',
});
await testEditor(BasicEditor, {
contentBefore: '<p>a [b]</p>',
stepFunction: deleteBackward,
contentAfter: '<p>a&nbsp;[]</p>',
});
});
it('should delete an empty paragraph in a table cell', () =>
addons/web_editor/static/src/js/editor/odoo-editor/test/spec/format.test.js
+1 -6
View File
@@ -554,12 +554,7 @@ describe('Format', () => {
stepFunction: async editor => {
insertText(editor, 'A');
},
contentAfter: `<p>A[]${s(`<br><br>`)}</p>`,
// Note: In the browser, the actual result is the following:
// contentAfter: `<p>${s(`A[]<br><br>`)}</p>`,
// It is arguable which version is better than the other but in
// any case this is a trade-off because it matches the native
// behavior of contentEditable in that case.
contentAfter: `<p>${s(`A[]<br><br>`)}</p>`,
});
});
it('should not format non-editable text (strikeThrough)', async () => {
addons/web_editor/static/src/js/editor/odoo-editor/test/spec/link.test.js
+2 -2
View File
@@ -105,13 +105,13 @@ describe('Link', () => {
testUrlRegex('google.com/foo/?data=hello&data2=foo#anchor', { expectedUrl: 'google.com/foo/?data=hello&data2=foo#anchor' });
});
describe('insert Link', () => {
// This fails, but why would the cursor stay inside the link
// if the next text insert should be outside of the link (see next test)
describe('range collapsed', () => {
it('should insert a link and preserve spacing', async () => {
await testEditor(BasicEditor, {
contentBefore: '<p>a [] c</p>',
stepFunction: createLink,
// Two consecutive spaces like one so `a [] c` is
// effectively the same as `a []c`.
contentAfter: '<p>a <a href="#">link</a>[]c</p>',
});
});