Closed phet-steele closed 5 years ago
I tested the area-model-algebra link on Chrome 65.0.3325.181 (64-bit) and had no issues, then upgraded to 66.0.3359.117, and also had no problem. I then tried it in an incognito window on the upgraded version and also had no problem. I tested sound in the incognito one and it worked fine. So, in short, no problems seen. I'll read up on the issue and see what I can learn.
It looks like this behavior is something that is being implemented in order to prevent audio from playing on a page without user interaction. We already have a workaround in place for a similar policy that we ran into a while back for mobile Safari. It was a bit tricky to implement because Scenery batches events (at least it used to, and I THINK it still does), which means the browser doesn't see them as directly resulting from user actions.
If this problem needs to be resolved quickly, we could try modifying the workaround to create the audio context on the first user interaction. This would require testing on all platforms. I'll let @ariel-phet work with @phet-steele and others to decide if this should be a high priority for me.
because Scenery batches events (at least it used to, and I THINK it still does)
We don't batch anymore, since Scenery 2.0 switched to waiting to update the display until updateDisplay is called. This means that a response to input can happen inline (to the best of my knowledge).
This thread appears to be on the same topic: https://bugs.chromium.org/p/chromium/issues/detail?id=807017
If I understand correctly, this was a warning in chrome 65 then turned to an error in the recent chrome 66 deploy.
Thanks @jonathanolson - that may simplify things. Is there a way to add a listener that basically says "Call this function the first time the user does any interaction with the sim (including keyboard interaction)"? That, I think, would be the ideal time to create the AudioContext.
Is there a way to add a listener that basically says "Call this function the first time the user does any interaction with the sim (including keyboard interaction)"?
For sims, it would be possible to add a one-off input listener to the Display. Does it need to be a specific type of event (like a down), or could it even be a mousemove?
Does it need to be a specific type of event (like a down), or could it even be a mousemove?
The goal would be to listen for an event that is sure to happen right away on any of our targeted platform so that the audio context would get created as soon as interaction started. This would need to be true for mouse events, touch events, and keyboard interaction. @jonathanolson - what would you recommend for this? If necessary, we could also set up multiple listeners and just disconnect them all once the audio context got created.
Does it need to be a specific type of event (like a down), or could it even be a mousemove?
The goal would be to listen for an event that is sure to happen right away on any of our targeted platform so that the audio context would get created as soon as interaction started. This would need to be true for mouse events, touch events, and keyboard interaction.
I was skeptical of this, since in the past browsers have restricted things to specific types of events or patterns. I looked into it, and:
keydown
listener + listening for a Scenery down
event. This will probably need testing to verify.Thanks @jonathanolson - looks like you've done some good research here. I'll work with @phet-steele to get a platform on which this can be easily duplicated and will experiment with different solutions.
Note to self: Whatever we come up with for fixing Sound.js in vibe will need to be propagated into tambo.
I'm seeing this warning 100% of the time that I start Equality Explorer in requirejs mode. Chrome 66.0.3359.139 on macOS 10.11.6.
Sound.js?bust=1525639065867:27 The AudioContext was not allowed to start. It must be resume (or created) after a user gesture on the page. https://goo.gl/7K7WLu
Resolving this issue is (probably) a pre-requisite to publishing Equality Explorer. See https://github.com/phetsims/equality-explorer/issues/98.
Bumping the priority on this, will investigate options shortly.
I learned some things while investigating this problem for Equality Explorer, see https://github.com/phetsims/equality-explorer/issues/98#issuecomment-388969839.
Noticed this icon in the upper-right in Chrome:
Looks like it's full-on auto-muting my local server.
I'm not sure what to respond to (for the developer meeting). Why is this tagged?
@jonathanolson - I haven't seen the dialog you show above. Can you provide more details on platform, OS version, and Chrome version where you saw this?
Happened on my MBP (macOS 10.13.4, Chrome Version 66.0.3359.139 (Official Build) (64-bit)).
Since I needed to hear sound for something related to RewardNode, I hit allow and now I don't get that icon showing up. Probably could figure out how to reproduce if needed.
When running http://localhost:8080/phet-io-wrapper-sonification/ohms-law/html/ohms-law-sonification.html?sonificationFile=sonificationOptions¤tSound=recordedLoop3&voltageSound=clicks&resistanceSound=clicks&accessibility I am able to get the dialog to appear consistently. I also see this in the console:
Platform details: Was running on Version 66.0.3359.139 (Official Build) (64-bit), Windows 10
We (@oliver-phet) will test this on Chrome 67 on Arnab's dev machine and go from there.
@jbphet and I just re-tested on Arnab's machine and it seems an automatic update to Chrome fixed the issue. We were unable to reproduce the bug (with incognito and without) in Version 66.0.3359.181.
Here is a link to an article about Google rolling back the auto play muting, but they plan to reintroduce it later. In case the link goes away, below are some excerpts:
Google Chrome team rolls back the update that muted many web games
Automating muting for Web Audio has been delayed until Chrome 70 in October.
With the release of Chrome 66 last month, the browser began automatically muting sound on videos by default. The only problem is that this tweak silenced a number of web games and art projects, which weren't built for the new feature. Now the Chrome team has rolled out a new v66 (66.0.3359.181 on all desktop platforms) that undoes the change as it applies to the Web Audio API specifically -- but not for the
audio
andvideo
HTML tags. The reprieve is temporary, however, as it's planning to re-apply the feature in Chrome 70 which is scheduled for release in October.A manager from Google said " [I]n this case we didn't do a good job of communicating the impact of the new autoplay policy to developers using the Web Audio API."
Since this is likely to come back, I should implement code that will make it a non-issue when it does.
I am now able to duplicate this problem reliably on Chrome 66.0.3359.170 (Official Build) (64-bit) on my Windows 10 machine by setting the Autoplay policy
flag (which can be found at chrome://flags/#autoplay-policy) to "Document user activation is required" and using the sim link for which this problem was originally reported.
I was able to duplicate this on a RequireJS version by making sure my Autoplay policy
was set as described in the preceding comment, opening a new incognito window, and running area-model-algebra. This is a great relief, because this has been a bear to duplicate. I then set a breakpoint and looked at the value of audioContext.state
and could see that is was "suspended". I manually invoked "audioContext.resume()" and then was able to proceed from the breakpoint.
There is a bit of a fly in the ointment here, and it relates to https://github.com/phetsims/chipper/issues/551: The way to start the context is to call audioContext.resume()
, but this is an asynchronous call that returns a Promise, and thus it assumes ES6. WebAudio isn't supported in IE11, and we could probably code around that, but it would definitely require an ES6 construct to handle this situation. The workaround code would probably look something like this:
if ( audioContext.state !== 'running' ){
audioContext.resume().then( function(){
self.play();
} );
return;
}
The .then
part is ES6. If we don't migrate to ES6 before this fix is needed, we could just comment the heck out of it and make sure it is never executed on IE11, and this will likely work.
Labeling for dev meeting to get input on prioritization and plan.
I want to be clear about something: My understanding is that if Chrome does indeed move forward with this change, it will break audio on all deployed PhET simulations that use audio.
The .then part is ES6. If we don't migrate to ES6 before this fix is needed, we could just comment the heck out of it and make sure it is never executed on IE11, and this will likely work.
My understanding is that you aren't creating a promise yourself (new Promise(...)
) and aren't using any ES6+ features, so it's fine. We should be able to use APIs that return promises.
Once a fix is in place, we will want to make sure we test it out on versions of Chrome where the autoplay is disabled by default. I just looked up Chrome's release schedule, here is a screenshot:
Based on this, I'll put a recurrent reminder on my calendar to start checking Chrome Canary on July 24 2018 to make sure our fix works well on version 70.
I am experimenting with resuming the audio context on the first user gesture. I'm going to document what I see pretty thoroughly, since I suspect that Chrome's autoPlay policy may evolve, and it's hard to get old versions of the code, so I want to have good records. For the tests described below, I was using Chrome Version 66.0.3359.170 (Official Build) (64-bit) on Windows 10 in a freshly created incognito window.
In this test, I added scenery\display\Display.js
to Sound.js
, and then added the following code, which is executed during RequireJS loading:
if (audioContext){
if ( audioContext.state !== 'running' ){
console.log( 'audio context was not running' );
console.log( 'audioContext.state = ' + audioContext.state );
Display.userGestureEmitter.addListener( function resumeAudioContext(){
console.log( 'user gesture emitter fired, attempting to resume audio context' );
audioContext.resume().then( function(){
console.log( 'the resume promise has resolved' );
console.log( 'audioContext.state = ' + audioContext.state );
} );
Display.userGestureEmitter.removeListener( resumeAudioContext ); // only do this once
} );
}
}
When I loaded the sim, I saw what I expected on the Chrome console, which was this:
I then clicked on the "Game" screen selector, and got the output we were hoping for on the console, so that it now looked like this:
I played with the game and, sure enough, sound worked.
I ran a similar test to the one described in the preceding comment but used the John Travoltage sim instead of Area Model Algebra. When using a mouse, the audio context was resumed on the first mouse down event, so sound was generated during discharge (that's the only time that the current master version of the sim generates sound).
However (and as suspected), if I load the sim, then use the touch screen on my laptop to move the leg back and forth until discharge occurs without doing any other using interaction, the audio context is not resumed and no sound is played upon discharge. I believe that this is not something that needs to be addressed now, due to the reasoning described in this comment.
I tested the code snippet described in https://github.com/phetsims/vibe/issues/32#issuecomment-391890629 on Firefox and didn't quite get the results I expected. It turns out that in Firefox, the audio context is always initially in the "suspended" state and then automatically transitions to the "running" state later. This behavior was causing the listener to be attached, and that listener would in turn call resume()
on an audio context that was already running. This didn't seem to cause any bad behavior, but it doesn't seem quite correct, and it could be misleading to future developers. Instead, I'm going to try always hooking up the gesture listener (when an audio context exists of course) and checking the audio context's state on the first user gesture, and then only calling resume()
if audioContext.state
indicates that it is needed.
Below is another shot at the code to resume the audio context. It is in Sound.js just before the constructor is defined. This includes a bunch of console.log statements so that I can clearly see the order of events.
if ( audioContext ) {
console.log( 'loading Sound.js, audioContext.state = ' + audioContext.state );
if ( audioContext.state !== 'running' ) {
console.log( 'audio context was not running at load time, hooking up listener to resume it later' );
Display.userGestureEmitter.addListener( function resumeAudioContext() {
console.log( 'user gesture emitter fired, audio context state = ' + audioContext.state );
if ( audioContext.state !== 'running' ) {
console.log( 'calling resume() on audio context' );
// the audio context isn't running, so tell it to resume
audioContext.resume().then( function() {
console.log( 'the resume promise has resolved, audioContext.state = ' + audioContext.state );
} );
}
else {
console.log( 'audio context was running, no need to call resume' );
}
Display.userGestureEmitter.removeListener( resumeAudioContext ); // only do this once
} );
}
else{
console.log( 'audio context running at load time, no further action is necessary' );
}
}
Using this code and testing on area-model-algebra and john-travoltage, the sound worked on all platforms, and I found that:
resume()
is called, and the audio context transitions to the "running" state quickly.resume()
didn't need to be called.The story for IOS is a little more complicated. There is already some special-case code in Sound.js
for IOS, and my hope was that the new code would supplant the old, but unfortunately it doesn't work in all cases. The new code works fine on IOS for multi-screen simulations where the user has to select a screen. However, it fails on a single screen sim if the user just touches and drags without doing a tap of any kind. The prime example is John Travoltage - if I remove the old IOS-specific code and have just the new code in place, then run JT and just drag the leg back and forth until a discharge occurs, no sound is emitted. This can be repeated multiple times, and the arm can be dragged as well, no sound will be produced. If I the tap the screen anywhere, then the sound will get activated. I suspect that this is because the IOS developers want to make sure that the user has engaged the web page in some way other than scrolling, so those events don't count as 'user events', but this is speculation on my part.
Because of this, I plan to leave the old special-case code for IOS in place, at least for now. For the record, this testing was done on the PhET machine named Liebniz, IOS version 9.3.5 + Mobile Safari.
Assigning to @jonathanolson to review. I tried to do this in a single commit to make maintenance releases easier, and also used a new Scenery feature, so @jonathanolson is probably the best choice.
@jonathanolson - the only thing I'm wondering about is the IOS situation where no user gesture is being received when dragging on a touch screen, such as moving John Travoltage's leg. My guess is that this is unavoidable, since they don't want dragging a web page to count as "interaction", but I thought I'd confirm. Read the 2nd to last paragraph of the preceding comment if you want to understand why I'm asking.
Once reviewed, please assign back to me.
Review looks good. I'm not sure why the gainNode bit was removed in the commit, can you comment on that?
I'm also curious - why not do both workarounds for mobileSafari (i.e. do the silent sound in addition to the normal workaround)?
I feel like I'll be able to handle the commit in whatever way it needs to get rewritten depending on the age of the sim (hopefully all sims that need the patch will have platform.mobileSafari defined). Presumably I should be able to test each variation with the Chrome flag and testing with incognito.
@ariel-phet and @jbphet, do you have a sense of when the maintenance release will need to be done by? I'd hope to do this release first (for testing) since it seems more important than the open-graph regression (https://github.com/phetsims/chipper/issues/677).
Responses to review comments:
I'm not sure why the gainNode bit was removed in the commit, can you comment on that?
It probably would have been better to do this in a separate commit - sorry to confuse things. I noticed that the gain node isn't being used anywhere, so it is unnecessary complexity, and I thought I might as well get rid of it.
I'm also curious - why not do both workarounds for mobileSafari (i.e. do the silent sound in addition to the normal workaround)?
When I tested on Mobile Safari, I found that the two workarounds have the exact same effect. More precisely, on Mobile Safari, the audio context is created when the sim loads but it is in the suspended state, which is exactly what the Chrome autoplay change does. Playing a sound as a result of a touchstart
event moves the context into the "running" state. So, doing both workarounds is redundant. As mentioned above, I tried using the new workaround for iOS, but it doesn't work as well, so I ended up with the either/or approach you saw in this commit.
[D]o you have a sense of when the maintenance release will need to be done by?
Google has said that they plan to reintroduce the autoplay prevention behavior in October, so if the maintenance release is done before then, we should be in good shape. I have a reminder to test this fix when the autoplay change appears in Chrome Canary, which is planned for July. If the maintenance release can wait until that testing is complete, we will have a bit more confidence that it works well.
Back to @jonathanolson for responses on my responses.
Ok, sounds good. I'm leaving myself assigned for the maintenance release.
Good maintenance release to perform in the near'ish future (ideally by the end of July)
New maintenance release code detects the following release branches:
[vibe] https://github.com/phetsims/vibe/issues/32
3bf8488a8801605887d34506255cfb862cc07f77
area-builder 1.1 phet
area-model-algebra 1.0 phet
area-model-decimals 1.0 phet
area-model-introduction 1.0 phet
area-model-multiplication 1.0 phet
arithmetic 1.0 phet
balancing-act 1.1 phet
balancing-chemical-equations 1.1 phet
balloons-and-static-electricity 1.2-phetio phet-io
balloons-and-static-electricity 1.3-phetio phet-io
build-an-atom 1.4-phetio phet-io
build-an-atom 1.5-phetio phet-io
build-an-atom 1.6 phet
expression-exchange 1.0-phetio phet-io
expression-exchange 1.1 phet
faradays-law 1.1 phet
faradays-law 1.3-phetio phet-io
forces-and-motion-basics 2.1-phetio phet-io
forces-and-motion-basics 2.3 phet
forces-and-motion-basics 2.3-phetio phet-io
fraction-matcher 1.1 phet
graphing-lines 1.2 phet
graphing-slope-intercept 1.0 phet
john-travoltage 1.3 phet
john-travoltage 1.4-phetio phet-io
make-a-ten 1.0 phet
ohms-law 1.3 phet
plinko-probability 1.1 phet
plinko-probability 1.1-phetio phet-io
reactants-products-and-leftovers 1.1 phet
The (very new) maintenance release process spit out:
I'm going to scan over these (make sure links are working, and some spot checks) before handing to QA.
@jbphet, what are the instructions I can give to QA for them to check if a sim is successfully patched?
Also whoops on the above list, I'll need to do additional chipper 2.0 detection to get the directories right.
Take two at the links:
Chrome Canary just updated to version 70. The full version string is:
70.0.3501.2 (Official Build) canary (64-bit)
I just tested the link that originally revealed this issue to us and got a warning that looks like the Chrome team has not re-enabled the blocking of autoplay yet, but plans to do so soon. Here's a screen capture:
And here is what the console output looks like when saved to a file:
area-model-algebra_all_phet.html:1062 The Web Audio autoplay policy will be re-enabled in Chrome 70 (October 2018). Please check that your website is compatible with it. https://goo.gl/7K7WLu
(anonymous) @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
l @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
l @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
l @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
l @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
l @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
l @ area-model-algebra_all_phet.html:1062
u @ area-model-algebra_all_phet.html:1062
(anonymous) @ area-model-algebra_all_phet.html:1062
setTimeout (async)
c @ area-model-algebra_all_phet.html:1062
(anonymous) @ area-model-algebra_all_phet.html:1062
(anonymous) @ area-model-algebra_all_phet.html:1062
Audio worked fine for the game on this sim, which supports the idea that the feature is threatened, but not yet implemented.
I also tested the 1.0.0 version of equality-explorer, which was published recently and so should have the workaround in place. The results were the same, i.e. the warning was printed but sound worked just fine.
I also added some debug code to print out the state of the audio context immediately after it is created and ran the RequireJS version of area-model-algebra, and the state is showing as running, further supporting the hypothesis.
It will be important that I track this and know exactly when the feature gets enabled so we can test it. I'll put monthly reminders on my calendar for it.
I looked around to see if there was a way to force Chrome 70.0.3501.2 to actually block the audio context, and I found something promising. It was to run Chrome with the flag --disable-features=AutoplayIgnoreWebAudio
and I saw it here: https://developers.google.com/web/updates/2017/09/autoplay-policy-changes#webaudio.
With this flag enabled, the error message changes, but audio still works, which is perplexing. Here is what the warning looks like:
I again added debug code and the state of the audio context is indeed showing up as suspended
, but sound still works. I get the sense that the Chrome team has some work to do to get this into a cohesive and comprehensible state.
@jonathanolson asked:
@jbphet, what are the instructions I can give to QA for them to check if a sim is successfully patched?
Unfortunately, with Chrome 70 not yet implementing the policy, there isn't a good way to test that the patch process was successful. I can't think of much else to do other than waiting until the policy is implemented and the behavior is reproducible before testing this. I've put monthly reminders on my calendar to check the state of it, and will update this issue with any news.
Ok, created the instructions in the above linked issue (https://github.com/phetsims/special-ops/issues/112)
Ack, it looks like the vibe patch also will depend on a Scenery patch for userGestureEmitter. I'll need to add those in.
I have added a monthly reminder on my calendar to test this so that we can determine when it gets turned on and test the maintenance release. In the mean time, I'm going to unassign myself.
Maintenance deploys for this completed. @jbphet anything else to do?
Thanks @jonathanolson. There's nothing more to do until the policy is actually implemented in either Chrome Canary or Chrome Beta, at which point I'll verify that this works correctly. I have reminders on my calendar to check this regularly.
Would chrome.exe --disable-features=AutoplayIgnoreWebAudio
allow you to test this before the new chrome is out? See https://developers.google.com/web/updates/2017/09/autoplay-policy-changes#webaudio
@samreid said:
Would
chrome.exe --disable-features=AutoplayIgnoreWebAudio
allow you to test this before the new chrome is out?
Good question, and my apologies for not documenting my prior experiences with this flag. The documentation would lead you to believe that this flag should make it possible to test the case where the audio context isn't enabled at startup, and it kind of does, but not entirely. When I run Chrome Canary version 70.0.3518.0 with this flag, the audio context is indeed not allowed to start, so it is in the suspended state immediately after it is created. I've set breakpoints in our workaround code and verified that it fires on a user interaction and makes the audio context transition from the suspended
state to the running
state. This is good. However, this version of Chrome apparently allows sound production from a suspended audio context, which seems dead wrong, but that's what it does. So, if I test the link for area-model-algebra that's shown in the original comment for this issue, it produces sound just fine. From the user's point of view, there is no difference. But if I use a RequireJS version, add log statements to the play()
method in Sound.js
to output the state of the audio context, and comment out the workaround, the log statements say that the audio context is in the suspended state. If I add a listener to the audio context for state changes, it never runs. Yet sound is still produced. Again, this seems like incorrect behavior, but that's what happens.
I'm not sure if this is a bug in Chrome Canary, or if they are not fully implementing the policy just yet, so I'm going to keep an eye on it as new versions come out. The fact that I can verify that the workaround code is updating the state of the audio context through the use of console.log and debugger statements gives me a pretty high degree of confidence that it will work once the autoplay policy is enacted, but I won't be 100% sure until sims without the workaround code don't produce sound and sims with it do.
A calendar reminder went off for me to test this, and Chrome Canary is now at version 70.0.3530.0. The behavior has not changed. To summarize, the behavior is as follows:
--disable-features=AutoplayIgnoreWebAudio
, the audio context is created in the "suspended" state, and the workaround code will kick in and set it to the "running" state.OneShotSoundClip.play()
, then cause a one shot sound to be played, I can see through the console that the audio context is in the "suspended" state, but playing the sound still works.Basically, no new news here. Will check again in a few weeks.
Another calendar reminder just went off, so I'm checking this again. Apparently, it is now possible to have three versions of Chrome installed on one machine (this may be new or I may have just missed it before), the "stable" channel (which is the main one that most people will have), the "beta" channel (the next version that will become live), and the "dev" channel (the version that will go live after the beta version). I'm going to run simple tests on all three. The goal here is to make sure that we don't end up in a situation where an update of Chrome comes out and our sims don't play sound.
The Chrome versions as of this writing are:
The version of area model that is linked in the first comment for this issue (https://bayes.colorado.edu/dev/html/area-model-algebra/1.0.0-rc.1/phet/area-model-algebra_all_phet.html) is known not to have the workaround for blocked autoplay. If I run that version on the current stable release, there is a warning message in the console that says:
The Web Audio autoplay policy will be re-enabled in Chrome 70 (October 2018)...
If I use that same link in the beta version, I see the message:
The Web Audio autoplay policy will be re-enabled in Chrome 71 (December 2018)...
This makes it look like they've moved back the implementation of this policy by a couple of months.
Launching that same URL in the dev version elicits the same error message about the policy going live in version 71, but sound still works. I tried launching the dev version with the following command sequence, which in theory should turn off autoplay:
cd C:\Program Files (x86)\Google\Chrome Dev\Application
chrome.exe --disable-features=PreloadMediaEngagementData,AutoplayIgnoreWebAudio,MediaEngagementBypassAutoplayPolicies
Still, sound plays just fine. I then tested the current RC release of the John Travoltage sim on the beta and dev versions (it has already been tested on the stable version) and sound was produced just fine. So, it seems like whatever they are doing, it isn't negatively affecting the ability of PhET sims to produce sounds, at least not yet. I'll keep checking until the policy is fully implemented.
When I load https://bayes.colorado.edu/dev/html/area-model-algebra/1.0.0-rc.1/phet/area-model-algebra_all_phet.html in Chrome, I get the following warning:
The link: https://goo.gl/7K7WLu If I see this warning, the audio in that sim does not work. If I don't see this warning, all is well.
I've only seen this on one Win 10 computer (out of three Win 10 Chromes and one macOS 10.13.4 Chrome), and I only see this on first load. Therefore, it is advised to open the sim in incognito while testing.
This was found while testing phetsims/QA/issues/113. @ariel-phet @jbphet please advise on how to proceed.