fix(geolocate): reset control when the browser permission prompt is dismissed

[giswqs]

Summary

Fixes #839.

MapLibre's built-in GeolocateControl permanently disables its button on any PERMISSION_DENIED (code 1) geolocation error. Browsers report code 1 both for a real denial and when the user merely dismisses/clicks away the permission prompt without making a choice. The result: dismissing the prompt left the geolocate button stuck in the blocked state (icon with a red line) with no way to retry, as reported in the issue.

Fix

When the control fires a code-1 error, query the Permissions API:

• If the permission state is not denied (i.e. still prompt — the dialog was dismissed), re-create the control so its button returns to a neutral, clickable state. Clicking again re-initiates the browser permission prompt.
• A genuine, persistent denial keeps MapLibre's disabled state (unchanged behavior).
• If the Permissions API is unavailable, default to re-creating so the user is never permanently stuck.

The re-creation is the deactivate/reactivate approach noted in the issue discussion (the control is not otherwise customizable). The error listener is removed when the control is torn down.

Verification

Drove the real web app with Playwright (geolocation + Permissions API mocked to fire a code-1 error), in both light and dark themes:

Scenario	Before click	After dismiss (code-1 error)	Result
Prompt dismissed (light)	enabled "Find my location"	enabled "Find my location"	retry possible
Prompt dismissed (dark)	enabled	enabled	retry possible
Real denial	disabled "Location not available"	disabled (unchanged)	stays blocked

Also: npm run build passes, tests/map-controller.test.ts (26 tests) passes, pre-commit clean.

[netlify]

✅ Deploy Preview for geolibre-app ready!

Name	Link
🔨 Latest commit	6d180db
🔍 Latest deploy log	https://app.netlify.com/projects/geolibre-app/deploys/6a3c96db24abff0008fab505
😎 Deploy Preview	https://deploy-preview-858--geolibre-app.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coderabbitai]

Warning

Review limit reached

@giswqs, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 30 minutes and 38 seconds. Learn how PR review limits work.

To continue reviewing without waiting, enable usage-based billing in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ac33d453-c53a-495e-84c8-fdda6fe0a63d

📥 Commits

Reviewing files that changed from the base of the PR and between 929fbea and 6d180db.

📒 Files selected for processing (2)

• packages/map/src/map-controller.ts
• tests/map-controller.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-839-geolocate-dismiss-reset

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚡ Cloudflare Pages preview

Item	Value
Preview	https://d48f63ef.geolibre-preview.pages.dev
Demo app	https://d48f63ef.geolibre-preview.pages.dev/demo/
Commit	4f2c722

[github-actions]

Code review

The fix is well-reasoned and the core approach — query the Permissions API after a code-1 error and re-create the control only when the permission state is not denied — correctly addresses the issue. The fallback to recreate() when the Permissions API is absent is a good defensive choice. Below are the findings.

---

Bugs

Finding	Confidence
Stale-control race in recreate() — handleGeolocateError captures nothing at error-time. Between the code-1 error and permissions.query() resolving, the geolocate control could be toggled off and back on (via setBuiltInControlVisible). When the Promise resolves, recreate() sees controlVisibility.geolocate === true, proceeds to call removeGeolocateControl(), and tears down the brand-new, healthy replacement. Fix: snapshot this.geolocateControl at error-time and short-circuit recreate() if this.geolocateControl !== capturedControl. (See inline comment.)	Medium
Missing this.map guard in recreate() — If destroy() is called before the async path resolves, controlVisibility.geolocate is still true, so recreate() isn't short-circuited. It calls removeGeolocateControl() and addGeolocateControl() — both exit safely via their own null-checks, so no crash occurs, but the guard at entry level would make intent clearer.	Low

Quality

Finding	Confidence
No new tests for handleGeolocateError — The PR description confirms 26 existing tests pass, but none exercise the new logic (dismiss path, denial path, no-Permissions-API fallback, .catch() branch). The comment at test line 607–611 acknowledges that instantiating real MapLibre controls in Node is hard, but the permission logic itself is pure JS and could be tested by reaching into internals (as the existing tests do) to call handleGeolocateError with a mock navigator.permissions.	High
Misleading comment scope — "Defer so we don't tear down the control mid error-dispatch" appears only on the queueMicrotask branch, but the permissions.query().then(...) path defers for the same reason (Promise callbacks are microtasks too). A reader might wrongly infer the Promise path doesn't have this property. (See inline comment.)	High

Security / Performance

Nothing of concern in this change.

CLAUDE.md

No violations. The changes are scoped to packages/map/src/map-controller.ts and do not touch UI strings, tile-host CSP, or commit-to-main restrictions.

[github-actions]

Code review

Overall the approach is sound — snapshotting controlAtError before the async boundary and guarding recreate() against stale-map / replaced-control scenarios are exactly the right patterns. The removeGeolocateControl cleanup correctly calls .off() to prevent a listener leak on the old control. Three findings below, one of which I'd call a real (low-probability) bug.

---

Bugs

#	Finding	Confidence
1	status.state !== "denied" can match "granted", not just "prompt" — if a browser quirk (e.g. OS-level revocation between check and use) returns "granted" from the Permissions API while still firing a code-1 error, recreate() is called, which immediately kicks off another geolocation attempt; if that also returns code-1, the cycle repeats indefinitely. Changing to status.state === "prompt" eliminates the case at zero cost. See inline comment on line 1725.	Medium

Quality

#	Finding	Confidence
2	No automated tests for handleGeolocateError — the three async paths (no Permissions API, prompt/granted, denied) and all five guard conditions in recreate() are covered only by a manual Playwright run. The mock infrastructure in map-controller.test.ts could support navigator.permissions stubs. See inline comment spanning lines 1692–1728.	High
3	Minor timing note on queueMicrotask vs Promise.then — both are safe, but they don't fire in the same microtask checkpoint; noted inline at line 1717 for future maintainers.	Low (informational)

Security, Performance, CLAUDE.md

Nothing to flag. The Permissions API is read-only, no user input is interpolated, and the change has no effect on the data-flow / store-driven rendering pipeline described in CLAUDE.md.

[github-actions]

Code review

Bugs

Finding

Confidence

permissions.query() synchronous throw not caught — .catch() only intercepts async rejections. If the Permissions API throws synchronously (partial implementation, CSP policy, Firefox private browsing), the exception escapes handleGeolocateError unhandled. The queueMicrotask fallback already encodes the correct "never leave the user stuck" policy; wrapping the whole block in try/catch { recreate() } applies it consistently. See inline comment at map-controller.ts:1720–1728.

Medium

Quality

Finding	Confidence
.catch(() => recreate()) path is untested — The test suite covers: dismissed prompt, genuine denial, contradictory grant, no Permissions API, non-permission error, replaced control, and destroyed map. The one missing branch is a rejected permissions.query() promise, which exercises .catch(() => recreate()). Adding a stub that returns Promise.reject(...) would close this.	Medium
Global monkey-patching of maplibregl.GeolocateControl in tests — withStubbedControl mutates the module-level constructor. Safe today because node:test runs tests sequentially within a describe, but fragile if --concurrency is ever added. A constructor-injectable factory would remove the global state. Low-priority refactor.	Low

What I checked and found clean

• Race conditions in recreate(): the controlAtError snapshot correctly guards against stale closures acting on a replaced or removed control; the microtask-delay ensures the error dispatch fully completes before any teardown runs.
• removeGeolocateControl() listener cleanup: .off("error", this.handleGeolocateError) correctly detaches the bound handler before removing the control from the map.
• Double-removal safety: removeControl() wraps map.removeControl() in a try/catch, so recreate() can call removeGeolocateControl() + addGeolocateControl() without risking a MapLibre throw.
• controlVisibility.geolocate default false: the guard in addGeolocateControl() means the error handler's recreate() is a no-op unless the geolocate control is actually supposed to be visible.
• flush() timing: setTimeout(resolve, 0) fires after all microtasks (Promise .then callbacks and queueMicrotask), so the test helper correctly drains the async permission check before asserting.
• No security issues, no performance issues, no CLAUDE.md violations.