Updating attributes is not working when there is no text content and using a custom document content expression

[philippkuehn]

Describe the bug
Updating attributes is not working when there is no text content. But this only happens when using a custom document content expression. In my example (gif below) I’m using content: 'taskList'. When using the default content: 'block+' everything works as expected.

To Reproduce
Steps to reproduce the behavior:

1. Use a custom content expression for document
2. Create a document without text content
3. Update attributes of a node without text content

Expected behavior
Attributes should always be synced.

Screenshots

Environment Information

• Chrome
• y-prosemirror@1.0.0, y-websocket@1.3.8

[dmonad]

I suspect that your taskList item needs textual content. If a y-prosemirror detects that the content doesn't conform to the schema, then the node is marked as invalid and the previous operation is reversed (in this case the operation to check the taskList apparantly leads to an invalid ProseMirror state).

The above editor detects that an attribute changes and probably propagates that change to the below editor.
The below editor tries to create the checked taskList item. I use ProseMirror to check if the node conforms to the schema. I don't do schema parsing in y-prosemirror.

Could you please check the ProseMirror state of both editors and print them here?

Could you try to create taskItem without textual content? Does it conform to your ProseMirror schema?

It's really hard to debug such an issue without something reproducible. I'm just guessing here. Could you please share a repository or a stackblitz? (you could fork this https://stackblitz.com/edit/y-prosemirror)

[dmonad]

Oh, and what a beautiful editor that is 😉

[philippkuehn]

It occurs not only for nodes with text. First I noticed the problem when I wanted to update an atom node when using such a content expression for the doc: content: 'aSingleAtomNode'. I try to reproduce it on stackblitz!

[philippkuehn]

Okay here it is:
https://stackblitz.com/edit/y-prosemirror-yttvpy?file=index.ts

Same issue:

[dmonad]

I was able to reproduce the issue. Here is a screenshot of the ProseMirror state.

The state is synchronized among clients at all times, but it seems that the view is not properly updated.

I don't think that's an issue with y-prosemirror. y-prosemirror only guarantees that the states are synced. It is probably related to the change event or ProseMirrors inner workings.

I guess what is happening is that the change event overwrites the change from yjs with the old value.

FYI: I think the change event is fired even when ProseMirror manipulates the checked property. So you need to account for that. You could check whether ydoc._transaction is defined to check if the current change comes from a remote peer (in this case you probably want to ignore the change event).

There is also a meta property on the ySyncPlugin that is defined when the current transaction originates from a remote client.

Just a lot of thoughts. Hopefully, they were helpful. But I don't think this is related to y-prosemirror. Feel free to continue the discussion & reopen if you think it is.

[philippkuehn]

Hey, sorry but I still don’t get it 😅
You say the state is synchronized correctly but why isn't it working on reload then?

When updating the checkbox the prosemirror state is updated correctly and should be persisted correctly on the websocket server.
But on reload I can see that the y-sync plugin is updating the state with an incorrect "checked" attribute.

(added some logging here: https://stackblitz.com/edit/y-prosemirror-hacaxk?file=index.ts)

[dmonad]

Instead of logging, could you check the state just before you reload the window?

You might log the state before it is changed.

[philippkuehn]

yeah of course. like this?

edit: more detailed here:

[dmonad]

I fixed it for you. It should have been 1. checkbox.checked = !!node.attrs.checked and 2. checkbox.checked = updatedNode.attrs.checked

https://stackblitz.com/edit/y-prosemirror-q4vspn?file=index.ts

The state synchronized properly. But the view did not after reload because of 1. So when you reloaded the window, you were able to read <input checked="checked" /> but it simply didn't render as such.

When you update the checkbox via mouse-click and then via state-update (in your case setAttribute('checked', 'checked')) then the view starts to fall apart. By using the checked property (as in 2.) you fix this problem.

FYI: Spec is pretty unclear on this. But it should probably be setAttribute('checked', ''). The DOM API specifically specifies the checked property though, so you should use that.

If you still can reproduce the issue, try checking the state after reloading the window. It should be the same as before reloading it. There might still be rendering issues. Also don't use the "prosemirror" room. There are lots of people connecting to it, so who knows what they are doing.

[philippkuehn]

Hey, thanks! But I can’t reproduce that it’s fixed at your link.
Both issues still exist (on reload and sync between multiple clients). Also the state on reload is incorrect :(

And it’s also not an issue with some boolean values. I’m working on a drawing node which has the exact same issue:

[dmonad]

That looks more like a sync issue. Or maybe the prosemirror-dispatch is only called after text content is added. Can you please check the ProseMirror state when you are in an invalid state? For comparison, can you please also log the ydoc.getXmlFragment().toString() state so we can check whether Yjs doesn't sync correctly or whether the y-prosemirror binding doesn't work correctly.

Possible bugs are

• Yjs-changes aren't synced between the clients doesn't work.
• The editor-binding doesn't sync attribute-changes to the Yjs document.
• The view doesn't update

I'm flying blind here, because my adaptation fixed the problem for me. I exchanged y-websocket for y-webrtc so we don't have to care about the regular demo-server outages. Let's use the following template now:

https://stackblitz.com/edit/y-prosemirror-kbalw8?file=index.ts

[philippkuehn]

Yeah, the issue exists on this y-webrtc demo too. But this is interesting. Not sure what this means but it should not be empty, right? 😅

[dmonad]

It is always very important to explain exactly what you are doing. As said, I'm flying blind here. I tested the above link on two windows and I only see expected behaviors. No bugs. It is basically impossible to reproduce what you are doing just by looking at a gif. In decentralized shared editing, there are just too many variables (how many clients, in which order did they join, the provider, the editor binding, the view-implementation, the order in which you executed actions, your network setup, and so on..).

Reproducing an issue might seem obvious to you. It is not for me. And I can't magically fix an issue. So please log what I asked for .. because I'm really flying blind here.

You started this ticked with an issue that the view is not updated. Now we talk about an issue where content is apparently not synced. Can you confirm that when you have two windows open, the views synchronize?

So for the other issue (initial state not synchronized), I also expected that you have two windows open that sync state. But I'm just guessing that you open a single window and reload the window and then wonder why the state is not the same as before...
The y-websocket demo-server deletes rooms that are unused. y-webrtc only syncs state among clients. So when you reload the window, it is expected that the Yjs document is empty (where should it get content from?).

However, your schema specifies that the document cannot be empty. So Yjs starts with the empty state, but ProseMirror does not. This is the expected behavior.

As soon as you manipulate the document, Yjs syncs state. In decentralized shared editing, the "starting state" must always be empty to ensure that two clients can open a document at the same time. If I would immediately sync the checkbox, then this would count as an insertion, and every time a user opens a document, an empty checkbox would be inserted.

The same often occurs with headlines that are set as the first item in a ProseMirror document. The current solution is quite satisfactory.

I can't help you with that. But that again is a different issue. You need to tell me exactly how I can reproduce it.

Regarding the original issue: The next time you see that the view is not synchronized between exactly two clients. Can you please log the Yjs state, and the ProseMirror state in both windows? If the Yjs states are synchronized among all clients, and if the ProseMirror state matches the ProseMirror state, then y-prosemirror works as expected. The exception is of course the initial state.

For the reloading issue: I explained above why you see empty state. The next time you reproduce the issue, can you please ensure that another client is connected to the window where you reproduced the bug? E.g. example.provider.awareness.getStates().size > 1

For the third issue (drawing not synchronized): Can you please open a separate ticket with detailed instructions on how I can reproduce that? Please also log the Yjs state, and the ProseMirror state in both windows. If the Yjs states are synchronized among all clients, and if the ProseMirror state matches the the ProseMirror state, then - again - y-prosemirror works as expected. It is always a good idea to check that.

[dmonad]

Through more guesswork, I was able to reproduce an issue by starting with a fresh document and then clicking the checkbox. In this case, the Yjs state should be updated. But it is not. Working on it..

[dmonad]

I think y-prosemirror@1.0.3 should fix the second and the first issue.

The issue was due to the comparison I do when checking if the "starting state" should be updated.

Can you verify this here? https://stackblitz.com/edit/y-prosemirror-kbalw8?file=index.ts

[philippkuehn]

Hey, sorry if I was unclear. I don't fully understand how y.js works under the hood, so it’s hard for me to debug correctly for you. But with y-prosemirror@1.0.3 everything works as expected – for both cases. Thank you very much! 🙌

[dmonad]

Great to hear! I come to realize that it is an art to write reproducible steps. In this case it was a simple misunderstanding. Looking forward to using your editor :)

…

-------- Original Message --------

On Dec 8, 2020, 6:15 PM, Philipp Kühn wrote: Hey, sorry if I was unclear. I don't fully understand how y.js works under the hood, so it’s hard for me to debug correctly for you. But with ***@***.*** everything works as expected – for both cases. Thank you very much! 🙌 — You are receiving this because you modified the open/close state. Reply to this email directly, [view it on GitHub](#30 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ABKL4XOPUPG2UYK54BSF7JLSTZNJ5ANCNFSM4UQHJAMQ).