remote-popup-cleanup: unguarded textContent write feeds its own MutationObserver (refresh loop never settles)

[divya0795]

Summary

remote-popup-cleanup.js feeds its own MutationObserver. Once any DOM change kicks the loop off, the script re-runs one full refresh per macrotask for as long as a remote-tooltip anchor is in the document, and never settles.

Observed on master @ 643c8f8 (1.0.9.4).

Root cause

web-panel/bridge/scripts/default/remote-popup-cleanup.js

// :98-107
const updateLinks = (remoteUrl) => {
  if (!remoteUrl) return

  for (const anchor of document.querySelectorAll("remote-tooltip a[href]")) {
    anchor.setAttribute("href", remoteUrl)
    anchor.textContent = remoteUrl.replace(/\/$/, "")   // ← no equality guard
  }
}

// :150-155
const observer = new MutationObserver(scheduleRefresh)
observer.observe(document.documentElement, { childList: true, subtree: true })

Assigning Node.textContent always removes the existing child text node and inserts a new one. It therefore emits a childList mutation record even when the assigned string is identical to what is already there.

The observer watches childList / subtree on documentElement, so that write is observed by the script itself:

refresh → updateLinks writes textContent → childList record →
scheduleRefresh → setTimeout(0) → refresh → …

refreshScheduled only coalesces concurrent schedules; it is reset before refresh() runs, so each pass schedules the next one.

Two nearby pieces of the same file are already correct, which is what makes this look unintended:

• setAttribute("href", …) is harmless — attributes is not in the observer's options.
• updateQrCodes guards its work with if (canvas.dataset.wandRemoteUrl === remoteUrl) continue (:116). updateLinks has no equivalent.
• installStyle is guarded too (if (!document.getElementById(style.id)), :81), so it is not a contributor.

Reproduction

The script's initial refresh() (:148) runs before observer.observe(...) (:152), so the loop needs one external DOM mutation to start. In practice the Wand renderer supplies those constantly; in a test one appendChild is enough.

Reproduced as a unit test under jsdom — open a remote-tooltip anchor, let the script install, then make a single unrelated DOM change and count mutations over 8 macrotasks:

AssertionError: expected 8 to be 0
- 0
+ 8

Exactly one mutation per macrotask, i.e. the loop is self-sustaining. Expected: 0 — with the link already correct, a refresh should change nothing.

Impact

While a remote tooltip is open the panel performs a continuous querySelectorAll sweep plus text-node teardown/rebuild every macrotask, with the attendant reflow and observer churn. No incorrect output — the link text ends up right — so the cost is wasted CPU that never stops rather than a visible failure.

Suggested fix

Compare before writing, mirroring the existing updateQrCodes guard:

const linkText = remoteUrl.replace(/\/$/, "")
for (const anchor of document.querySelectorAll("remote-tooltip a[href]")) {
  if (anchor.getAttribute("href") !== remoteUrl) anchor.setAttribute("href", remoteUrl)
  if (anchor.textContent !== linkText) anchor.textContent = linkText
}

A fix with that test attached is ready to submit.

[divya0795]

Fix submitted in #169.