🐛(frontend) fix camera/microphone permission detection on mobile browsers#1157
Open
mmaudet wants to merge 2 commits intosuitenumerique:mainfrom
Open
🐛(frontend) fix camera/microphone permission detection on mobile browsers#1157mmaudet wants to merge 2 commits intosuitenumerique:mainfrom
mmaudet wants to merge 2 commits intosuitenumerique:mainfrom
Conversation
Collaborator
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis pull request introduces a browser-agnostic approach to handling permission state changes in the permissions watching hook. A new utility function detects browsers with unreliable Permissions API change events (Safari, iOS browsers, and Firefox). The hook is refactored to conditionally enable polling only when events are unreliable or when permissions are in 'prompt' state. Individual permission queries are now wrapped in try/catch handlers with fallback behavior, while event listeners are guarded and conditionally removed during cleanup. The polling loop queries camera and microphone states separately with guards for unsupported queries and terminates once both permissions exit 'prompt' state. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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. Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontend/src/features/rooms/hooks/useWatchPermissions.ts`:
- Around line 30-39: The catch blocks in useWatchPermissions currently write to
permissionsStore (cameraPermission/microphonePermission) even if the hook has
been torn down; add a cancellation guard (e.g., an isActive/isMounted boolean or
AbortController flag stored in the hook scope) that is set false in the
useEffect cleanup and check that flag before mutating permissionsStore in both
catch paths (and after any awaited navigator.permissions.query calls); ensure
the cleanup toggles the flag so no writes occur after unmount.
- Around line 74-116: In useWatchPermissions, the polling loop can run forever
if navigator.permissions.query() always throws (updatedCameraState and
updatedMicrophoneState remain null); update the stop condition so the poll also
stops when both permission queries are unsupported (both states are null).
Concretely, inside the polling completion logic (the block using
updatedCameraState/updatedMicrophoneState, permissionsStore and the "Stop
polling when both permissions are resolved" check), add a branch that treats
{updatedCameraState === null && updatedMicrophoneState === null} as a terminal
condition and stop the interval / return early (preserving existing updates to
permissionsStore when one or the other is non-null). Ensure you reference
useWatchPermissions, updatedCameraState, updatedMicrophoneState,
navigator.permissions.query, permissionsStore and isCancelled when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: eda8fcf9-705d-4436-9277-779ef00a33f6
📒 Files selected for processing (2)
src/frontend/src/features/rooms/hooks/useWatchPermissions.tssrc/frontend/src/utils/livekit.ts
Comment on lines
+30
to
+39
| } catch { | ||
| permissionsStore.cameraPermission = 'prompt' | ||
| } | ||
|
|
||
| try { | ||
| microphonePermission = await navigator.permissions.query({ | ||
| name: 'microphone' as PermissionName, | ||
| }) | ||
| } catch { | ||
| permissionsStore.microphonePermission = 'prompt' |
There was a problem hiding this comment.
Guard catch-path store writes after cancellation.
Line 31 and Line 39 can still mutate global permissionsStore after teardown if unmount happens while queries are in flight.
Proposed fix
} catch {
- permissionsStore.cameraPermission = 'prompt'
+ if (!isCancelled) {
+ permissionsStore.cameraPermission = 'prompt'
+ }
}
@@
} catch {
- permissionsStore.microphonePermission = 'prompt'
+ if (!isCancelled) {
+ permissionsStore.microphonePermission = 'prompt'
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/features/rooms/hooks/useWatchPermissions.ts` around lines 30
- 39, The catch blocks in useWatchPermissions currently write to
permissionsStore (cameraPermission/microphonePermission) even if the hook has
been torn down; add a cancellation guard (e.g., an isActive/isMounted boolean or
AbortController flag stored in the hook scope) that is set false in the
useEffect cleanup and check that flag before mutating permissionsStore in both
catch paths (and after any awaited navigator.permissions.query calls); ensure
the cleanup toggles the flag so no writes occur after unmount.
Comment on lines
74
to
116
| let updatedCameraState: PermissionState | null = null | ||
| let updatedMicrophoneState: PermissionState | null = null | ||
|
|
||
| try { | ||
| const updatedCamera = await navigator.permissions.query({ | ||
| name: 'camera' as PermissionName, | ||
| }) | ||
| updatedCameraState = updatedCamera.state | ||
| } catch { | ||
| // Permission query not supported, keep current state | ||
| } | ||
|
|
||
| if (isCancelled) return | ||
| try { | ||
| const updatedMicrophone = await navigator.permissions.query({ | ||
| name: 'microphone' as PermissionName, | ||
| }) | ||
| updatedMicrophoneState = updatedMicrophone.state | ||
| } catch { | ||
| // Permission query not supported, keep current state | ||
| } | ||
|
|
||
| const cameraChanged = | ||
| permissionsStore.cameraPermission !== updatedCamera.state | ||
| const microphoneChanged = | ||
| permissionsStore.microphonePermission !== | ||
| updatedMicrophone.state | ||
| if (isCancelled) return | ||
|
|
||
| if (cameraChanged) { | ||
| permissionsStore.cameraPermission = updatedCamera.state | ||
| if ( | ||
| updatedCameraState && | ||
| permissionsStore.cameraPermission !== updatedCameraState | ||
| ) { | ||
| permissionsStore.cameraPermission = updatedCameraState | ||
| } | ||
|
|
||
| if (microphoneChanged) { | ||
| permissionsStore.microphonePermission = | ||
| updatedMicrophone.state | ||
| if ( | ||
| updatedMicrophoneState && | ||
| permissionsStore.microphonePermission !== | ||
| updatedMicrophoneState | ||
| ) { | ||
| permissionsStore.microphonePermission = updatedMicrophoneState | ||
| } | ||
|
|
||
| // Stop polling when both permissions are resolved | ||
| if ( | ||
| updatedCamera.state !== 'prompt' && | ||
| updatedMicrophone.state !== 'prompt' | ||
| (updatedCameraState ?? 'prompt') !== 'prompt' && | ||
| (updatedMicrophoneState ?? 'prompt') !== 'prompt' | ||
| ) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "useWatchPermissions.ts" 2>/dev/nullRepository: suitenumerique/meet
Length of output: 125
🏁 Script executed:
cat -n ./src/frontend/src/features/rooms/hooks/useWatchPermissions.tsRepository: suitenumerique/meet
Length of output: 8538
Prevent unbounded polling when permission queries consistently fail.
On lines 114-115, null is coerced to 'prompt'. If navigator.permissions.query() keeps throwing (lines 78-84, 87-93), that side never resolves and the 500ms interval runs indefinitely for the session.
Proposed fix
- // Stop polling when both permissions are resolved
- if (
- (updatedCameraState ?? 'prompt') !== 'prompt' &&
- (updatedMicrophoneState ?? 'prompt') !== 'prompt'
- ) {
+ // Stop polling when both permissions are resolved.
+ // If a query is unsupported (no PermissionStatus object), treat it as non-pollable.
+ const cameraStateForStop =
+ updatedCameraState ?? cameraPermission?.state ?? 'unavailable'
+ const microphoneStateForStop =
+ updatedMicrophoneState ??
+ microphonePermission?.state ??
+ 'unavailable'
+
+ if (
+ cameraStateForStop !== 'prompt' &&
+ microphoneStateForStop !== 'prompt'
+ ) {
if (intervalId) {
clearInterval(intervalId)
intervalId = undefined
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/features/rooms/hooks/useWatchPermissions.ts` around lines 74
- 116, In useWatchPermissions, the polling loop can run forever if
navigator.permissions.query() always throws (updatedCameraState and
updatedMicrophoneState remain null); update the stop condition so the poll also
stops when both permission queries are unsupported (both states are null).
Concretely, inside the polling completion logic (the block using
updatedCameraState/updatedMicrophoneState, permissionsStore and the "Stop
polling when both permissions are resolved" check), add a branch that treats
{updatedCameraState === null && updatedMicrophoneState === null} as a terminal
condition and stop the interval / return early (preserving existing updates to
permissionsStore when one or the other is non-null). Ensure you reference
useWatchPermissions, updatedCameraState, updatedMicrophoneState,
navigator.permissions.query, permissionsStore and isCancelled when making the
change.
Fix camera/microphone permissions not being detected on Android Firefox and iOS Chrome. Extend the polling workaround beyond Safari to all browsers with unreliable permission change events. Add individual error handling for navigator.permissions.query(). Closes suitenumerique#775 suitenumerique#769
mmaudet
force-pushed
the
fix/775-769-mobile-permissions-detection
branch
from
99ecbfe to
a7723d0
Compare
…rmissions - Add isCancelled guard in catch blocks to prevent store mutations after unmount - Fix polling stop condition to handle unsupported permission queries (null state)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isSafari()check), but iOS Chrome uses WebKit (same limitations) and Android Firefox has unreliable permission change events toohasUnreliablePermissionsEvents()utility that detects Safari, all iOS browsers (WebKit-based), and Firefoxnavigator.permissions.query()calls, which may throw on some mobile browsershandleMicrophoneChangethat checkedmicrophonePermission.stateinstead ofcameraPermission.statewhen deciding whether to stop pollingRoot cause: On iOS, all browsers use WebKit (Apple's policy), so iOS Chrome has the same Permissions API limitations as Safari. Android Firefox also doesn't reliably fire permission change events. The polling mechanism that works around these limitations was only activated for Safari.
Closes #775
Closes #769
Test plan