`patchAttributes` causes forced reflow on Chrome

[yyjlincoln]

We are using TipTap for our editors, but we have debugged this issue down to ProseMirror.

When initialising the editor (where Prosemirror is attaching to an fresh DOM element), in patchAttributes (reference), the dom.setAttribute statement would set contenteditable to false (where it was undefined and false by default).

Per this gist and its accompanying web.dev post, we have found that this setting of contenteditable does trigger an immediate style recalculation through profiling on the Prosemirror example. This happens even if one sets from undefined (new DOM element -- default to false) to false.

This itself is not an issue with a simple page like this and with one editor; however our use case requires us to use multiple nested editors on a reasonably complex article (e.g. embedded, interactive elements that also uses editors to render its rich text content). This causes multiple setAttributes (across the main and embedded editors) to be called in the same tick, so the browser hangs (and all style recaluations except for the last one in the tick are marked as "forced reflow"). In some extreme cases (where over 50 editors are displayed at the same tick) this causes Interaction-to-Pointer (ITP) delays of over 10 seconds (as the page is very complex with other elements other than the editor). Without these forced reflows, the page performance could increase by more than 90%.

In the image above, the next "Recalculate style", triggered by a nested editor within the same browser tick (as multiple editors are being initialised together) makes the previous style recalculation redundant.

To confirm the issue further, we added debug statements:

function patchAttributes(dom, prev, cur) {
    console.log("PATCH CUR IND", cur);
    for (let name in prev)
        if (name != "class" && name != "style" && name != "nodeName" && !(name in cur))
            dom.removeAttribute(name);
    for (let name in cur)
        if (name != "class" && name != "style" && name != "nodeName" && cur[name] != prev[name]) {
            dom.setAttribute(name, cur[name]);
            if (name === "contenteditable") {
                console.log("SET ATTR", name, cur[name], dom, prev, JSON.stringify(cur)); // <-- debug
                console.trace();
            }
        }

and we observed that prosemirror was trying to set contenteditable to false.

Guarding the dom.setAttribute using:

if (dom.getAttribute(name) !== cur[name])

Seems to have resolved the issue - by default, the DOM element has contenteditable=false so in this case it will skip the setting of contenteditable while initialising, thus avoiding the style recalc/repaint.

Could this please be confirmed by Prosemirror's end?

[marijnh]

I'm definitely missing some context here. What kind of decoration is adding the contenteditable=false attribute, and to what kind of DOM element? Where does the false that this element has 'by default' come from?

[yyjlincoln]

Thanks for getting back :)

Where does the false that this element has 'by default' come from?

For a newly created DOM fragment (say, div), it is not editable by default and has the same behaviour as if contenteditable=false.

It's not necessarily decorations related - from the call stack:

  | patchAttributes | @ | index.js:1704 // PM
  | patchOuterDeco | @ | index.js:1690 // PM
  | applyOuterDeco | @ | index.js:1732 // PM
  | docViewDesc | @ | index.js:1489 // PM
  | updateStateInner | @ | index.js:5411 // PM
  | updateState | @ | index.js:5359 // PM
  | createView | @ | index.js:4502 // TipTap -- `this.view.updateState(newState);` (*)
  | Editor | @ | index.js:4287 // TipTap
  | createEditor | @ | index.js:1022 // TipTap
  | refreshEditorInstance | @ | index.js:1103 // TipTap
  | eval | @ | index.js:1073 // TipTap

(*) A few lines above this:

this.view = new EditorView(...) // PM View
const newState = this.state.reconfigure(...) // PM State

This happens when TipTap creates a view. Let me pull out the newState and see what's in there, but it seems that it happens when we were merely creating an editor instance (on TipTap), which creates a new view

[marijnh]

For a newly created DOM fragment (say, div), it is not editable by default and has the same behaviour as if contenteditable=false.

That doesn't sound right. Such an element will return null when calling getAttribute("contenteditable"), and it will inherit the editablility of its parent—if in editable context, it will be editable, if not, it won't be. But setting contenteditable to "false" definitely has a different meaning than not setting it at all.

[yyjlincoln]

I think you are right; weirdly though when I logged it (for the first patch call) it was already set to false for some reasons.

I added logging to patchAttributes and this is what it showed me:

name: contenteditable
cur[name]: false
dom: <div contenteditable=​"false" translate=​"no" class=​"tiptap ProseMirror focus:​outline-none focus:​border-le-blue">​…​</div>​ 
prev: OuterDecoLevel {}
JSON.stringify(cur): {"class":"ProseMirror focus:outline-none focus:border-le-blue","contenteditable":"false","translate":"no"}
dom.getAttribute(name): false

It could be TipTap sending something bad down to ProseMirror; but as the style recalc is reproducible on ProseMirror demo via profiling (and it is indeed a setAttribute call, most likely setting contenteditable) it probably will still be an issue if multiple editors are rendered at the same time (say, having a button that displays 30 editors in the same tick)...

Any ideas?

In the meantime, I will dig further and see what I have.

[marijnh]

I think boiling this down to a minimal, ProseMirror-only script might help demonstrate what's going on.

[yyjlincoln]

Will do - this will take some time though due to availability. Will give you an update when it's ready.