-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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(http): queue jsonp <script> tag onLoad event handler in microtask #39512
fix(http): queue jsonp <script> tag onLoad event handler in microtask #39512
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just like I commented in #39496, I think we can use a promise.then
here to delay the execution.
Also could you add a test case to verify this fix?
@JiaLiPassion thank you for your comments! |
Before this change, when trying to load a JSONP script that calls the JSONP callback inside a microtask, it will fail in Internet Explorer 11 and EdgeHTML. This commit changes the onLoad cleanup to be queued after the loaded endpoint executed any potential microtask itself. This ensures that the aforementioned browsers will first evaluate the loaded script calling the JSONP callback and only then run the cleanup inside onLoad. Fixes #39496
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for the research and the fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewed-for: global-approvers
…#39512) Before this change, when trying to load a JSONP script that calls the JSONP callback inside a microtask, it will fail in Internet Explorer 11 and EdgeHTML. This commit changes the onLoad cleanup to be queued after the loaded endpoint executed any potential microtask itself. This ensures that the aforementioned browsers will first evaluate the loaded script calling the JSONP callback and only then run the cleanup inside onLoad. Fixes #39496 PR Close #39512
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes #39496
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #39496
Before this change, when trying to load a JSONP script that calls the JSONP callback inside a setTimeout function, it will fail in Internet Explorer 11 and EdgeHTML.
What is the new behavior?
This commit changes the event handler on the script tag to be queued with setTimeout. This ensures that the aforementioned browsers will first evaluate the loaded script calling the JSONP callback and only then run the load handler which does the tear down of the callback function and script tag.
Does this PR introduce a breaking change?