From 9a6c2b83140baa580bf3dbb9979ab9c0f3a7ef28 Mon Sep 17 00:00:00 2001
From: divya0795 <12871391+divya0795@users.noreply.github.com>
Date: Sun, 26 Jul 2026 13:54:36 +0000
Subject: [PATCH] =?UTF-8?q?=EF=BB=BFfix(remote-popup):=20stop=20the=20refr?=
 =?UTF-8?q?esh=20loop=20from=20re-triggering=20itself?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

updateLinks assigned anchor.textContent unconditionally. Assigning textContent
always removes the existing child text node and inserts a new one, so it emits
a childList mutation record even when the string is identical.

The script observes childList/subtree on document.documentElement, so that
write fed straight back into its own observer:

  refresh -> updateLinks writes textContent -> childList record ->
  scheduleRefresh -> setTimeout(0) -> refresh -> ...

Once any DOM change kicked it off, the loop sustained itself for as long as a
remote-tooltip anchor was in the document, re-running querySelectorAll sweeps
and text-node churn one pass per macrotask and never quiescing.

Compare the href write, which is safe because attributes are not observed, and
updateQrCodes, which already guards with
"canvas.dataset.wandRemoteUrl === remoteUrl". Apply the same idea: compare
before writing, so a link that already holds the right value is left untouched.

Added bridge/src/remote-popup-cleanup.test.ts, which kicks the loop with one
external mutation and then asserts the document stops changing. Against the
unfixed script it records 8 mutations over 8 macrotasks -- one per pass,
confirming the loop is self-sustaining -- and 0 with the fix.

The suite needs DOM globals, which bridge/tsconfig.json omits because it targets
the Node-side bridge, so the test file pulls the DOM lib in locally with a
reference directive rather than widening the project config.
---
 .../scripts/default/remote-popup-cleanup.js   | 14 ++++-
 .../bridge/src/remote-popup-cleanup.test.ts   | 56 +++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 web-panel/bridge/src/remote-popup-cleanup.test.ts

diff --git a/web-panel/bridge/scripts/default/remote-popup-cleanup.js b/web-panel/bridge/scripts/default/remote-popup-cleanup.js
index 750e848..13db45e 100644
--- a/web-panel/bridge/scripts/default/remote-popup-cleanup.js
+++ b/web-panel/bridge/scripts/default/remote-popup-cleanup.js
@@ -100,9 +100,19 @@ import { resolveQrRenderer as findWandQrRenderer } from "./remote-popup-cleanup/
       return
     }
 
+    const linkText = remoteUrl.replace(/\/$/, "")
     for (const anchor of document.querySelectorAll("remote-tooltip a[href]")) {
-      anchor.setAttribute("href", remoteUrl)
-      anchor.textContent = remoteUrl.replace(/\/$/, "")
+      if (anchor.getAttribute("href") !== remoteUrl) {
+        anchor.setAttribute("href", remoteUrl)
+      }
+
+      // Assigning textContent always replaces the child text node, which emits a
+      // childList record even when the string is unchanged. The observer below
+      // watches childList on documentElement, so an unguarded write here would
+      // schedule the next refresh and never settle.
+      if (anchor.textContent !== linkText) {
+        anchor.textContent = linkText
+      }
     }
   }
 
diff --git a/web-panel/bridge/src/remote-popup-cleanup.test.ts b/web-panel/bridge/src/remote-popup-cleanup.test.ts
new file mode 100644
index 0000000..93dd5b5
--- /dev/null
+++ b/web-panel/bridge/src/remote-popup-cleanup.test.ts
@@ -0,0 +1,56 @@
+// bridge/tsconfig.json targets the Node-side bridge and so omits the DOM lib.
+// This suite drives a renderer script under vitest's jsdom environment, so it
+// pulls the DOM types in locally rather than widening the project config.
+/// <reference lib="dom" />
+
+import { afterAll, describe, expect, it } from 'vitest';
+
+const REMOTE_URL = 'http://192.168.1.5:8080/';
+
+/** Lets the script's setTimeout(0) refresh loop run for a few turns. */
+async function settle(turns = 8) {
+  for (let index = 0; index < turns; index += 1) {
+    await new Promise((resolve) => setTimeout(resolve, 0));
+  }
+}
+
+describe('remote popup cleanup', () => {
+  afterAll(() => {
+    document.body.innerHTML = '';
+  });
+
+  it('leaves the remote link alone once it matches, so the refresh loop settles', async () => {
+    document.body.innerHTML =
+      '<remote-tooltip><a href="http://stale.invalid/">stale</a></remote-tooltip>';
+    (globalThis as Record<string, unknown>).__wandRemoteBridgeUrl = REMOTE_URL;
+
+    // Installs on import: runs one refresh, then observes documentElement.
+    await import('../scripts/default/remote-popup-cleanup.js');
+    await settle();
+
+    const anchor = document.querySelector('remote-tooltip a[href]');
+    expect(anchor?.getAttribute('href')).toBe(REMOTE_URL);
+    expect(anchor?.textContent).toBe('http://192.168.1.5:8080');
+
+    // The script's own refresh happens before it starts observing, so the loop
+    // needs one external mutation to kick it off. After this the link already
+    // holds the right value, so a correct updateLinks writes nothing further.
+    document.body.appendChild(document.createElement('div'));
+    await settle(2);
+
+    // An unguarded `anchor.textContent = ...` replaces the child text node on
+    // every pass. That is a childList record on documentElement, which
+    // re-triggers the script's observer, which schedules another refresh --
+    // a loop that never quiesces while a remote-tooltip anchor is present.
+    let mutationCount = 0;
+    const probe = new MutationObserver((records) => {
+      mutationCount += records.length;
+    });
+    probe.observe(document.documentElement, { childList: true, subtree: true });
+
+    await settle();
+    probe.disconnect();
+
+    expect(mutationCount).toBe(0);
+  });
+});