-
Notifications
You must be signed in to change notification settings - Fork 2
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
PHeT simulations throwing uncaught JS errors #1002
Comments
Thanks for reporting the issue @SteelWagstaff. I see the errors in the link you provided, as well as in the following simple HTML: <!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>Hidden Sim test</title>
</head>
<body>
<div hidden id="container">
<iframe src="https://phet.colorado.edu/sims/html/masses-and-springs-basics/latest/masses-and-springs-basics_en.html" id="iframe" allowfullscreen scrolling="no" title="Interact with {{PHET_SIM_TITLE}}"></iframe>
</div>
<button id="testButton">Toggle Hidden</button>
</body>
<script type="text/javascript">
const container = document.getElementById( 'container' );
const button = document.getElementById( 'testButton' );
button.addEventListener( 'click', ( event ) => {
if ( container.hidden ) {
container.removeAttribute( 'hidden' );
}
else {
container.hidden = true;
}
} );
</script>
</html> When assertions are enabled, we first hit
|
I'm able to reproduce the problem on the masses and springs version referenced above, but not in a freshly built Faraday's law. |
Assigning to @ariel-phet and @oliver-phet, can you please review this issue? |
@SteelWagstaff when I try the provided link, everything seems to work? Did @connerbw make a fix, or do we need to investigate further? Also does Pressbooks currently have a more formal relationship with our project? If not, could you give us the contact info of the correct person to discuss partnerships with? |
We worked on a hack:
Did you look at the network console? I can still reproduce. The bug is still there. |
@jonathanolson is going to spend about an hour seeing if he can identify what is going on here. |
The root cause is that when running in an invisible iframe, Chrome is showing window.innerWidth and window.innerHeight as both 0. We have some graceful fallbacks, but PhetButton's onResize is missing the fallbacks and hits a hard error. This is easy enough to patch, however there's a more fundamental tricky issue to resolve when our iframe is hidden: our text bounds detection method fails, so our sim handles layout with all text acting as if it takes zero height: I've traced that back to the SVG-based method we use to get the height of a font, which uses getBBox(). It always returns a 0-size rectangle when the iframe we're running in is hidden (and workarounds like https://stackoverflow.com/questions/28282295/getbbox-of-svg-when-hidden of "put the SVG content somewhere not hidden" is unavailable if everything under our html element is hidden). We need correct values for this at sim startup, because (a) we cache the font heights for performance, and (b) many times we lay things out statically based on the text height. We can definitely detect when we're in this situation, but I don't see any great options of what to do. It would be possible to compute font/text heights using another method, but the values would be slightly different, and so the sim would look somewhat "different" in layout depending on whether it loaded in a hidden iframe or not (less than ideal). The best solution I can think of on our end would be to delay sim startup until we have a well-defined width/height (and the SVG text method works). This means the sim wouldn't load in the background when hidden, but would trigger loading when it was shown. On our end, we could detect the SVG issue and delay startup if that is the case (and listen for a resize event and recheck and startup then). Alternatively, having the sims start up in a way where they are not marked as "hidden" is the best workaround for currently-published simulations that I'm aware of. @ariel-phet, thoughts? |
@jonathanolson what is wrong with delaying sim startup? That seems totally reasonable to me, you open the frame, and the sim "boots" as it were. Seems like acceptable behavior. If we delayed start-up, it sounds like that solves the various issues? Should we also make a separate issue for the PhETButton Resize fallbacks? |
This actually looks like it was resolved in master since we branched masses-and-springs-basics.
Yes, it seems like the best option to me. It will likely require some changes in chipper to our HTML and loading process (we can't use the main image/phet-io loading delay section, as code could already have executed buggily). This seems generally useful to me. @ariel-phet should I proceed with changes to implement that? |
@jonathanolson yes please proceed. Can you document progress in a separate chipper issue and tag this one? |
Users of our Pressbooks authoring tool have reported some issues with embedded PhET simulations when placed within a "collapsible subsection." You can see an example at https://phetcollapse.textopress.com/chapter/chapter-1/
If you view the JavaScript console in the example book you'll see JavaScript prints two fatal errors (one for each hidden PhET simulation).
It appears that these PHeT simulations are attempting to run an invisible/hidden/{non-existent until expanded} property, which fails. Not sure if this is the right place to report this issue, so if it needs to be moved or placed elsewhere, please let us know.
The text was updated successfully, but these errors were encountered: