Skip to content

Commit

Permalink
Improve composition text node finding
Browse files Browse the repository at this point in the history
FIX: Fix an issue where, with IME systems that kept the cursor at the start of the
composed text, the editor misidentified the target node and disrupted composition.

Closes codemirror/dev#1357
  • Loading branch information
marijnh committed Mar 22, 2024
1 parent 0c538f7 commit e05dae1
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 31 deletions.
47 changes: 21 additions & 26 deletions src/docview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import {ContentBuilder, NullWidget} from "./buildview"
import browser from "./browser"
import {Decoration, DecorationSet, WidgetType, addRange, MarkDecoration} from "./decoration"
import {getAttrs} from "./attributes"
import {clientRectsFor, isEquivalentPosition, maxOffset, Rect, scrollRectIntoView,
getSelection, hasSelection, textRange, DOMSelectionState} from "./dom"
import {clientRectsFor, isEquivalentPosition, Rect, scrollRectIntoView,
getSelection, hasSelection, textRange, DOMSelectionState,
textNodeBefore, textNodeAfter} from "./dom"
import {ViewUpdate, decorations as decorationsFacet, outerDecorations,
ChangedRange, ScrollTarget, scrollHandler, getScrollMargins, logException} from "./extension"
import {EditorView} from "./editorview"
Expand All @@ -29,6 +30,7 @@ export class DocView extends ContentView {
hasComposition: {from: number, to: number} | null = null
markedForComposition: Set<ContentView> = new Set
compositionBarrier = Decoration.none
lastCompositionAfterCursor = false

// Track a minimum width for the editor. When measuring sizes in
// measureVisibleLineHeights, this is updated to point at the width
Expand Down Expand Up @@ -260,7 +262,7 @@ export class DocView extends ContentView {
if (browser.gecko) {
let nextTo = nextToUneditable(anchor.node, anchor.offset)
if (nextTo && nextTo != (NextTo.Before | NextTo.After)) {
let text = nearbyTextNode(anchor.node, anchor.offset, nextTo == NextTo.Before ? 1 : -1)
let text = (nextTo == NextTo.Before ? textNodeBefore : textNodeAfter)(anchor.node, anchor.offset)
if (text) anchor = new DOMPos(text.node, text.offset)
}
}
Expand Down Expand Up @@ -621,7 +623,22 @@ class BlockGapWidget extends WidgetType {

export function findCompositionNode(view: EditorView, headPos: number): {from: number, to: number, node: Text} | null {
let sel = view.observer.selectionRange
let textNode = sel.focusNode && nearbyTextNode(sel.focusNode, sel.focusOffset, 0)
if (!sel.focusNode) return null
let textBefore = textNodeBefore(sel.focusNode, sel.focusOffset)
let textAfter = textNodeAfter(sel.focusNode, sel.focusOffset)
let textNode = textBefore || textAfter
if (textAfter && textBefore && textAfter.node != textBefore.node) {
let descAfter = ContentView.get(textAfter.node)
if (!descAfter || descAfter instanceof TextView && descAfter.text != textAfter.node.nodeValue) {
textNode = textAfter
} else if (view.docView.lastCompositionAfterCursor) {
let descBefore = ContentView.get(textBefore.node)
if (!(!descBefore || descBefore instanceof TextView && descBefore.text != textBefore.node.nodeValue))
textNode = textAfter
}
}
view.docView.lastCompositionAfterCursor = textNode != textBefore

if (!textNode) return null
let from = headPos - textNode.offset
return {from, to: from + textNode.node.nodeValue!.length, node: textNode.node}
Expand Down Expand Up @@ -655,28 +672,6 @@ function findCompositionRange(view: EditorView, changes: ChangeSet, headPos: num
}
}

function nearbyTextNode(startNode: Node, startOffset: number, side: number): {node: Text, offset: number} | null {
if (side <= 0) for (let node = startNode, offset = startOffset;;) {
if (node.nodeType == 3) return {node: node as Text, offset: offset}
if (node.nodeType == 1 && offset > 0) {
node = node.childNodes[offset - 1]
offset = maxOffset(node)
} else {
break
}
}
if (side >= 0) for (let node = startNode, offset = startOffset;;) {
if (node.nodeType == 3) return {node: node as Text, offset: offset}
if (node.nodeType == 1 && offset < node.childNodes.length && side >= 0) {
node = node.childNodes[offset]
offset = 0
} else {
break
}
}
return null
}

const enum NextTo { Before = 1, After = 2 }

function nextToUneditable(node: Node, offset: number) {
Expand Down
38 changes: 38 additions & 0 deletions src/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ export function domIndex(node: Node): number {
}
}

export function isBlockElement(node: Node): boolean {
return node.nodeType == 1 && /^(DIV|P|LI|UL|OL|BLOCKQUOTE|DD|DT|H\d|SECTION|PRE)$/.test(node.nodeName)
}

function scanFor(node: Node, off: number, targetNode: Node, targetOff: number, dir: -1 | 1): boolean {
for (;;) {
if (node == targetNode && off == targetOff) return true
Expand Down Expand Up @@ -336,3 +340,37 @@ export function atElementStart(doc: HTMLElement, selection: SelectionRange) {
export function isScrolledToBottom(elt: HTMLElement) {
return elt.scrollTop > Math.max(1, elt.scrollHeight - elt.clientHeight - 4)
}

export function textNodeBefore(startNode: Node, startOffset: number): {node: Text, offset: number} | null {
for (let node = startNode, offset = startOffset;;) {
if (node.nodeType == 3 && offset > 0) {
return {node: node as Text, offset: offset}
} else if (node.nodeType == 1 && offset > 0) {
if ((node as HTMLElement).contentEditable == "false") return null
node = node.childNodes[offset - 1]
offset = maxOffset(node)
} else if (node.parentNode && !isBlockElement(node)) {
offset = domIndex(node)
node = node.parentNode
} else {
return null
}
}
}

export function textNodeAfter(startNode: Node, startOffset: number): {node: Text, offset: number} | null {
for (let node = startNode, offset = startOffset;;) {
if (node.nodeType == 3 && offset < node.nodeValue!.length) {
return {node: node as Text, offset: offset}
} else if (node.nodeType == 1 && offset < node.childNodes.length) {
if ((node as HTMLElement).contentEditable == "false") return null
node = node.childNodes[offset]
offset = 0
} else if (node.parentNode && !isBlockElement(node)) {
offset = domIndex(node) + 1
node = node.parentNode
} else {
return null
}
}
}
6 changes: 1 addition & 5 deletions src/domreader.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {ContentView} from "./contentview"
import {domIndex, maxOffset} from "./dom"
import {domIndex, maxOffset, isBlockElement} from "./dom"
import {EditorState} from "@codemirror/state"

export const LineBreakPlaceholder = "\uffff"
Expand Down Expand Up @@ -105,10 +105,6 @@ function isAtEnd(parent: Node, node: Node | null, offset: number) {
}
}

function isBlockElement(node: Node): boolean {
return node.nodeType == 1 && /^(DIV|P|LI|UL|OL|BLOCKQUOTE|DD|DT|H\d|SECTION|PRE)$/.test(node.nodeName)
}

export class DOMPoint {
pos: number = -1
constructor(readonly node: Node, readonly offset: number) {}
Expand Down

0 comments on commit e05dae1

Please sign in to comment.