inkandswitch/peritext

Uncaught RangeError when deleting all of the text

Closed this issue · 6 comments

Hey, I think I found a bug in the peritext demo. If I select all of the text and then press delete or backspace, the following exception is raised:
Screenshot 2021-11-27 at 11 59 16 am

BTW I love the article and demo, it's really a fantastic example of what you can achieve with academic writing 😄

Thanks for this bug report! I was able to reproduce and will look into a fix.

I suspect the problem is that this is trying to delete the paragraph node containing the inline content, but our CRDT only knows about the positions within the inline content, so it can't interpret the delete index correctly.

BTW, on my machine, it seems like you can still delete all the text fine if you use the mouse to highlight all the characters; it only doesn't work if you use ctrl-A to select all the letters. Curious if that's the same for you too.

Yeah, it works on my computer if I select the text with my mouse.

That is because mod-A is binded to a selectAll command in the baseKeymap.

An AllSelection is different from a normal TextSelection in that it selects the entire document with [0, doc.content.size], not only the ranges accessible by a normal TextSelection (which requires its bound to be within textblocks).

Since from = 0, contentPosFromProsemirrorPos(0) results in -1 which is out of bounds.

Edit: I tried looking into these two functions

function contentPosFromProsemirrorPos(position: number) {
    return position - 1
}

function prosemirrorPosFromContentPos(position: number) {
    return position + 1
}

If Peritext were to support nodes, would the positioning be nearly identical to how ProseMirror represents selections with its "absolute position" where a position is basically the cumulative sum of node.nodeSize before it? Or like YJS's "relative position" sort of like this from the article

  start: { type: "before", opId: "5@A" },
  end:   { type: "after",  opId: "16@B" }

Thanks @BrianHung for the helpful notes. I just pushed a fix. @quandary-item could you pull main and confirm it works as expected now?

re: positions once Peritext has block nodes -- not sure yet, positions will definitely get more complicated though.

I've pulled main and tested this out. It works as expected now. I see delete operations for all of the characters that are deleted.
Screenshot 2021-12-02 at 2 27 29 pm