[Bug]: @tiptap/html no longer handles style attributes correctly

[dcneiner]

Affected Packages

html, pm

Version(s)

2.4.0+ (depending on which version of prosemirror-model landed in the lock file)

Bug Description

This only impacts parsing HTML in Node.js using @tiptap/html. From what I can tell zeed-dom will need to be changed to address the change in prosemirror-model However, I'm starting by reporting it here for awareness and in case you'd like to address some other way.

Prior to [email protected], ProseMirror parsed the style attribute itself instead of depending on the underlying CSSStyleDeclaration object via the style property on the DOM node. Now it expects style to be a CSSStyleDeclaration most notably with length, item and getPropertyValue apis. Since zeed-dom does not provide any of those (style is just a plain object), the expectation that @tiptap/html will parse HTML accurately to the in-browser editor is no longer true.

Browser Used

Other

Code Example URL

No response

Expected Behavior

Since this is a server-side bug, the CodeSandbox templates don't apply. However, a simple node app is sufficient to illustrate the issue:

// index.js
const assert = require("node:assert/strict");

const { generateJSON, generateHTML } = require("@tiptap/html");
const { Document } = require("@tiptap/extension-document");
const { Paragraph } = require("@tiptap/extension-paragraph");
const { Text } = require("@tiptap/extension-text");
const { Underline } = require("@tiptap/extension-underline");

const extensions = [Document, Paragraph, Text, Underline];

const input =
  '<p><span style="text-decoration: underline">example text</span></p>';
const expected = "<p><u>example text</u></p>";

assert.equal(
  generateHTML(generateJSON(input, extensions), extensions),
  expected
);

This will fail the assertion, producing instead <p>example text</p> instead of <p><u>example text</u></p>

If you then add overrides to package.json, the issue is resolved:

"overrides": {
  "prosemirror-model": "1.21.0"
}

To fully fix this, zeed-dom will need to be updated or replaced or prosemirror-model pinned to version 1.21.0 (which of course is not a long term solution)

Additional Context (Optional)

No response

Dependency Updates

• Yes, I've updated all my dependencies.

[dcneiner]

This issue on zeed-dom is related to the set of changes that landed in prosemirror-model: holtwick/zeed-dom#12

[bdbch]

We could pin the prosemirror-model version in @tiptap/pm until the issue is resolved on zeed-dom's end. cc @nperez0111 @svenadlung

[dcneiner]

Thanks for the consideration @bdbch

As we are adding more tests for our server parsing, we also discovered that at least with <code class="language-css">...</code> the language-css is getting stripped due to missing apis in zeed-dom (firstElementChild, then classList is not iterable) that prosemirror is using. Mentioning here because it fits in the theme of tiptap/html not producing the same output as tiptap/core does in the browser.

For now we have switched to https://github.com/WebReflection/linkedom with code based on tiptap/html. We are optimistic it will meet our needs. Our super limited tests indicated it is faster too, but its too early to know what we might be losing that zeed-dom offers.

Let me know if I can provide any more details in the mean time.

[nperez0111]

We were considering removing zeed-dom for another issue as well. It has been less updated recently so maybe we can switch to this library instead

[tehbelinda]

I ran into something similar recently and created a code sandbox that illustrates the differences, was about to file a new issue but I think this is the same bug. I managed it to reproduce it even on the browser-side as long as I import the @tiptap/html package:

https://codesandbox.io/p/sandbox/inspiring-pine-s9hh5z

(see the console logs to compare the difference in output)

For more context:

in my case I was trying to extend the highlight extension to parse background-color from spans, not just marks. We have some html that was generated in another text editor that we need to import. This initialisation has to happen server-side as we are using a Hocuspocus server for collaborative editing. If we initialise on the client side, we run into issues with duplicate content.

[nperez0111]

Related: #5515

[nperez0111]

Actually, as an alternative to this, I have actually made a static renderer from tiptap JSON into HTML, this means that we can convert tiptap JSON on a server without needing to emulate any browser APIs
#5528

I mean, the @tiptap/html package should still work like it used to (probably via switching to happy-dom), but this would be an even more lightweight way of achieving the same thing

[dcneiner]

@nperez0111 I looked through the linked PR and it looks amazing. I'd love the ability to render straight to React without the full ProseMirror wire up.

I did want to mention that my original reported issue was an issue on the parsing side (parsing HTML into JSON structure) than it was on the rendering side. I realize my example doesn't make that clear as it uses both generateJSON and generateHTML.

We had hoped to use the same HTML parsing on server side and client side to ensure similar security parsing/sanitization (something that happens less when passing around the JSON document). However, we pivoted to a dedicated sanitization pipeline and just ensure any extra attributes we support in the client are not stripped in that process.

Regardless, the static renderer looks like its going to be a great addition to this library!

[tehbelinda]

like dcneiner, that looks awesome but our issue is also on the parsing side rather than the rendering side. curious @dcneiner if you could share any tips on your work-around/pivot? e.g. are you using other html parsing libraries?

[nperez0111]

I got the maintainer of zeed-dom to update the package to support this now. I don't believe that it will solve everything, since a number of those APIs are unimplemented, but at least we can update this in a patch version and have things work again.

Next we will probably look into happy-dom as a replacement. My only concerns are differences in parsing behavior between zeed-dom and happy-dom that I kind of want to leave this to be resolved in a major change rather than some minor or patch change