From f27607a74cb8b6d2d4f7c724cdae5bfeb6346ff9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 16 May 2019 17:58:22 +0100 Subject: [PATCH 1/4] don't put cursor position in NewlinePart after adding it You can't append to it anyway, so mark it uneditable and skip uneditable parts if that's where an edit ended up. This has the added advantage that if there is text after a newly insert pill, the cursor will be put just before it rather than in the pill, after the last character. --- src/editor/model.js | 12 +++++++++++- src/editor/parts.js | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/editor/model.js b/src/editor/model.js index 85dd425b0e..c6f0c5529f 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -88,7 +88,8 @@ export default class EditorModel { } this._mergeAdjacentParts(); const caretOffset = diff.at - removedOffsetDecrease + addedLen; - const newPosition = this._positionForOffset(caretOffset, true); + let newPosition = this._positionForOffset(caretOffset, true); + newPosition = newPosition.skipUneditableParts(this._parts); this._setActivePart(newPosition); this._updateCallback(newPosition); } @@ -261,4 +262,13 @@ class DocumentPosition { get offset() { return this._offset; } + + skipUneditableParts(parts) { + const part = parts[this.index]; + if (part && !part.canEdit) { + return new DocumentPosition(this.index + 1, 0); + } else { + return this; + } + } } diff --git a/src/editor/parts.js b/src/editor/parts.js index a20b857fee..8ba18997a9 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -205,6 +205,14 @@ export class NewlinePart extends BasePart { get type() { return "newline"; } + + // this makes the cursor skip this part when it is inserted + // rather than trying to append to it, which is what we want. + // As a newline can also be only one character, it makes sense + // as it can only be one character long. This caused #9741. + get canEdit() { + return false; + } } export class RoomPillPart extends PillPart { From 98e033a5292b34e81fa16cbfe86896a6894041f9 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 16 May 2019 18:13:48 +0100 Subject: [PATCH 2/4] don't allow newline parts of longer than one newline --- src/editor/parts.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/editor/parts.js b/src/editor/parts.js index 8ba18997a9..bf792b1ab9 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -57,7 +57,7 @@ class BasePart { appendUntilRejected(str) { for (let i = 0; i < str.length; ++i) { const chr = str.charAt(i); - if (!this.acceptsInsertion(chr)) { + if (!this.acceptsInsertion(chr, i)) { this._text = this._text + str.substr(0, i); return str.substr(i); } @@ -180,8 +180,8 @@ class PillPart extends BasePart { } export class NewlinePart extends BasePart { - acceptsInsertion(chr) { - return this.text.length === 0 && chr === "\n"; + acceptsInsertion(chr, i) { + return (this.text.length + i) === 0 && chr === "\n"; } acceptsRemoval(position, chr) { From 245f48a22c9a7d5de6f2756ad754558f2054835e Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 16 May 2019 18:39:20 +0100 Subject: [PATCH 3/4] =?UTF-8?q?set=20caret=20on=20mount=20as=20we=20usuall?= =?UTF-8?q?y=20do,=20so=20FF=20doesn't=20enter=202=20newlines=20?= =?UTF-8?q?=F0=9F=A4=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/components/views/elements/MessageEditor.js | 7 +------ src/editor/model.js | 10 ++++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/components/views/elements/MessageEditor.js b/src/components/views/elements/MessageEditor.js index b42923954b..b59227ecc8 100644 --- a/src/components/views/elements/MessageEditor.js +++ b/src/components/views/elements/MessageEditor.js @@ -144,12 +144,7 @@ export default class MessageEditor extends React.Component { componentDidMount() { this._updateEditorState(); - const sel = document.getSelection(); - const range = document.createRange(); - range.selectNodeContents(this._editorRef); - range.collapse(false); - sel.removeAllRanges(); - sel.addRange(range); + setCaretPosition(this._editorRef, this.model, this.model.getPositionAtEnd()); this._editorRef.focus(); } diff --git a/src/editor/model.js b/src/editor/model.js index c6f0c5529f..5a571640c6 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -61,6 +61,16 @@ export default class EditorModel { return null; } + getPositionAtEnd() { + if (this._parts.length) { + const index = this._parts.length - 1; + const part = this._parts[index]; + return new DocumentPosition(index, part.text.length); + } else { + return new DocumentPosition(0, 0); + } + } + serializeParts() { return this._parts.map(({type, text}) => {return {type, text};}); } From 690ee63bb460273c10b616b10f301fb97bc468ba Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 16 May 2019 19:14:24 +0100 Subject: [PATCH 4/4] prevent zero-length removals from deleting uneditable parts This solves an issue where, when backspacing the proceeding character next to a pill, chrome reports the caret as being in the pill node, not at the start of the proceeding text node. This would cause the pill to be removed together with proceeding character. This is a bug in any case, removing 0 characters shouldn't remove the part --- src/editor/model.js | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/editor/model.js b/src/editor/model.js index 5a571640c6..13066897b9 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -183,21 +183,26 @@ export default class EditorModel { // part might be undefined here let part = this._parts[index]; const amount = Math.min(len, part.text.length - offset); - if (part.canEdit) { - const replaceWith = part.remove(offset, amount); - if (typeof replaceWith === "string") { - this._replacePart(index, this._partCreator.createDefaultPart(replaceWith)); - } - part = this._parts[index]; - // remove empty part - if (!part.text.length) { - this._removePart(index); + // don't allow 0 amount deletions + if (amount) { + if (part.canEdit) { + const replaceWith = part.remove(offset, amount); + if (typeof replaceWith === "string") { + this._replacePart(index, this._partCreator.createDefaultPart(replaceWith)); + } + part = this._parts[index]; + // remove empty part + if (!part.text.length) { + this._removePart(index); + } else { + index += 1; + } } else { - index += 1; + removedOffsetDecrease += offset; + this._removePart(index); } } else { - removedOffsetDecrease += offset; - this._removePart(index); + index += 1; } len -= amount; offset = 0;