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

Should we "check if we can run script" before calling callbacks? #993

Open
domenic opened this issue Jun 24, 2021 · 2 comments
Open

Should we "check if we can run script" before calling callbacks? #993

domenic opened this issue Jun 24, 2021 · 2 comments

Comments

@domenic
Copy link
Member

domenic commented Jun 24, 2021

Most places in HTML that use prepare to run script call check if we can run script beforehand, and bail out if you can't:

However, Web IDL notably does not! See:

This is important because the first step in "check if we can run the script" is to return false if the given realm (in this case, the realm of the callback or of the object) comes from a non-fully-active document, e.g. a detached iframe or a bfcached page.

Chrome seems to include such a "check if we can run script" check for Web IDL callbacks: see https://jsfiddle.net/shaseley/w4nsq2oj/7/ which runs the task in Firefox and Safari but does not run the task in Chrome. Discovered by @shaseley @natechapin.

Worse, https://jsfiddle.net/n04m3jpo/ shows a variant with promise handlers (instead of setTimeout's Web IDL callbacks) where Chrome and Safari do run the promise handler, and Firefox does not.

(On the spec side, I think we cargo-culted the "check if we can run script" check into HostEnqueueFinalizationRegistryCleanupJob and HostEnqueuePromiseJob without testing, because I thought it was about the user disabling scripting, and I forgot it was also about non-fully-active documents.)

My instinct is that we should be uniform here and not run the handler in any cases. /cc @smaug---- @rakina for bfcache connection, @syg for promise/finalization registry connection.

Related: whatwg/html#2621

@syg
Copy link
Contributor

syg commented Jun 24, 2021

Worse, https://jsfiddle.net/n04m3jpo/ shows a variant with promise handlers (instead of setTimeout's Web IDL callbacks) where Chrome and Safari do run the promise handler, and Firefox does not.

That both Chrome and Safari run the promise handlers here give me pause that changing the behavior might not be web compatible. What's your intuition?

@domenic
Copy link
Member Author

domenic commented Jun 25, 2021

I agree it's not clear-cut. It's a combination of:

  • Firefox gets away with it.

  • Chrome and Safari don't run Web IDL handlers, which are potentially more common.

  • Anything involving detached frames is a mess and unlikely to be something web developers can rely on. E.g. inside your callback the properties of the global object will mostly be nulled out or throw exceptions, in inconsistent ways across browsers.

I think the main risk would be if there's a site which is trying to do "JS-spec-only" things (e.g. Node.js-isomorphic code) on detached iframes, and using promises to schedule things there. But such a site wouldn't work in Firefox, so I'm hoping such cases are rare...

Separately from my intuition on whether this is compatible, I also think it's desirable, as part of the ongoing efforts of folks like @xharaken and his team to make detached iframes as GC-able as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants