Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix safari not capturing both display and camera #1127

Merged

Conversation

owi92
Copy link
Member

@owi92 owi92 commented Sep 27, 2023

Safari needs a "user gesture" (i.e. a button click) to call getDisplayMedia. Calling the user capture first would prevent the display capture to register the triggering button click, so this switches the ordering (for safari only).
So in Safari, the user will always get asked to share their screen before they get asked to share their webcam.

(I did notice that when calling both startUserCapture and startDisplayCapture inside a Promise.all(), the ordering in Safari is consistently switched again, but with no issues (meaning user capture is asked before screen capture). This would be preferrable, but I'm not sure if we can rely on this always being the case. So in the interest of consistency I chose to just call startDisplayCapture first...)

Closes #934

Unless `unexpeted` is a real word.
@owi92 owi92 changed the title Fix safari capturing display and camera Fix safari not capturing both display and camera Sep 27, 2023
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR having two commits is certainly unexpeted.

Thanks! Just two tiny things.

src/steps/video-setup/source-select.tsx Outdated Show resolved Hide resolved
src/steps/video-setup/source-select.tsx Outdated Show resolved Hide resolved
Safari needs a "user gesture" (i.e. a button click)
to call `getDisplayMedia`. Calling the user capture
first would prevent the display capture to register
the triggering button click, so this switches the
ordering (for safari only).
Closes elan-ev#934.
@owi92 owi92 force-pushed the fix-safari-capturing-display-and-camera branch from 46f9f28 to 2c6dd38 Compare September 27, 2023 12:31
@lkiesow
Copy link
Contributor

lkiesow commented Sep 27, 2023

I'm wondering if the code for Safari wouldn't actually work on in all major vrowsers.
Do we really need two code branches here?

@LukasKalbertodt
Copy link
Member

It's complicated. The current order was specifically chosen in commit 66865d3

Request webcam before display share

If sharing a specific window from the desktop, some browsers bring that window into the foreground. If it does not overlap the browser window, users might be confused as the webcam is not requested. This only happens after the browser window is focused again. Changing the order works around this problem.

Once we fix #403 is fixed, we might be able to remove the extra branch here. The branch is annoying but we didn't want to risk maybe making the workflow for some people worse. Or well, just changing anything might annoy people.

@LukasKalbertodt LukasKalbertodt merged commit d4c1507 into elan-ev:master Sep 28, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Capturing "Display & Camera" does not work in Safari (macOS)
3 participants