From a82dc8d3f121a9745c0952b01507d46e157494aa Mon Sep 17 00:00:00 2001
From: Umputun <umputun@gmail.com>
Date: Sat, 22 Aug 2026 03:09:40 -0500
Subject: [PATCH] Restore the legacy /web/*.js URLs and fix iframe reuse
 (#2192)

* Serve the legacy /web/*.js names from their .mjs siblings

The build emitted <name>.js alongside <name>.mjs until the two compilations
were collapsed into one. Dropping the second compilation was right, but it
removed URLs the project itself had published: the v1.16.4 SPA documentation
named /web/embed.js directly and its loader snippet requested .js. Pages that
hard-coded those names now 404 with no deprecation.

webFiles.Open retries a missing .js against the .mjs sibling. The bundles
contain no import or export, so the same bytes serve both names. The retry
runs only once both sources report the name missing, so a real .js still
wins, and an unreadable sibling reports its own error rather than being
flattened into the requested file's 404.

Related to #2178

* Reuse only the comments iframe embed created

createInstance took root.firstElementChild as its iframe, so anything a page
left inside #remark42 was adopted instead. A <noscript> fallback became the
"iframe", createIframe never ran, and the height messages went to an element
that cannot show comments.

That also defeats the placeholder support, which promises content in the root
is cleared once the iframe reports inited: a text placeholder works, but any
element placeholder is mistaken for the iframe, so inited never arrives and
the cleanup never runs.

The iframe now carries data-remark42-iframe and the lookup is scoped to a
direct child, so a second createInstance still reuses it while nothing else
in the root can be adopted.

Related to #1990

* Assert the backup contents rather than the compressed size

TestBackup_MakeBackup and TestBackup_Do pinned the gzip output at 52 bytes,
which ties them to the exact output of compress/flate. The same input encodes
to 57 bytes on go 1.27, so both fail for anyone building on a toolchain newer
than the one CI pins.

They now read the backup back and compare it against what the exporter wrote,
which is what the tests were reaching for and does not move with the
compressor. The payload is a shared constant so the two cannot drift.
---
 backend/app/migrator/backup_test.go      | 29 +++++++---
 backend/app/rest/api/webfiles.go         | 30 +++++++++-
 backend/app/rest/api/webfiles_test.go    | 73 ++++++++++++++++++++++++
 e2e/widgets_test.go                      | 38 ++++++++++++
 frontend/apps/remark42/app/embed.test.ts | 53 +++++++++++++++++
 frontend/apps/remark42/app/embed.ts      |  7 ++-
 6 files changed, 219 insertions(+), 11 deletions(-)
 create mode 100644 frontend/apps/remark42/app/embed.test.ts

diff --git a/backend/app/migrator/backup_test.go b/backend/app/migrator/backup_test.go
index c9c86879..1e4d4b10 100644
--- a/backend/app/migrator/backup_test.go
+++ b/backend/app/migrator/backup_test.go
@@ -1,6 +1,7 @@
 package migrator
 
 import (
+	"compress/gzip"
 	"context"
 	"fmt"
 	"io"
@@ -50,9 +51,7 @@ func TestBackup_MakeBackup(t *testing.T) {
 	expFile := fmt.Sprintf("/tmp/remark-backups.test/backup-site1-%s.gz", time.Now().Format("20060102"))
 	assert.Equal(t, expFile, fname)
 
-	fi, err := os.Lstat(expFile)
-	assert.NoError(t, err)
-	assert.Equal(t, int64(52), fi.Size())
+	assert.Equal(t, exportedPayload, gzContent(t, expFile))
 }
 
 func TestBackup_Do(t *testing.T) {
@@ -71,15 +70,31 @@ func TestBackup_Do(t *testing.T) {
 		bk.Do(ctx)
 
 		expFile := fmt.Sprintf("/tmp/remark-backups.test/backup-site1-%s.gz", time.Now().Format("20060102"))
-		fi, err := os.Lstat(expFile)
-		assert.NoError(t, err)
-		assert.Equal(t, int64(52), fi.Size())
+		assert.Equal(t, exportedPayload, gzContent(t, expFile))
 	})
 }
 
+const exportedPayload = "some export blah blah 1234567890"
+
+// the compressed size is not assertable: it moves with the compress/flate version
+func gzContent(t *testing.T, name string) string {
+	t.Helper()
+	fh, err := os.Open(name) //nolint:gosec // path is built by the test
+	require.NoError(t, err)
+	defer func() { assert.NoError(t, fh.Close()) }()
+
+	gz, err := gzip.NewReader(fh)
+	require.NoError(t, err)
+	defer func() { assert.NoError(t, gz.Close()) }()
+
+	b, err := io.ReadAll(gz)
+	require.NoError(t, err)
+	return string(b)
+}
+
 type mockExporter struct{}
 
 func (mock *mockExporter) Export(w io.Writer, _ string) (int, error) {
-	_, err := w.Write([]byte("some export blah blah 1234567890"))
+	_, err := w.Write([]byte(exportedPayload))
 	return 1000, err
 }
diff --git a/backend/app/rest/api/webfiles.go b/backend/app/rest/api/webfiles.go
index d82d67f8..e5c3b120 100644
--- a/backend/app/rest/api/webfiles.go
+++ b/backend/app/rest/api/webfiles.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"io/fs"
 	"path/filepath"
+	"strings"
 )
 
 // webFiles serves /web from two sources: a name present in the frontend build is served from there,
@@ -13,15 +14,38 @@ type webFiles struct {
 	embedded fs.FS
 }
 
-// Open looks the name up in the frontend build first. Only a missing file falls through to the
-// embedded assets; every other error is returned so an unreadable file keeps reporting as one
-// rather than being replaced by the embedded copy or reported as missing.
+// Open resolves the name against both sources, and answers a missing .js with the .mjs sibling.
+// The build stopped emitting .js while integrations still request it; the bundles carry no module
+// syntax, so the same bytes serve both names.
 func (w webFiles) Open(name string) (fs.File, error) {
 	// fs.ValidPath alone is not enough: it accepts names the operating system rejects, NUL among
 	// them, and os.DirFS turns those into fs.ErrInvalid, which renders as 500 rather than 404
 	if _, err := filepath.Localize(name); err != nil || !fs.ValidPath(name) {
 		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrNotExist}
 	}
+
+	f, err := w.open(name)
+	if err == nil {
+		return f, nil
+	}
+	if !errors.Is(err, fs.ErrNotExist) || !strings.HasSuffix(name, ".js") {
+		return nil, err
+	}
+
+	alias, aliasErr := w.open(strings.TrimSuffix(name, ".js") + ".mjs")
+	if aliasErr == nil {
+		return alias, nil
+	}
+	if !errors.Is(aliasErr, fs.ErrNotExist) {
+		return nil, aliasErr
+	}
+	return nil, err
+}
+
+// open looks the name up in the frontend build first. Only a missing file falls through to the
+// embedded assets; every other error is returned so an unreadable file keeps reporting as one
+// rather than being replaced by the embedded copy or reported as missing.
+func (w webFiles) open(name string) (fs.File, error) {
 	f, err := w.frontend.Open(name)
 	if err == nil {
 		return f, nil
diff --git a/backend/app/rest/api/webfiles_test.go b/backend/app/rest/api/webfiles_test.go
index f82647e7..c4e994e5 100644
--- a/backend/app/rest/api/webfiles_test.go
+++ b/backend/app/rest/api/webfiles_test.go
@@ -54,6 +54,79 @@ func TestWebFiles_Open(t *testing.T) {
 	}
 }
 
+func TestWebFiles_OpenJSAlias(t *testing.T) {
+	frontend := fstest.MapFS{
+		"embed.mjs":   {Data: []byte("module embed")},
+		"counter.js":  {Data: []byte("operator's own counter")},
+		"counter.mjs": {Data: []byte("module counter")},
+		"widget.mjs":  {Data: []byte("module widget")},
+	}
+	embedded := fstest.MapFS{
+		"legacy.mjs": {Data: []byte("module legacy")},
+		"widget.js":  {Data: []byte("embedded widget")},
+	}
+	w := webFiles{frontend: frontend, embedded: embedded}
+
+	tbl := []struct {
+		name    string
+		lookup  string
+		want    string
+		wantErr error
+	}{
+		{name: "missing js served from the mjs sibling", lookup: "embed.js", want: "module embed"},
+		{name: "alias reaches the embedded assets too", lookup: "legacy.js", want: "module legacy"},
+		{name: "a real js file wins over its sibling", lookup: "counter.js", want: "operator's own counter"},
+		{name: "an embedded js wins over a frontend sibling", lookup: "widget.js", want: "embedded widget"},
+		{name: "mjs is still served directly", lookup: "embed.mjs", want: "module embed"},
+		{name: "neither name present", lookup: "absent.js", wantErr: fs.ErrNotExist},
+		{name: "only js aliases, not other extensions", lookup: "embed.html", wantErr: fs.ErrNotExist},
+	}
+
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			f, err := w.Open(tt.lookup)
+			if tt.wantErr != nil {
+				require.Error(t, err)
+				assert.ErrorIs(t, err, tt.wantErr)
+				return
+			}
+			require.NoError(t, err)
+			defer f.Close()
+			b, err := io.ReadAll(f)
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, string(b))
+		})
+	}
+}
+
+func TestWebFiles_OpenJSAliasNamesTheRequestedFile(t *testing.T) {
+	w := webFiles{frontend: fstest.MapFS{}, embedded: fstest.MapFS{}}
+
+	_, err := w.Open("absent.js")
+	require.Error(t, err)
+	assert.ErrorIs(t, err, fs.ErrNotExist)
+	assert.Contains(t, err.Error(), "absent.js")
+	assert.NotContains(t, err.Error(), "absent.mjs")
+}
+
+func TestWebFiles_OpenJSAliasUnreadableSibling(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root ignores file permissions")
+	}
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "embed.mjs"), []byte("module embed"), 0o000))
+	w := webFiles{frontend: os.DirFS(dir), embedded: fstest.MapFS{}}
+
+	f, err := w.Open("embed.js")
+	require.Error(t, err)
+	assert.ErrorIs(t, err, fs.ErrPermission)
+	assert.NotErrorIs(t, err, fs.ErrNotExist, "an unreadable sibling must not render as 404")
+	if err == nil {
+		_ = f.Close()
+	}
+}
+
 // TestEmptyFS_ServesNothing pins the stand-in used when the frontend source cannot be opened:
 // every name must report as missing rather than panicking, since it backs a nil-free fallback.
 func TestEmptyFS_ServesNothing(t *testing.T) {
diff --git a/e2e/widgets_test.go b/e2e/widgets_test.go
index ca58ccff..4d7a150d 100644
--- a/e2e/widgets_test.go
+++ b/e2e/widgets_test.go
@@ -104,6 +104,44 @@ func TestWidgets_CounterFillsInTheCommentCount(t *testing.T) {
 	})
 }
 
+func TestWidgets_LegacyJSURLLoadsAsAClassicScript(t *testing.T) {
+	thread := threadURL(t)
+
+	poster := newPage(t)
+	frame := openThread(t, poster)
+	signInAnon(t, frame, "aliastester")
+	postComment(t, frame, "alias "+runID)
+
+	want := strconv.Itoa(commentCount(t, poster, thread))
+
+	page := newPage(t)
+	pauseForAuthLimit()
+	_, err := page.Goto(baseURL + "/web/privacy.html")
+	require.NoError(t, err)
+
+	_, err = page.Evaluate(`([host, url]) => {
+		window.remark_config = { host, site_id: 'remark' };
+		const node = document.createElement('span');
+		node.className = 'remark42__counter';
+		node.dataset.url = url;
+		document.body.appendChild(node);
+	}`, []any{baseURL, thread})
+	require.NoError(t, err)
+
+	counter := page.Locator(".remark42__counter")
+	blank, err := pollText(counter)
+	require.NoError(t, err)
+	require.Empty(t, blank, "the counter must start blank or the assertion below is vacuous")
+
+	_, err = page.AddScriptTag(playwright.PageAddScriptTagOptions{URL: playwright.String(baseURL + "/web/counter.js")})
+	require.NoError(t, err)
+
+	eventually(t, waitTimeout, "the legacy counter.js url never filled the counter", func() bool {
+		txt, ierr := pollText(counter)
+		return ierr == nil && txt == want
+	})
+}
+
 // commentCount asks the API what the counter should be showing
 func commentCount(t *testing.T, page playwright.Page, url string) int {
 	t.Helper()
diff --git a/frontend/apps/remark42/app/embed.test.ts b/frontend/apps/remark42/app/embed.test.ts
new file mode 100644
index 00000000..d7375d11
--- /dev/null
+++ b/frontend/apps/remark42/app/embed.test.ts
@@ -0,0 +1,53 @@
+process.env.REMARK_NODE = 'remark42';
+
+const MARKER = 'data-remark42-iframe';
+const MARKED = `iframe[${MARKER}]`;
+
+async function mount(placeholder = '') {
+  document.body.innerHTML = `<div id="remark42">${placeholder}</div>`;
+  jest.resetModules();
+  await import('./embed');
+
+  return document.getElementById('remark42')!;
+}
+
+describe('embed', () => {
+  beforeEach(() => {
+    jest.useFakeTimers();
+  });
+
+  afterEach(() => {
+    jest.useRealTimers();
+    document.body.innerHTML = '';
+  });
+
+  it('creates its iframe rather than adopting an element placeholder', async () => {
+    const root = await mount('<noscript>Please enable JavaScript to view the comments.</noscript>');
+
+    expect(root.querySelectorAll(MARKED)).toHaveLength(1);
+  });
+
+  it('creates its iframe rather than adopting an unmarked one', async () => {
+    const root = await mount('<iframe title="placeholder"></iframe>');
+
+    expect(root.querySelectorAll('iframe')).toHaveLength(2);
+    expect(root.querySelectorAll(MARKED)).toHaveLength(1);
+  });
+
+  it('creates its iframe rather than adopting a marked one further down the tree', async () => {
+    const root = await mount(`<div><iframe ${MARKER}></iframe></div>`);
+
+    expect(root.querySelectorAll('iframe')).toHaveLength(2);
+    expect(root.querySelectorAll(`:scope > ${MARKED}`)).toHaveLength(1);
+  });
+
+  it('reuses its own iframe on a second createInstance', async () => {
+    const root = await mount();
+    const first = root.querySelector(MARKED);
+
+    window.REMARK42.createInstance(window.remark_config);
+
+    expect(root.querySelectorAll('iframe')).toHaveLength(1);
+    expect(root.querySelector(MARKED)).toBe(first);
+  });
+});
diff --git a/frontend/apps/remark42/app/embed.ts b/frontend/apps/remark42/app/embed.ts
index ae1a6eea..3fa5f8b5 100644
--- a/frontend/apps/remark42/app/embed.ts
+++ b/frontend/apps/remark42/app/embed.ts
@@ -4,6 +4,10 @@ import { createIframe } from 'utils/create-iframe';
 import type { Theme } from 'common/types';
 import { closeProfile, openProfile } from 'profile';
 
+// marks the iframe this module owns, so a second createInstance reuses it instead of adopting
+// whatever the integrator left in the root as a loading placeholder
+const IFRAME_MARKER = 'data-remark42-iframe';
+
 if (document.readyState === 'loading') {
   document.addEventListener('DOMContentLoaded', init);
 } else {
@@ -36,7 +40,8 @@ function createInstance(config: typeof window.remark_config) {
 
   config.url = (config.url || `${window.location.origin}${window.location.pathname}`).split('#')[0];
 
-  const iframe = (root.firstElementChild as HTMLIFrameElement) || createIframe(config);
+  const iframe = root.querySelector<HTMLIFrameElement>(`:scope > iframe[${IFRAME_MARKER}]`) ?? createIframe(config);
+  iframe.setAttribute(IFRAME_MARKER, '');
 
   root.appendChild(iframe);