Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selection/cursor-change and edits racing condition #1

Open
pedrosanta opened this issue May 25, 2017 · 3 comments
Open

Selection/cursor-change and edits racing condition #1

pedrosanta opened this issue May 25, 2017 · 3 comments

Comments

@pedrosanta
Copy link
Owner

pedrosanta commented May 25, 2017

A known issue of this approach/example is a racing condition which makes cursors stuck/out-of-correct-position sometimes when a selection-change cursor update (which goes through cursors socket) is quickly followed by some edits (which goes through ShareDB and its socket) - like using the back arrow key with Ctrl to go to the beginning of a word and immediately start typing.

How to replicate:

  1. Open two side-by-side windows on https://quill-sharedb-cursors.herokuapp.com;
  2. On one of them try to move to the beginning of a word with the keyboard shortcut (Ctrl/Option+Left Arrow) and inserting some text immediately after;
  3. If you do these things quick enough, the cursor on the other client might become off-position.

My take is that:

  • Sometimes, the insert operation gets processed first (and vice-versa) on server (which moves cursor forward) and then the message with absolute position (from the arrow caret move) comes in and places the cursor behind from it is on the other client;
  • Sometimes the cursor doesn't even move from the place of start, meaning that it only gets updated/shifted on the insert operations, like the server hasn't ever received the caret change update/message in the first place;

One solution for this was if... cursor updates could be transmitted through ShareDB (probably as ephemeral metadata like stated on share/sharedb#128).

Still needs more testing.

@sferoze
Copy link

sferoze commented Jul 3, 2017

@pedrosanta

I noticed this issue too. The cursor is sometimes sent before the sharedb delta operation is updated. So it sometimes forces the cursor onto a new line or it hides the cursor.

What do you think about this as a temp solution?

the quill.on 'selection-change' even is only called on initial focus, when you have selected or deselected text, and when you place caret in a different spot. It is not called on each character typed though. Was this your initial assumption when writing the code?

So the cursor updating is really dependent on doc.on 'nothing pending', debouncedSendCursorData event. Since the original timeout was really high, it caused issues.

So I reduced the timeout to 1ms. Basically always updating the cursor whenever operations have competed. This way the cursor always is updating right after the delta's have been applied.

@pedrosanta In my testing it seems this solutions works better, and just wondering what your thoughts are?

debouncedSendCursorData = utils.debounce((->
      range = quill.getSelection()
      if range
        console.log '[cursors] Stopped typing, sending a cursor update/refresh.'
        sendCursorData range
      return
    ), 1)
    doc.on 'nothing pending', debouncedSendCursorData

quill.on 'selection-change', (range, oldRange, source) ->
      sendCursorData range
      return

@sferoze
Copy link

sferoze commented Jul 3, 2017

But I do notice one downside is the constant sending of cursor data via sockets...

Otherwise the cursor module shifts the cursor to the correct spot depending on the delta, not requiring the cursor update.

So reducing the timeout does help cursor placement but also uses more network resources...

update:

in my testing reducing debounce timeout to 10ms doesn't make the cursor update method called significantly more. I think it is best to reduce the debounce timeout.

@pedrosanta
Copy link
Owner Author

Hey @sferoze, thank you for the comments. I've been a bit low on time in the latest few days, but let me catch this up and I'll get back to you. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants