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

allow audio to play even when not initiated with a user's gesture #179204

Merged
merged 10 commits into from
Apr 5, 2023

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Apr 4, 2023

Under discussion currently, as this will impact all media with autoplay

When a user gesture doesn't initiate a sound, it won't play. You can bypass this with a chromium arg.

fix #176284
fix #178642

fyi @rebornix @amunger

@meganrogge meganrogge requested a review from bpasero April 4, 2023 21:57
@meganrogge meganrogge self-assigned this Apr 4, 2023
@meganrogge meganrogge added this to the April 2023 milestone Apr 4, 2023
@mjbvz
Copy link
Collaborator

mjbvz commented Apr 4, 2023

@meganrogge Let's make sure this doesn't enable auto playing videos with audio on extension pages. You can use edit one the built-in extension's README.md to have a video tag such as:

<video autoplay>
<source src="http://127.0.0.1:8080/example.mp4">
</video>

I can help test this too if you're running into issues

@deepak1556
Copy link
Collaborator

The flag will affect all media tags across frames which have an autoplay attribute.

src/main.js Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
bpasero
bpasero previously requested changes Apr 5, 2023
src/main.js Outdated Show resolved Hide resolved
@meganrogge meganrogge requested a review from bpasero April 5, 2023 13:40
src/main.js Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
Co-authored-by: Robo <hop2deep@gmail.com>
Copy link
Collaborator

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM on the change, but I would leave the decision to @mjbvz on whether to enable this by default for all users.

src/main.js Outdated
Comment on lines 255 to 258
/**
* Allow media to play even if no user gesture is used to fix issues like #176284
*/
app.commandLine.appendSwitch('autoplay-policy', 'no-user-gesture-required');
Copy link
Member

Choose a reason for hiding this comment

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

If this is restricted to just the renderer (not webviews) then it's not a big deal imo, it will not work in web of course though.

@deepak1556
Copy link
Collaborator

Looking a bit more into the history of the autoplay policy in Chromium, it used to be that desktop allowed autoplay by default until the following feature landed in 2017 https://bugs.chromium.org/p/chromium/issues/detail?id=715049 to avoid annoyances to users from Ads on both mobile and desktop. The design doc also highlights the workarounds a website can do to get autoplay, checkout the Media Engagement section of the linked doc.

Based on the above I feel we can just disable the UnifiedAutoplay feature to get the default behavior of autoplay. @meganrogge can you replace the current change with change to L253 app.commandLine.appendSwitch('disable-features', 'CalculateNativeWinOcclusion,UnifiedAutoplay'); and see if it still addresses the issue.

@meganrogge
Copy link
Contributor Author

Thanks for investigating @deepak1556 and yes that fixes it 👍🏼

@meganrogge meganrogge enabled auto-merge April 5, 2023 15:05
@meganrogge meganrogge merged commit 40e27f4 into main Apr 5, 2023
@meganrogge meganrogge deleted the merogge/auto-play-bypass branch April 5, 2023 15:15
/**
* Allow media to play even if no user gesture is used to fix issues like #176284
*/
app.commandLine.appendSwitch('disable-features', 'CalculateNativeWinOcclusion,UnifiedAutoplay');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I must have misread the diff, can you de-duplicate L253 and this one. We only need single app.commandLine.appendSwitch('disable-features'

@github-actions github-actions bot locked and limited conversation to collaborators May 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants