fix: clamp positions if out of bounds

[nperez0111]

On looking into this comment: https://github.com/yjs/y-prosemirror/pull/176#issuecomment-2825364764 it turns out that he is right, I was able to get a failed selection restoration because the position was restored outside the bounds of the document. I'm not entirely clear how this occurs, but clamping the position to the document size seems like the right thing to do

[dmonad]

Wouldn't this suggest that there is a deeper underlying issue?

Just setting the cursor to the end of the document doesn't seem like the right solution. :/

[nperez0111]

Wouldn't this suggest that there is a deeper underlying issue?

Just setting the cursor to the end of the document doesn't seem like the right solution. :/

Totally fair, my current understanding is that the relative selection can point at a node which was "pushed down" by new content that came in. Using his example here, I reproduced this:

So, the relative selection when resolved back to an absolute position is a much larger value, I'm unclear on why that is though 🤔. One idea is that the document at that point in time has both the new & old content, even though the old content should be considered deleted and not count towards the document size it is?

Interestingly, if using my other PR, this issue doesn't seem to occur: https://github.com/yjs/y-prosemirror/pull/182

[dmonad]

I implemented a few fixes / changes for cursors (ab23460ed70afdc63258c329f042a2a6fb364444). Maybe they will fix this specific issue as well.

Anyway, I want to investigate this further. But for now I want to avoid clamping.

[nperez0111]

Hey @dmonad I was looking into this again, and I think we may want to consider adding clamping to this. I have this test file which shows the issue with the exact error.

For some reason, it seems that when the XMLFragment is empty, relativePositionToAbsolutePosition seems to over count when traversing the parents. I can't completely figure out why because the code is a bit convoluted for it. I would like to fix the root cause, but may be missing something fundamental here in what this is meant to be doing in the first place. I also feel like it may be getting some stale values from the mapping in the binding (since this occurs in a transition).

But, maybe it is pragmatic to just clamp the values & revisit this at another point in time.

Here is the test file which reproduces the error:

import * as t from 'lib0/testing'
import * as Y from 'yjs'
import {
  relativePositionToAbsolutePosition,
  absolutePositionToRelativePosition,
  ySyncPlugin,
  prosemirrorToYDoc
} from '../src/y-prosemirror.js'
import { ySyncPluginKey } from '../src/plugins/keys.js'
import { EditorState } from 'prosemirror-state'
import { EditorView } from 'prosemirror-view'
import { Schema } from 'prosemirror-model'

/**
 * Test the exact scenario from the user's example using prosemirrorToYDoc
 * This replicates the exact test case provided by the user
 * @param {t.TestCase} _tc
 */
export const testExactUserScenario = (_tc) => {
  // Create the exact schema from the user's example
  const schema = new Schema({
    nodes: {
      doc: {
        content: 'container+ | singleContainer'
      },
      container: {
        content: 'paragraph+',
        toDOM () {
          return ['div', 0]
        }
      },
      singleContainer: {
        content: 'paragraph+',
        toDOM () {
          return ['div', 0]
        }
      },
      paragraph: {
        content: 'text*',
        toDOM () {
          return ['p', 0]
        }
      },
      text: {}
    }
  })

  // Create the exact document structure from the user's example manually
  const docNode = schema.node('doc', null, [
    schema.node('container', null, [
      schema.node('paragraph', null, schema.text('paragraph 0'))
    ]),
    schema.node('container', null, [
      schema.node('paragraph', null, schema.text('paragraph 1'))
    ])
  ])

  // Convert to Yjs document using prosemirrorToYDoc
  const ydoc1 = prosemirrorToYDoc(docNode)
  const xmlFragment1 = ydoc1.getXmlFragment('prosemirror')

  const mockDiv = { appendChild: () => {}, removeChild: () => {} }
  const view1 = new EditorView(mockDiv, {
    state: EditorState.create({
      doc: docNode,
      plugins: [ySyncPlugin(xmlFragment1)]
    })
  })

  // Create second document and sync
  const ydoc2 = new Y.Doc()
  Y.applyUpdate(ydoc2, Y.encodeStateAsUpdate(ydoc1))
  const xmlFragment2 = ydoc2.getXmlFragment('prosemirror')
  const view2 = new EditorView(mockDiv, {
    state: EditorState.create({
      doc: docNode,
      plugins: [ySyncPlugin(xmlFragment2)]
    })
  })

  console.log('=== EXACT USER SCENARIO TEST ===')
  console.log('Initial document size:', view1.state.doc.content.size)
  console.log('Initial Yjs fragment length:', xmlFragment1.length)
  console.log('Initial document structure:', JSON.stringify(view1.state.doc.toJSON(), null, 2))

  // Test all positions before replace
  const mapping1 = ySyncPluginKey.getState(view1.state)?.binding?.mapping || new Map()
  const docSizeBefore = view1.state.doc.content.size
  let overCountingDetected = false

  console.log('=== POSITION TESTING BEFORE REPLACE ===')
  for (let pos = 0; pos < docSizeBefore; pos++) {
    const relPos = absolutePositionToRelativePosition(pos, xmlFragment1, mapping1)
    const absPos = relativePositionToAbsolutePosition(ydoc1, xmlFragment1, relPos, mapping1)
    console.log(`Position ${pos}: relPos -> absPos = ${absPos} (${absPos === pos ? '✓' : '✗'})`)

    if (absPos !== pos) {
      console.log('OVER-COUNTING DETECTED BEFORE REPLACE!')
      console.log('Expected:', pos, 'Got:', absPos, 'Difference:', absPos - pos)
      overCountingDetected = true
    }
  }

  // Perform the exact replace operation from the user's example
  console.log('=== PERFORMING REPLACE OPERATION ===')
  const newDoc = schema.node('doc', null, [
    schema.node('singleContainer', null, [
      schema.node('paragraph', null, schema.text('paragraph 0')),
      schema.node('paragraph', null, schema.text('paragraph 1'))
    ])
  ])

  try {
    view1.dispatch(
      view1.state.tr.replaceWith(
        0,
        view1.state.doc.content.size,
        newDoc
      )
    )
    view2.dispatch(
      view2.state.tr.replaceWith(
        0,
        view2.state.doc.content.size,
        newDoc
      )
    )
  } catch (error) {
    console.log('ERROR during replace operation:', error.message)
    console.log('This suggests over-counting in position calculation!')
    overCountingDetected = true
  }

  // Sync the documents
  const update1 = Y.encodeStateAsUpdate(ydoc1)
  const update2 = Y.encodeStateAsUpdate(ydoc2)
  Y.applyUpdate(ydoc1, update2)
  Y.applyUpdate(ydoc2, update1)

  console.log('=== AFTER REPLACE AND SYNC ===')
  console.log('Document size after replace:', view1.state.doc.content.size)
  console.log('Yjs fragment length after replace:', xmlFragment1.length)
  console.log('Document structure after replace:', JSON.stringify(view1.state.doc.toJSON(), null, 2))

  // Test all positions after replace
  const mapping1After = ySyncPluginKey.getState(view1.state)?.binding?.mapping || new Map()
  const docSizeAfter = view1.state.doc.content.size

  console.log('=== POSITION TESTING AFTER REPLACE ===')
  for (let pos = 0; pos < docSizeAfter; pos++) {
    const relPos = absolutePositionToRelativePosition(pos, xmlFragment1, mapping1After)
    const absPos = relativePositionToAbsolutePosition(ydoc1, xmlFragment1, relPos, mapping1After)
    console.log(`Position ${pos}: relPos -> absPos = ${absPos} (${absPos === pos ? '✓' : '✗'})`)

    if (absPos !== pos) {
      console.log('OVER-COUNTING DETECTED AFTER REPLACE!')
      console.log('Expected:', pos, 'Got:', absPos, 'Difference:', absPos - pos)
      overCountingDetected = true
    }
  }

  if (overCountingDetected) {
    console.log('OVER-COUNTING DETECTED in exact user scenario!')
  }

  t.assert(true, 'Exact user scenario test completed - check console for over-counting issues')
}