Fix several bugs around the handling of non-inclusive replaced blocks

FIX: Fix a crash related to non-inclusive replacing block decorations.

Closes codemirror/dev#1251

src/blockview.ts

@@ -2,14 +2,14 @@ import {ContentView, DOMPos, ViewFlag, noChildren, mergeChildrenInto} from "./co
 import {DocView} from "./docview"
 import {TextView, MarkView, inlineDOMAtPos, joinInlineInto, coordsInChildren} from "./inlineview"
 import {clientRectsFor, Rect, clearAttributes} from "./dom"
-import {LineDecoration, WidgetType, BlockType} from "./decoration"
+import {LineDecoration, WidgetType, PointDecoration} from "./decoration"
 import {Attrs, combineAttrs, attrsEq, updateAttrs} from "./attributes"
 import browser from "./browser"
 import {EditorView} from "./editorview"
 import {Text} from "@codemirror/state"
 
 export interface BlockView extends ContentView {
-  type: BlockType
+  covers(side: -1 | 1): boolean
   dom: HTMLElement | null
 }
 
@@ -153,7 +153,7 @@ export class LineView extends ContentView implements BlockView {
 
   become(_other: ContentView) { return false }
 
-  get type() { return BlockType.Text }
+  covers() { return true }
 
   static find(docView: DocView, pos: number): LineView | null {
     for (let i = 0, off = 0; i < docView.children.length; i++) {
@@ -174,7 +174,7 @@ export class BlockWidgetView extends ContentView implements BlockView {
   breakAfter = 0
   prevWidget: WidgetType | null = null
 
-  constructor(public widget: WidgetType, public length: number, public type: BlockType) {
+  constructor(public widget: WidgetType, public length: number, public deco: PointDecoration) {
     super()
   }
 
@@ -193,7 +193,7 @@ export class BlockWidgetView extends ContentView implements BlockView {
   split(at: number) {
     let len = this.length - at
     this.length = at
-    let end = new BlockWidgetView(this.widget, len, this.type)
+    let end = new BlockWidgetView(this.widget, len, this.deco)
     end.breakAfter = this.breakAfter
     return end
   }
@@ -222,7 +222,7 @@ export class BlockWidgetView extends ContentView implements BlockView {
       if (this.dom && !this.prevWidget) this.prevWidget = this.widget
       this.widget = other.widget
       this.length = other.length
-      this.type = other.type
+      this.deco = other.deco
       this.breakAfter = other.breakAfter
       return true
     }
@@ -244,4 +244,9 @@ export class BlockWidgetView extends ContentView implements BlockView {
     super.destroy()
     if (this.dom) this.widget.destroy(this.dom)
   }
+
+  covers(side: -1 | 1) {
+    let {startSide, endSide} = this.deco
+    return startSide == endSide ? false : side < 0 ? startSide < 0 : endSide > 0
+  }
 }

src/buildview.ts

@@ -1,5 +1,5 @@
 import {SpanIterator, RangeSet, Text, TextIterator} from "@codemirror/state"
-import {DecorationSet, Decoration, PointDecoration, LineDecoration, MarkDecoration, BlockType, WidgetType} from "./decoration"
+import {DecorationSet, Decoration, PointDecoration, LineDecoration, MarkDecoration, WidgetType} from "./decoration"
 import {ContentView} from "./contentview"
 import {BlockView, LineView, BlockWidgetView} from "./blockview"
 import {WidgetView, TextView, MarkView, WidgetBufferView} from "./inlineview"
@@ -32,7 +32,7 @@ export class ContentBuilder implements SpanIterator<Decoration> {
     if (this.content.length == 0)
       return !this.breakAtStart && this.doc.lineAt(this.pos).from != this.pos
     let last = this.content[this.content.length - 1]
-    return !last.breakAfter && !(last instanceof BlockWidgetView && last.type == BlockType.WidgetBefore)
+    return !(last.breakAfter || last instanceof BlockWidgetView && last.deco.endSide < 0)
   }
 
   getLine() {
@@ -59,7 +59,7 @@ export class ContentBuilder implements SpanIterator<Decoration> {
   finish(openEnd: number) {
     if (this.pendingBuffer && openEnd <= this.bufferMarks.length) this.flushBuffer()
     else this.pendingBuffer = Buf.No
-    if (!this.posCovered()) this.getLine()
+    if (!openEnd && !this.posCovered()) this.getLine()
   }
 
   buildText(length: number, active: readonly MarkDecoration[], openStart: number) {
@@ -108,9 +108,8 @@ export class ContentBuilder implements SpanIterator<Decoration> {
     let len = to - from
     if (deco instanceof PointDecoration) {
       if (deco.block) {
-        let {type} = deco
-        if (type == BlockType.WidgetAfter && !this.posCovered()) this.getLine()
-        this.addBlockWidget(new BlockWidgetView(deco.widget || new NullWidget("div"), len, type))
+        if (deco.startSide > 0 && !this.posCovered()) this.getLine()
+        this.addBlockWidget(new BlockWidgetView(deco.widget || new NullWidget("div"), len, deco))
       } else {
         let view = WidgetView.create(deco.widget || new NullWidget("span"), len, len ? 0 : deco.startSide)
         let cursorBefore = this.atCursorPos && !view.isEditable && openStart <= active.length &&

src/decoration.ts

@@ -334,7 +334,7 @@ export class PointDecoration extends Decoration {
 
   // Only relevant when this.block == true
   get type() {
-    return this.startSide < this.endSide ? BlockType.WidgetRange
+    return this.startSide != this.endSide ? BlockType.WidgetRange
       : this.startSide <= 0 ? BlockType.WidgetBefore : BlockType.WidgetAfter
   }
 

src/docview.ts

@@ -4,7 +4,7 @@ import {BlockView, LineView, BlockWidgetView} from "./blockview"
 import {TextView, MarkView} from "./inlineview"
 import {ContentBuilder} from "./buildview"
 import browser from "./browser"
-import {Decoration, DecorationSet, WidgetType, BlockType, addRange, MarkDecoration} from "./decoration"
+import {Decoration, DecorationSet, WidgetType, addRange, MarkDecoration} from "./decoration"
 import {getAttrs} from "./attributes"
 import {clientRectsFor, isEquivalentPosition, maxOffset, Rect, scrollRectIntoView,
         getSelection, hasSelection, textRange} from "./dom"
@@ -363,15 +363,18 @@ export class DocView extends ContentView {
   }
 
   coordsAt(pos: number, side: number): Rect | null {
-    for (let off = this.length, i = this.children.length - 1;; i--) {
-      let child = this.children[i], start = off - child.breakAfter - child.length
-      if (pos > start ||
-          (pos == start && child.type != BlockType.WidgetBefore && child.type != BlockType.WidgetAfter &&
-           (!i || side == 2 || this.children[i - 1].breakAfter ||
-            (this.children[i - 1].type == BlockType.WidgetBefore && side > -2))))
-        return child.coordsAt(pos - start, side)
+    let best = null, bestPos = 0
+    for (let off = this.length, i = this.children.length - 1; i >= 0; i--) {
+      let child = this.children[i], end = off - child.breakAfter, start = end - child.length
+      if (end < pos) break
+      if (start <= pos && (start < pos || child.covers(-1)) && (end > pos || child.covers(1)) &&
+          (!best || child instanceof LineView && !(best instanceof LineView && side >= 0))) {
+        best = child
+        bestPos = start
+      }
       off = start
     }
+    return best ? best.coordsAt(pos - bestPos, side) : null
   }
 
   coordsForChar(pos: number) {

src/heightmap.ts

@@ -631,11 +631,11 @@ class NodeBuilder implements SpanIterator<Decoration> {
 
   addBlock(block: HeightMapBlock) {
     this.enterLine()
-    let type = block.deco?.type
-    if (type == BlockType.WidgetAfter && !this.isCovered) this.ensureLine()
+    let deco = block.deco
+    if (deco && deco.startSide > 0 && !this.isCovered) this.ensureLine()
     this.nodes.push(block)
     this.writtenTo = this.pos = this.pos + block.length
-    if (type != BlockType.WidgetBefore) this.covering = block
+    if (deco && deco.endSide > 0) this.covering = block
   }
 
   addLineDeco(height: number, breaks: number, length: number) {

test/webtest-coords.ts

@@ -103,25 +103,33 @@ describe("EditorView coords", () => {
   it("takes coordinates before side=1 block widgets", () => {
     let widget = Decoration.widget({widget: block, side: 1, block: true})
     let cm = tempView("ab", [deco(widget.range(0), widget.range(1), widget.range(2))])
-    let sides = Array.prototype.map.call(cm.contentDOM.querySelectorAll(".widget"), w => w.getBoundingClientRect().top)
-    ist(near(cm.coordsAtPos(0, 1)!.bottom, sides[0]))
+    let sides = Array.from(cm.contentDOM.querySelectorAll(".widget")).map(w => w.getBoundingClientRect().top)
     ist(near(cm.coordsAtPos(0, -1)!.bottom, sides[0]))
-    ist(near(cm.coordsAtPos(1, 1)!.bottom, sides[1]))
+    ist(near(cm.coordsAtPos(0, 1)!.bottom, sides[1]))
     ist(near(cm.coordsAtPos(1, -1)!.bottom, sides[1]))
-    ist(near(cm.coordsAtPos(2, 1)!.bottom, sides[2]))
+    ist(near(cm.coordsAtPos(1, 1)!.bottom, sides[2]))
     ist(near(cm.coordsAtPos(2, -1)!.bottom, sides[2]))
+    ist(near(cm.coordsAtPos(2, 1)!.bottom, sides[2]))
   })
 
   it("takes coordinates after side=-1 block widgets", () => {
     let widget = Decoration.widget({widget: block, side: -1, block: true})
     let cm = tempView("ab", [deco(widget.range(0), widget.range(1), widget.range(2))])
     let sides = Array.prototype.map.call(cm.contentDOM.querySelectorAll(".widget"), w => w.getBoundingClientRect().bottom)
-    ist(near(cm.coordsAtPos(0, 1)!.top, sides[0]))
     ist(near(cm.coordsAtPos(0, -1)!.top, sides[0]))
+    ist(near(cm.coordsAtPos(0, 1)!.top, sides[0]))
+    ist(near(cm.coordsAtPos(1, -1)!.top, sides[0]))
     ist(near(cm.coordsAtPos(1, 1)!.top, sides[1]))
-    ist(near(cm.coordsAtPos(1, -1)!.top, sides[1]))
+    ist(near(cm.coordsAtPos(2, -1)!.top, sides[1]))
     ist(near(cm.coordsAtPos(2, 1)!.top, sides[2]))
-    ist(near(cm.coordsAtPos(2, -1)!.top, sides[2]))
+  })
+
+  it("takes coordinates around non-inclusive block widgets", () => {
+    let widget = Decoration.replace({widget: block, inclusive: false, block: true})
+    let cm = tempView("ab", [deco(widget.range(0, 2))])
+    let rect = cm.contentDOM.querySelector(".widget")!.getBoundingClientRect()
+    ist(near(cm.coordsAtPos(0, 1)!.bottom, rect.top))
+    ist(near(cm.coordsAtPos(2, -1)!.top, rect.bottom))
   })
 
   it("takes proper coordinates for elements on decoration boundaries", () => {

test/webtest-draw-decoration.ts

@@ -685,6 +685,15 @@ describe("EditorView decoration", () => {
       ist(!cm.contentDOM.querySelector(".line"))
     })
 
+    it("draws lines around non-inclusive block widgets", () => {
+      let cm = decoEditor("1\n23\n4", [
+        br(0, 1, "X", false),
+        br(2, 4, "Y", false),
+        br(5, 6, "Z", false)
+      ])
+      ist(cm.contentDOM.querySelectorAll(".cm-line").length, 6)
+    })
+
     it("raises an error when providing block widgets from plugins", () => {
       ist.throws(() => {
         tempView("abc", [ViewPlugin.fromClass(class {