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

dcneiner commented 1 month ago

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 prosemirror-model@1.21.1, 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 example text instead of example text

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

[X] Yes, I've updated all my dependencies.

dcneiner commented 1 month ago

This issue on zeed-dom is related to the set of changes that landed in prosemirror-model: https://github.com/holtwick/zeed-dom/issues/12

bdbch commented 1 month ago

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

dcneiner commented 1 month ago

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 commented 1 month ago

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 commented 2 weeks ago

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 commented 2 weeks ago

nperez0111 commented 1 week ago

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 https://github.com/ueberdosis/tiptap/pull/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

ueberdosis / tiptap