Description
Opening the JavaScript Editor widget adds 4 stylesheet <link> tags to document.head. Closing the widget doesn't remove them, and there's nothing else in the file that ever cleans them up. So if you open the editor, close it, and open it again, you get 4 more. Do that a few times and document.head just keeps growing.
Here's the constructor in js/widgets/jseditor.js (lines 57-67):
this._currentStyle = 0;
this._styles = ["dracula", "github", "railscasts", "vs"].map(name => {
const link = document.createElement("link");
link.href = `././lib/codejar/styles/${name}.min.css`;
link.rel = "stylesheet";
link.disabled = "true";
document.head.appendChild(link);
return link;
});
this._styles[this._currentStyle].removeAttribute("disabled");
this._styles only ever gets touched again for theme switching (_changeStyle, ~line 1217) — nothing removes these links.
And the close handler (_setup(), lines 284-294) only cleans up the resize listeners, not the stylesheets:
this.widgetWindow.onclose = () => {
if (this._resizeHandlers) {
document.removeEventListener("mousemove", this._resizeHandlers.doResize);
document.removeEventListener("mouseup", this._resizeHandlers.stopResize);
this._resizeHandlers = null;
}
this.isOpen = false;
defaultOnClose();
};
There's a second thing making it worse: toggleJSEditor() in js/activity/help-controller.js has no check for whether the editor is already open — it just does new JSEditor(this.activity) every time (line 70). Compare that to showStats() a few lines below it, which does check:
async toggleJSEditor() { // no guard
await lazyLoad([...]);
new JSEditor(this.activity);
}
async showStats() { // guarded
if (!this.activity.statsWindow || !this.activity.statsWindow.isOpen) {
await lazyLoad("widgets/statistics");
this.activity.statsWindow = new StatsWindow(this.activity);
}
}
So every click of the JS Editor button in the toolbar makes a brand new JSEditor instance regardless of whether one's already open, which means 4 more <link> tags every time.
How to reproduce
I wrote a quick Jest test using the mocks already in jseditor.test.js:
const countLinks = () => document.head.querySelectorAll('link[href*="codejar/styles/"]').length;
new JSEditor({});
countLinks(); // 4
mockWidgetWindow.onclose();
countLinks(); // still 4
new JSEditor({});
countLinks(); // 8
mockWidgetWindow.onclose();
new JSEditor({});
countLinks(); // 12
Ran it, confirmed it, and reverted the file after (didn't want to leave a stray test in the repo). In the actual app this is just: open the JS Editor from the toolbar, close it, open it again. Nothing unusual about the sequence, it's exactly how you'd normally use the widget.
Expected behavior
Opening/closing the editor a bunch of times shouldn't leave a growing pile of orphaned <link> tags in document.head.
Console errors
None — it doesn't throw, it just quietly piles up DOM nodes.
Environment
- current
master
- reproducible via Jest/jsdom as shown above, browser-independent
Why it matters
This is a normal usage pattern, not some edge case — open/close is how the widget is meant to be used. Someone leaving Music Blocks open for a while (a classroom session is the obvious case here) and repeatedly checking the generated JS output ends up with a growing number of dead stylesheet nodes for no reason. Each link also triggers a fetch/cache lookup for its CSS file, so it's not just inert bloat.
Possible fix
Either remove the links in onclose (same pattern already used for the resize listeners right there), or just create the 4 links once and reuse them across opens instead of rebuilding them every time. Also worth adding the same "already open" guard showStats() has, so the widget doesn't get reconstructed on every click in the first place. Would need a regression test covering an open → close → open cycle checking the link count doesn't grow.
Checklist
Description
Opening the JavaScript Editor widget adds 4 stylesheet
<link>tags todocument.head. Closing the widget doesn't remove them, and there's nothing else in the file that ever cleans them up. So if you open the editor, close it, and open it again, you get 4 more. Do that a few times anddocument.headjust keeps growing.Here's the constructor in
js/widgets/jseditor.js(lines 57-67):this._stylesonly ever gets touched again for theme switching (_changeStyle, ~line 1217) — nothing removes these links.And the close handler (
_setup(), lines 284-294) only cleans up the resize listeners, not the stylesheets:There's a second thing making it worse:
toggleJSEditor()injs/activity/help-controller.jshas no check for whether the editor is already open — it just doesnew JSEditor(this.activity)every time (line 70). Compare that toshowStats()a few lines below it, which does check:So every click of the JS Editor button in the toolbar makes a brand new
JSEditorinstance regardless of whether one's already open, which means 4 more<link>tags every time.How to reproduce
I wrote a quick Jest test using the mocks already in
jseditor.test.js:Ran it, confirmed it, and reverted the file after (didn't want to leave a stray test in the repo). In the actual app this is just: open the JS Editor from the toolbar, close it, open it again. Nothing unusual about the sequence, it's exactly how you'd normally use the widget.
Expected behavior
Opening/closing the editor a bunch of times shouldn't leave a growing pile of orphaned
<link>tags indocument.head.Console errors
None — it doesn't throw, it just quietly piles up DOM nodes.
Environment
masterWhy it matters
This is a normal usage pattern, not some edge case — open/close is how the widget is meant to be used. Someone leaving Music Blocks open for a while (a classroom session is the obvious case here) and repeatedly checking the generated JS output ends up with a growing number of dead stylesheet nodes for no reason. Each link also triggers a fetch/cache lookup for its CSS file, so it's not just inert bloat.
Possible fix
Either remove the links in
onclose(same pattern already used for the resize listeners right there), or just create the 4 links once and reuse them across opens instead of rebuilding them every time. Also worth adding the same "already open" guardshowStats()has, so the widget doesn't get reconstructed on every click in the first place. Would need a regression test covering an open → close → open cycle checking the link count doesn't grow.Checklist