Note editor cannot survive a ProseMirror reconfigure: two plugin-teardown bugs (break plugin interop)

[Marie-Jean THORAVAL]

Hi all,

While debugging citation/link popups dying when two note-editor plugins are active together, I traced the failure to two latent bugs in the bundled note editor's plugin teardown. Vanilla Zotero never runs that code path, so they have gone unnoticed; any plugin that injects a ProseMirror plugin into the editor triggers it. Everything below was verified on a clean profile (Zotero 10.0-beta.22, no plugins installed beyond a dev bridge that does not touch the editor).

Reproduction (console only, no plugins needed) — run once against a live editor view:

view.updateState(view.state.reconfigure({ plugins: [...view.state.plugins] }));

Verified result: the plugin-view destroy loop throws

TypeError: can't access property "dom", this.editorView is undefined

the update aborts mid-way (live plugin views dropped from 15 to 13 in my run), and plugin-view recreation never runs. Popup plugin views (citation / link / highlight — "Show Item", "Edit Citation", "Go to Page") are left dead or orphaned from that point on.

Bug 1: drag.js destroy() has never worked. On current master (file last touched 2025-12):

constructor(view, options) { this.view = view; ... this.handlers = ['mousemove'].map((name) => { ... return { name, handler }; }); } destroy() { this.handlers.forEach((name, handler) => { return this.editorView.dom.removeEventListener(name, handler); }); }

this.editorView is never assigned anywhere in the class (the constructor stores this.view), so destroy() throws — this is the first error the reconfigure hits. And even with that fixed, the forEach signature is wrong: handlers is an array of {name, handler} objects, so the callback receives (element, index), not (name, handler) — the listener would never actually be removed, leaking a mousemove handler per teardown. Suggested fix:

destroy() { this.handlers.forEach(({ name, handler }) => { this.view.dom.removeEventListener(name, handler); }); }

Bug 2: the three colour plugins call a destroy() that does not exist. The view wrappers all do destroy() { pluginState.destroy(); }, but these state classes define update() only, no destroy() (checked against current master):

src/core/plugins/highlight-color.js -- class HighlightColor, wrapper destroy at lines 172-173 src/core/plugins/text-color.js -- class Color, wrapper destroy at lines 116-117 src/core/plugins/underline-color.js -- class UnderlineColor, wrapper destroy at lines 185-186

Fix: add destroy() {} to the three classes, or make the wrappers defensive: pluginState.destroy?.();

Why the two bugs surface one at a time (and why symptom reports can differ): ProseMirror's destroy loop pops each view before calling its destroy(), so the throwing Drag view is removed by the failed pass. A second reconfigure then gets past drag and reaches the colour wrappers, whose missing destroy surfaces in the shipped minified bundle as

TypeError: t.destroy is not a function

which is the form we originally captured in the wild. With two editor plugins installed, that is exactly the sequence: the first plugin's injection trips bug 1, the second plugin's injection trips bug 2.

Why vanilla never sees any of this: the editor never reconfigures itself, and teardown does not call view.destroy() either (the iframe is simply unloaded). The destroy path is dead code until a plugin injects into the editor — at which point both bugs are live.

Real-world impact: we saw this as "citation popups stop working when Weavero and Better Notes are both enabled". Weavero now ships a workaround (hardening plugin-view destroys around updateState), but the fixes belong in the editor — they are one line each.

Happy to send a PR covering both.

[Martynas Bagdonas]

I pushed the fixes to note-editor. Let us know if everything works as expected now. Thanks for the detailed description of the issue.

[Marie-Jean THORAVAL]

Thank you for the quick turnaround! I can confirm the fix fully resolves the problems — verified end-to-end on a source build (10.0.SOURCE) with the note-editor submodule at e43f358, with the same plugin combination as the original report (Weavero + Better Notes). Detailed report below:

What I ran, against the exact failure modes from my report:

Vanilla-editor leg (Weavero disabled; Better Notes 3.3.0-beta.5 and Zotero Annotation Markdown 0.5.2 active): three consecutive view.updateState(view.state.reconfigure(...)) calls — zero exceptions, plugin views stable at 18 through every round (previously: round 1 threw the drag TypeError and dropped views 15 -> 13, round 2 threw "t.destroy is not a function"). Listener accounting is clean too: mousemove on the editor root and mouseup on the window both come back to their starting counts, so the leak fixed alongside (the old destroy never actually removed its listener) is gone as well.

Interop leg — the original failing scenario: Weavero AND Better Notes active together (plus Annotation Markdown), with Weavero injecting its ProseMirror plugin — the reconfigure that used to kill the popup plugin views. The injection succeeds, three further reconfigures stay clean, and the popup-class plugin views remain functional afterwards (decorations render, dispatch works). The "citation popups die with Weavero + Better Notes" symptom is gone.

Runtime spot-check: the live drag view carries exactly the new destroy() (named handlers removed, throttle cancelled, drag-handle node removed).

Also ran the new plugin-lifecycle regression suite standalone: 5 passing. Two small notes there from a fresh clone: the tests need locales/en-US/*.ftl to exist for module resolution (empty files suffice, register.cjs stubs the content), and under Node 24 mocha loads the test file through the ESM path, bypassing @babel/register, so the extensionless src/ imports fail — it runs fine forced through the CommonJS require path. Your CI presumably pins an older Node, so this may only affect local runs.

From my side this is fully resolved — thanks again!