fix(windows): hide native desktop cursor during browser capture#79
fix(windows): hide native desktop cursor during browser capture#79LILQK wants to merge 2 commits intowebadderall:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
|
PR removes system-audio capture path for Windows browser capture, please fix |
![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/hooks/useScreenRecorder.ts`:
- Around line 577-583: The cursor:"never" constraint set in
displayMediaVideoConstraints gets lost when applyConstraints() is called; update
the applyConstraints() call on the videoTrack (in useScreenRecorder.ts) to
include cursor: "never" along with frameRate, width, and height (use the
existing TARGET_FRAME_RATE, TARGET_WIDTH, TARGET_HEIGHT constants) and cast to
MediaTrackConstraints so the cursor constraint persists.
- Around line 604-611: The catch block that handles audio capture errors in
useScreenRecorder (around the getDisplayMedia call producing screenMediaStream)
must not retry with audio: false when the user cancelled or denied the share
dialog; modify the catch for audioError to inspect audioError.name (or
error.code) and if it is "NotAllowedError" or "AbortError" (or clearly indicates
user denial/abort) then rethrow or return/exit instead of calling
navigator.mediaDevices.getDisplayMedia again; only perform the fallback to
video-only for other error names (non-user-denial/unrecoverable) so that user
cancellations don't trigger a second picker.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f4081559-ae2d-47e5-b1af-25ea30a3fcdb
📒 Files selected for processing (2)
-src/hooks/useScreenRecorder.ts
💤 Files with no reviewable changes (1)
| const displayMediaVideoConstraints = { | ||
| displaySurface: selectedSource.id?.startsWith("window:") ? "window" : "monitor", | ||
| width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH }, | ||
| height: { ideal: TARGET_HEIGHT, max: TARGET_HEIGHT }, | ||
| frameRate: { ideal: TARGET_FRAME_RATE, max: TARGET_FRAME_RATE }, | ||
| cursor: "never" as const, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/hooks && wc -l useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 88
🏁 Script executed:
cd src/hooks && sed -n '690,710p' useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 690
🏁 Script executed:
cd src/hooks && sed -n '577,600p' useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 1016
🏁 Script executed:
cd src/hooks && grep -n "applyConstraints" useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 111
🏁 Script executed:
cd src/hooks && sed -n '577,583p' useScreenRecorder.ts && echo "---" && sed -n '697,710p' useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 932
Include cursor: "never" in the applyConstraints() call at line 697.
The cursor: "never" constraint set during initialization (line 577) will be cleared when applyConstraints() is invoked because it replaces the constraint set and restores omitted properties to defaults. Reapply the constraint:
Code change needed
await videoTrack.applyConstraints({
frameRate: { ideal: TARGET_FRAME_RATE, max: TARGET_FRAME_RATE },
width: { ideal: TARGET_WIDTH, max: TARGET_WIDTH },
height: { ideal: TARGET_HEIGHT, max: TARGET_HEIGHT },
cursor: "never",
} as MediaTrackConstraints);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useScreenRecorder.ts` around lines 577 - 583, The cursor:"never"
constraint set in displayMediaVideoConstraints gets lost when applyConstraints()
is called; update the applyConstraints() call on the videoTrack (in
useScreenRecorder.ts) to include cursor: "never" along with frameRate, width,
and height (use the existing TARGET_FRAME_RATE, TARGET_WIDTH, TARGET_HEIGHT
constants) and cast to MediaTrackConstraints so the cursor constraint persists.
| } catch (audioError) { | ||
| console.warn("System audio capture failed, falling back to video-only:", audioError); | ||
| alert("System audio is not available for this source. Recording will continue without system audio."); | ||
| screenMediaStream = await (navigator.mediaDevices as any).getUserMedia({ | ||
| screenMediaStream = await navigator.mediaDevices.getDisplayMedia({ | ||
| audio: false, | ||
| video: browserScreenVideoConstraints, | ||
| video: displayMediaVideoConstraints, | ||
| ...displayMediaOptions, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and examine the code around lines 604-611
wc -l src/hooks/useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 98
🏁 Script executed:
# Read the relevant section with more context around lines 604-611
sed -n '580,630p' src/hooks/useScreenRecorder.tsRepository: webadderall/Recordly
Length of output: 1965
🌐 Web query:
MDN getDisplayMedia exceptions AbortError NotAllowedError
💡 Result:
For navigator.mediaDevices.getDisplayMedia(), MDN documents these two DOMException rejections:
NotAllowedError: The user denied the screen-share permission prompt, or the page isn’t allowed to use screen capture in the current context (for example, blocked by a Permissions Policy). [1]AbortError: A generic failure—used when the capture fails for a reason that doesn’t match the other, more specific exceptions. [1]
MDN also notes a closely related case that’s often confused with NotAllowedError:
InvalidStateError: Thrown ifgetDisplayMedia()wasn’t called from a transient user activation (e.g., not directly in a click handler), or if the document isn’t fully active/focused, etc. [1]
Typical handling pattern:
try {
const stream = await navigator.mediaDevices.getDisplayMedia({ video: true, audio: true });
} catch (e) {
if (e.name === "NotAllowedError") {
// user denied OR blocked by policy/context
} else if (e.name === "AbortError") {
// unexpected failure not covered by other exceptions
} else if (e.name === "InvalidStateError") {
// not triggered by a user gesture / not focused / etc.
}
}Sources: MDN getDisplayMedia() exceptions list and definitions. [1]
Don't reprompt after a cancelled or denied share dialog.
MDN confirms NotAllowedError (user denied) and AbortError (generic failure) are thrown by getDisplayMedia(). Line 604 currently retries with audio: false after every failure, causing user cancellations to trigger a second picker instead of aborting cleanly. Only downgrade to video-only for cases that don't indicate user denial or unrecoverable failures.
Suggested guard
} catch (audioError) {
+ if (
+ audioError instanceof DOMException &&
+ (audioError.name === "AbortError" || audioError.name === "NotAllowedError")
+ ) {
+ throw audioError;
+ }
console.warn("System audio capture failed, falling back to video-only:", audioError);
alert("System audio is not available for this source. Recording will continue without system audio.");
screenMediaStream = await navigator.mediaDevices.getDisplayMedia({
audio: false,
video: displayMediaVideoConstraints,
...displayMediaOptions,
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (audioError) { | |
| console.warn("System audio capture failed, falling back to video-only:", audioError); | |
| alert("System audio is not available for this source. Recording will continue without system audio."); | |
| screenMediaStream = await (navigator.mediaDevices as any).getUserMedia({ | |
| screenMediaStream = await navigator.mediaDevices.getDisplayMedia({ | |
| audio: false, | |
| video: browserScreenVideoConstraints, | |
| video: displayMediaVideoConstraints, | |
| ...displayMediaOptions, | |
| }); | |
| } catch (audioError) { | |
| if ( | |
| audioError instanceof DOMException && | |
| (audioError.name === "AbortError" || audioError.name === "NotAllowedError") | |
| ) { | |
| throw audioError; | |
| } | |
| console.warn("System audio capture failed, falling back to video-only:", audioError); | |
| alert("System audio is not available for this source. Recording will continue without system audio."); | |
| screenMediaStream = await navigator.mediaDevices.getDisplayMedia({ | |
| audio: false, | |
| video: displayMediaVideoConstraints, | |
| ...displayMediaOptions, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useScreenRecorder.ts` around lines 604 - 611, The catch block that
handles audio capture errors in useScreenRecorder (around the getDisplayMedia
call producing screenMediaStream) must not retry with audio: false when the user
cancelled or denied the share dialog; modify the catch for audioError to inspect
audioError.name (or error.code) and if it is "NotAllowedError" or "AbortError"
(or clearly indicates user denial/abort) then rethrow or return/exit instead of
calling navigator.mediaDevices.getDisplayMedia again; only perform the fallback
to video-only for other error names (non-user-denial/unrecoverable) so that user
cancellations don't trigger a second picker.
2 participants
Summary
getDisplayMediaconstraints withcursor: "never"Why
On Windows, recordings could show the OS cursor again, which caused a duplicate cursor when Recordly renders its own cursor layer. This brings back the previous expected behavior where only Recordly's cursor rendering is visible.
Notes
src/hooks/useScreenRecorder.tsis includedAGENTS.mdis intentionally not part of this PRSummary by CodeRabbit
New Features
Refactor