Skip to content

Manipulation: Always make _evalUrl requests as dataType: script #4258

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.

Sign up for GitHub

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

Jump to bottom
Merged
gibson042 merged 2 commits into from
Dec 13, 2018
Merged

Manipulation: Always make _evalUrl requests as dataType: script #4258

gibson042 merged 2 commits into from
Dec 13, 2018

Conversation

@gibson042
Copy link
Member

@gibson042 gibson042 commented Dec 13, 2018

IE and iOS <10 XHR transport does not succeed on data: URIs
Ref gh-4243
Ref gh-4126

@gibson042
Manipulation: Always make _evalUrl requests as dataType: script
9af0a4e 
IE and iOS <10 XHR transport does not succeed on data: URIs
Ref jquerygh-4243
Ref jquerygh-4126
@mgol
Copy link
Member

mgol commented Dec 13, 2018

@gibson042 Could you explain what the issue is and why your new solution preserves the solution from #4243 while not triggering issues in IE or old iOS?

@gibson042
Copy link
Member Author

gibson042 commented Dec 13, 2018

It reverts the change away from dataType: "script", which can take advantage of script-tag transport rather than XHR and is apparently necessary for some browsers in some cases. It then fixes #4126 in a different way, by overriding the default text-to-script converter (the usual invocation point of jQuery.globalEval) and instead using the jQuery.ajax dataFilter setting, which has the convenient property of being invoked ahead of conversion only for successful responses.

I added some comments explaining the kludge.

mgol
mgol approved these changes Dec 13, 2018
View reviewed changes
Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good to me (I assume you've tested that on IE or old iOS).

timmywil
timmywil approved these changes Dec 13, 2018
View reviewed changes
Copy link
Member

@timmywil timmywil left a comment

Choose a reason for hiding this comment

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

LGTM

@gibson042 gibson042 merged commit 13de7c9 into jquery:master Dec 13, 2018
@skrobinson skrobinson mentioned this pull request Apr 26, 2019
Closed
@lock lock bot locked as resolved and limited conversation to collaborators Jun 11, 2019
@mgol mgol added this to the 3.4.0 milestone Jan 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@timmywil timmywil timmywil approved these changes

@mgol mgol mgol approved these changes

Assignees

No one assigned

Labels

Manipulation

Milestone

3.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@gibson042 @mgol @timmywil