"/home/yossef/notes/git/projects/maplibra/risks/Theme Controller Listener Cleanup.md"

path: maplibra/risks/Theme Controller Listener Cleanup.md

- **fileName**: Theme Controller Listener Cleanup
- **Created on**: 2026-06-16 00:48:42

Theme Controller Listener Cleanup

Migrated from: obs_vault

Parent: ./Theme Risk Index.md

Severity: high.

Summary

ThemeController registers anonymous event listeners but destroy() removes different references. After repeated map loads, old controllers can keep handling theme events while holding references to old maps, layer managers, and route controllers.

Code Evidence

• src/ui/ThemeController.js: lines 31-33 register the click listener with an anonymous arrow wrapper.
• src/ui/ThemeController.js: lines 37-44 register an anonymous storage listener.
• src/ui/ThemeController.js: lines 46-55 register an anonymous matchMedia listener.
• src/ui/ThemeController.js: lines 125-128 call removeEventListener('click', this.handleThemeToggle), which is not the same function object that was added.
• script.js: cleanup calls themeController?.destroy?.(), so this broken teardown is on the reload path.

Trigger Conditions

• Loading a new MVF after an old map has already run.
• Calling window.InGuide.dispose() or any cleanup/restart path.
• Toggling theme after one or more old controllers were destroyed.
• Cross-tab storage events after old controllers leaked.
• OS color-scheme changes while stale matchMedia listeners still exist.

Likely Symptoms

• Theme toggles fire multiple times after repeated loads.
• Old maps or layer managers receive theme updates after removal.
• Console warnings from stale Mapbox map operations.
• Memory grows across map reloads.
• Storage and system theme changes appear flaky or duplicated.

Why It Matters

This is a lifecycle leak. It can keep large objects alive, including Mapbox map instances, old layer managers, and pathfinding controllers. It is also hard to see in a single-load happy path, so it can survive normal manual testing.

Current Tests

• No dedicated ThemeController tests were found.
• tests/bootstrap-shell.test.js mocks storage and matchMedia indirectly, but does not test controller cleanup.

Missing Tests

• ThemeController.destroy removes click storage and media listeners.
• repeated init destroy does not multiply theme toggles.
• destroyed controller ignores storage theme changes.
• system color-scheme listener is removed on destroy.
• toggle emits one theme-changed event.

How To Make Better

• Store listener references on the instance: this.handleToggleClick, this.handleStorageChange, and this.handleSystemThemeChange.
• Store this.mediaQueryList so cleanup can remove the change listener.
• Support older MediaQueryList.addListener/removeListener if the app needs older WebViews.
• Add this.destroyed = true in destroy() and guard async/event handlers.
• Make handleThemeToggle() compute the next theme and call applyTheme() once, instead of toggleTheme() calling setTheme() and applyTheme() calling setTheme() again.
• Separate “apply theme to document” from “persist explicit user choice” so system-derived initial theme does not become a permanent saved preference.

Debug Steps

1. Load a map, toggle theme once, then load another map.
2. Add instrumentation around handleThemeToggle() and applyTheme().
3. Toggle theme again and confirm whether old controller instances still fire.
4. Dispatch a synthetic storage event and verify only the active controller reacts.

See Also

• ./Theme Controller Risks.md

• ./Shell Bootstrap Risks.md

• ./Theme Tests and Fix Plan.md

  continue:[[]]
  before:[[]]