Skip to content

Commit

Permalink
Don't directly query the DOM selection
Browse files Browse the repository at this point in the history 
Since that will trigger an unncessary relayout in many situations.

FIX: Fix a problem where the DOM update would unnecessarily trigger browser relayouts.
  • Loading branch information
@marijnh
marijnh committed Nov 26, 2021
1 parent 9560923 commit cb63f18
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 60 deletions.
25 changes: 14 additions & 11 deletions src/docview.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export class DocView extends ContentView {
// we don't mess it up when reading it back it
impreciseAnchor: DOMPos | null = null
impreciseHead: DOMPos | null = null
forceSelection = false

dom!: HTMLElement

Expand Down Expand Up @@ -78,22 +79,21 @@ export class DocView extends ContentView {
// getSelection than the one that it actually shows to the user.
// This forces a selection update when lines are joined to work
// around that. Issue #54
let forceSelection = (browser.ie || browser.chrome) && !this.compositionDeco.size && update &&
update.state.doc.lines != update.startState.doc.lines
if ((browser.ie || browser.chrome) && !this.compositionDeco.size && update &&
update.state.doc.lines != update.startState.doc.lines)
this.forceSelection = true

let prevDeco = this.decorations, deco = this.updateDeco()
let decoDiff = findChangedDeco(prevDeco, deco, update.changes)
changedRanges = ChangedRange.extendWithRanges(changedRanges, decoDiff)

let pointerSel = update.transactions.some(tr => tr.isUserEvent("select.pointer"))
if (this.dirty == Dirty.Not && changedRanges.length == 0 &&
!(update.flags & UpdateFlag.Viewport) &&
update.state.selection.main.from >= this.view.viewport.from &&
update.state.selection.main.to <= this.view.viewport.to) {
this.updateSelection(forceSelection, pointerSel)
return false
} else {
this.updateInner(changedRanges, deco, update.startState.doc.length, forceSelection, pointerSel)
this.updateInner(changedRanges, deco, update.startState.doc.length)
return true
}
}
Expand All @@ -102,14 +102,15 @@ export class DocView extends ContentView {
if (this.dirty) {
this.view.observer.ignore(() => this.view.docView.sync())
this.dirty = Dirty.Not
this.updateSelection(true)
} else {
this.updateSelection()
}
if (sel) this.updateSelection()
}

// Used both by update and checkLayout do perform the actual DOM
// update
private updateInner(changes: readonly ChangedRange[], deco: readonly DecorationSet[],
oldLength: number, forceSelection = false, pointerSel = false) {
private updateInner(changes: readonly ChangedRange[], deco: readonly DecorationSet[], oldLength: number) {
this.updateChildren(changes, deco, oldLength)

let {observer} = this.view
Expand All @@ -127,8 +128,7 @@ export class DocView extends ContentView {
let track = browser.chrome || browser.ios ? {node: observer.selectionRange.focusNode!, written: false} : undefined
this.sync(track)
this.dirty = Dirty.Not
if (track && (track.written || observer.selectionRange.focusNode != track.node)) forceSelection = true
this.updateSelection(forceSelection, pointerSel)
if (track && (track.written || observer.selectionRange.focusNode != track.node)) this.forceSelection = true
this.dom.style.height = ""
})
let gaps = []
Expand Down Expand Up @@ -211,9 +211,12 @@ export class DocView extends ContentView {
}

// Sync the DOM selection to this.state.selection
updateSelection(force = false, fromPointer = false) {
updateSelection(mustRead = false, fromPointer = false) {
if (mustRead) this.view.observer.readSelectionRange()
if (!(fromPointer || this.mayControlSelection()) ||
browser.ios && this.view.inputState.rapidCompositionStart) return
let force = this.forceSelection
this.forceSelection = false

let main = this.view.state.selection.main
// FIXME need to handle the case where the selection falls inside a block range
Expand Down
22 changes: 13 additions & 9 deletions src/dom.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ export function getSelection(root: DocumentOrShadowRoot): Selection {
return target.getSelection()!
}

export type SelectionRange = {
focusNode: Node | null, focusOffset: number,
anchorNode: Node | null, anchorOffset: number
}

export function contains(dom: HTMLElement, node: Node | null) {
return node ? dom.contains(node.nodeType != 1 ? node.parentNode : node) : false
}
Expand Down Expand Up @@ -182,7 +177,12 @@ export function scrollRectIntoView(dom: HTMLElement, rect: Rect, side: -1 | 1, c
}
}

export class DOMSelection {
export interface SelectionRange {
focusNode: Node | null, focusOffset: number,
anchorNode: Node | null, anchorOffset: number
}

export class DOMSelectionState implements SelectionRange {
anchorNode: Node | null = null
anchorOffset: number = 0
focusNode: Node | null = null
Expand All @@ -193,9 +193,13 @@ export class DOMSelection {
this.focusNode == domSel.focusNode && this.focusOffset == domSel.focusOffset
}

set(domSel: SelectionRange) {
this.anchorNode = domSel.anchorNode; this.anchorOffset = domSel.anchorOffset
this.focusNode = domSel.focusNode; this.focusOffset = domSel.focusOffset
setRange(range: SelectionRange) {
this.set(range.anchorNode, range.anchorOffset, range.focusNode, range.focusOffset)
}

set(anchorNode: Node | null, anchorOffset: number, focusNode: Node | null, focusOffset: number) {
this.anchorNode = anchorNode; this.anchorOffset = anchorOffset
this.focusNode = focusNode; this.focusOffset = focusOffset
}
}

Expand Down
72 changes: 36 additions & 36 deletions src/domobserver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import browser from "./browser"
import {ContentView, Dirty} from "./contentview"
import {EditorView} from "./editorview"
import {editable} from "./extension"
import {hasSelection, getSelection, DOMSelection, isEquivalentPosition, SelectionRange, deepActiveElement} from "./dom"
import {hasSelection, getSelection, DOMSelectionState, isEquivalentPosition, deepActiveElement} from "./dom"

const observeOptions = {
childList: true,
Expand All @@ -21,12 +21,20 @@ export class DOMObserver {

observer: MutationObserver
active: boolean = false
ignoreSelection: DOMSelection = new DOMSelection

// The known selection. Kept in our own object, as opposed to just
// directly accessing the selection because:
// - Safari doesn't report the right selection in shadow DOM
// - Reading from the selection forces a DOM layout
// - This way, we can ignore selectionchange events if we have
// already seen the 'new' selection
selectionRange: DOMSelectionState = new DOMSelectionState
// Set when a selection change is detected, cleared on flush
selectionChanged = false

delayedFlush = -1
resizeTimeout = -1
queue: MutationRecord[] = []
lastFlush = 0

onCharData: any

Expand All @@ -37,9 +45,6 @@ export class DOMObserver {
gapIntersection: IntersectionObserver | null = null
gaps: readonly HTMLElement[] = []

// Used to work around a Safari Selection/shadow DOM bug (#414)
_selectionRange: SelectionRange | null = null

// Timeout for scheduling check of the parents that need scroll handlers
parentCheck = -1

Expand All @@ -49,7 +54,6 @@ export class DOMObserver {
this.dom = view.contentDOM
this.observer = new MutationObserver(mutations => {
for (let mut of mutations) this.queue.push(mut)
this._selectionRange = null
// IE11 will sometimes (on typing over a selection or
// backspacing out a single character text node) call the
// observer callback before actually updating the DOM.
Expand Down Expand Up @@ -106,10 +110,12 @@ export class DOMObserver {
}, {})
}
this.listenForScroll()
this.readSelectionRange()
this.dom.ownerDocument.addEventListener("selectionchange", this.onSelectionChange)
}

onScroll(e: Event) {
if (this.intersecting) this.flush()
if (this.intersecting) this.flush(false)
this.onScrollChanged(e)
}

Expand All @@ -122,10 +128,11 @@ export class DOMObserver {
}

onSelectionChange(event: Event) {
if (this.lastFlush < Date.now() - 50) this._selectionRange = null
if (!this.readSelectionRange()) return
let {view} = this, sel = this.selectionRange
if (view.state.facet(editable) ? view.root.activeElement != this.dom : !hasSelection(view.dom, sel))
return

let context = sel.anchorNode && view.docView.nearest(sel.anchorNode)
if (context && context.ignoreEvent(event)) return

Expand All @@ -134,28 +141,26 @@ export class DOMObserver {
// reported.
// (Selection.isCollapsed isn't reliable on IE)
if (browser.ie && browser.ie_version <= 11 && !view.state.selection.main.empty &&
sel.focusNode && isEquivalentPosition(sel.focusNode, sel.focusOffset, sel.anchorNode, sel.anchorOffset))
sel.focusNode && isEquivalentPosition(sel.focusNode, sel.focusOffset, sel.anchorNode, sel.anchorOffset))
this.flushSoon()
else
this.flush()
this.flush(false)
}

get selectionRange(): SelectionRange {
if (!this._selectionRange) {
let {root} = this.view, sel: SelectionRange = getSelection(root)
// The Selection object is broken in shadow roots in Safari. See
// https://github.com/codemirror/codemirror.next/issues/414
if (browser.safari && (root as any).nodeType == 11 && deepActiveElement() == this.view.contentDOM)
sel = safariSelectionRangeHack(this.view) || sel
this._selectionRange = sel
}
return this._selectionRange
readSelectionRange() {
let {root} = this.view, domSel = getSelection(root)
// The Selection object is broken in shadow roots in Safari. See
// https://github.com/codemirror/codemirror.next/issues/414
let range = browser.safari && (root as any).nodeType == 11 && deepActiveElement() == this.view.contentDOM &&
safariSelectionRangeHack(this.view) || domSel
if (this.selectionRange.eq(range)) return false
this.selectionRange.setRange(range)
return this.selectionChanged = true
}

setSelectionRange(anchor: {node: Node, offset: number}, head: {node: Node, offset: number}) {
if (!(this._selectionRange as any)?.type)
this._selectionRange = {anchorNode: anchor.node, anchorOffset: anchor.offset,
focusNode: head.node, focusOffset: head.offset}
this.selectionRange.set(anchor.node, anchor.offset, head.node, head.offset)
this.selectionChanged = false
}

listenForScroll() {
Expand Down Expand Up @@ -194,7 +199,6 @@ export class DOMObserver {
start() {
if (this.active) return
this.observer.observe(this.dom, observeOptions)
this.dom.ownerDocument!.addEventListener("selectionchange", this.onSelectionChange)
if (useCharData)
this.dom.addEventListener("DOMCharacterDataModified", this.onCharData)
this.active = true
Expand All @@ -204,20 +208,15 @@ export class DOMObserver {
if (!this.active) return
this.active = false
this.observer.disconnect()
this.dom.ownerDocument!.removeEventListener("selectionchange", this.onSelectionChange)
if (useCharData)
this.dom.removeEventListener("DOMCharacterDataModified", this.onCharData)
}

clearSelection() {
this.ignoreSelection.set(this.selectionRange)
}

// Throw away any pending changes
clear() {
this.observer.takeRecords()
this.queue.length = 0
this.clearSelection()
this.selectionChanged = false
}

flushSoon() {
Expand Down Expand Up @@ -254,24 +253,24 @@ export class DOMObserver {
}

// Apply pending changes, if any
flush() {
flush(readSelection = true) {
if (readSelection) this.readSelectionRange()

// Completely hold off flushing when pending keys are set—the code
// managing those will make sure processRecords is called and the
// view is resynchronized after
if (this.delayedFlush >= 0 || this.view.inputState.pendingAndroidKey) return

this.lastFlush = Date.now()
let {from, to, typeOver} = this.processRecords()
let selection = this.selectionRange
let newSel = !this.ignoreSelection.eq(selection) && hasSelection(this.dom, selection)
let newSel = this.selectionChanged && hasSelection(this.dom, this.selectionRange)
if (from < 0 && !newSel) return

this.selectionChanged = false
let startState = this.view.state
this.onChange(from, to, typeOver)

// The view wasn't updated
if (this.view.state == startState) this.view.docView.reset(newSel)
this.clearSelection()
}

readMutation(rec: MutationRecord): {from: number, to: number, typeOver: boolean} | null {
Expand Down Expand Up @@ -314,6 +313,7 @@ function findChild(cView: ContentView, dom: Node | null, dir: number): ContentVi
return null
}

// Used to work around a Safari Selection/shadow DOM bug (#414)
function safariSelectionRangeHack(view: EditorView) {
let found = null as null | StaticRange
// Because Safari (at least in 2018-2021) doesn't provide regular
Expand Down
13 changes: 9 additions & 4 deletions src/editorview.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ export class EditorView {
if (this.state.facet(styleModule) != this.styleModules) this.mountStyles()
this.updateAttrs()
this.showAnnouncements(transactions)
this.docView.updateSelection(redrawn, transactions.some(tr => tr.isUserEvent("select.pointer")))
} finally { this.updateState = UpdateState.Idle }
if (redrawn || scrollTarget || this.viewState.mustEnforceCursorAssoc) this.requestMeasure()
if (!update.empty) for (let listener of this.state.facet(updateListener)) listener(update)
Expand Down Expand Up @@ -347,7 +348,7 @@ export class EditorView {
this.inputState.update(update)
}
this.updateAttrs()
if (changed) this.docView.update(update)
let redrawn = changed > 0 && this.docView.update(update)
for (let i = 0; i < measuring.length; i++) if (measured[i] != BadMeasure) {
try { measuring[i].write(measured[i], this) }
catch(e) { logException(this.state, e) }
Expand All @@ -356,6 +357,7 @@ export class EditorView {
this.docView.scrollIntoView(this.viewState.scrollTarget)
this.viewState.scrollTarget = null
}
if (changed) this.docView.updateSelection(redrawn)
if (this.viewport.from == oldViewport.from && this.viewport.to == oldViewport.to && this.measureRequests.length == 0) break
}
} finally {
Expand All @@ -377,8 +379,6 @@ export class EditorView {
let editorAttrs = combineAttrs(this.state.facet(editorAttributes), {
class: "cm-editor" + (this.hasFocus ? " cm-focused " : " ") + this.themeClasses
})
updateAttrs(this.dom, this.editorAttrs, editorAttrs)
this.editorAttrs = editorAttrs
let contentAttrs: Attrs = {
spellcheck: "false",
autocorrect: "off",
Expand All @@ -392,7 +392,12 @@ export class EditorView {
}
if (this.state.readOnly) contentAttrs["aria-readonly"] = "true"
combineAttrs(this.state.facet(contentAttributes), contentAttrs)
updateAttrs(this.contentDOM, this.contentAttrs, contentAttrs)

this.observer.ignore(() => {
updateAttrs(this.contentDOM, this.contentAttrs, contentAttrs)
updateAttrs(this.dom, this.editorAttrs, editorAttrs)
})
this.editorAttrs = editorAttrs
this.contentAttrs = contentAttrs
}

Expand Down

0 comments on commit cb63f18

Please sign in to comment.