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

Conversion doesn't check types #116

Open
2 tasks done
andy-zhou opened this issue May 23, 2022 · 2 comments
Open
2 tasks done

Conversion doesn't check types #116

andy-zhou opened this issue May 23, 2022 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@andy-zhou
Copy link

Please save me some time and use the following template. In 90% of all issues I can't reproduce the problem because I don't know what exactly you are doing, in which environment, or which y-* version is responsible. Just use the following template even if you think the problem is obvious.

Checklist

Describe the bug
A clear and concise description of what the bug is.

When converting from a Prosemirror Node to a YXmlFragment, y-prosemirror doesn't check that attributes it sets are actually strings.

const type = new Y.XmlElement(node.type.name)
for (const key in node.attrs) {
  const val = node.attrs[key]
  if (val !== null && key !== 'ychange') {
    type.setAttribute(key, val)[Kevin Jahns, 2 years ago:  revert toms refactor](https://sourcegraph.com/github.com/yjs/y-prosemirror/-/commit/e39c91e45e113220b5d66311a24c9397b7af0a25)
  }
}

This is problematic because a Prosemirror Node can have a non-string attribute, such as a number. Using YXmlElement.getElement(...) can therefore return a non-string which claims to be a string.

To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Open console
  4. See error

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Environment Information

  • Browser / Node.js [e.g. Chrome, Firefox, Node.js]
  • Yjs version and the versions of the y-* modules you are using [e.g. yjs v13.0.1, y-webrtc v1.2.1]. Use npm ls yjs to find out the exact version you are using.

Additional context
Add any other context about the problem here.

@andy-zhou andy-zhou added the bug Something isn't working label May 23, 2022
@andy-zhou
Copy link
Author

Ping on this. I think there's a bunch of options to fix this, but not sure how you think about backwards compatibility.

@YousefED
Copy link

YousefED commented Nov 5, 2024

Related issue: TypeCellOS/BlockNote#1123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants