Skip to content

Conversation

@mgol
Copy link
Member

@mgol mgol commented Jan 12, 2023

Summary

The AJAX script transport has two versions: XHR + jQuery.globalEval or appending a script tag (note that jQuery.globalEval also appends a script tag now, but inline). The latter cannot support the headers option which has so far not been taken into account.

For jQuery 3.x, the main consequence was the option not being respected for cross-domain requests. Since in 4.x we use the latter way more often, the option was being ignored in more cases.

The transport now checks whether the headers option is specified and uses the XHR way unless scriptAttrs are specified as well.

Fixes gh-5142

+5 bytes

Checklist

@mgol mgol added Ajax Needs review Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. labels Jan 12, 2023
@mgol mgol added this to the 4.0.0 milestone Jan 12, 2023
@mgol mgol self-assigned this Jan 12, 2023
@mgol mgol changed the title Ajax: Support headers for script transport even when crossDomain Ajax: Support headers for script transport even when cross-domain Jan 12, 2023
The AJAX script transport has two versions: XHR + `jQuery.globalEval` or
appending a script tag (note that `jQuery.globalEval` also appends a
script tag now, but inline). The former cannot support the `headers`
option which has so far not been taken into account.

For jQuery 3.x, the main consequence was the option not being respected
for cross-domain requests. Since in 4.x we use the latter way more
often, the option was being ignored in more cases.

The transport now checks whether the `headers` option is specified and
uses the XHR way unless `scriptAttrs` are specified as well.

Fixes jquerygh-5142
@mgol mgol force-pushed the ajax-script-headers-gh-5142 branch from 890f0a6 to 2f867d9 Compare January 12, 2023 23:42
}

// Install script dataType. Don't specify `content.script` so that an explicit
// Install script dataType. Don't specify `contents.script` so that an explicit
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not related but since I was already modifying the nearby code... The comment was added in 025da4d and had the typo from the start.

@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Jan 30, 2023
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.

Agreed, just go the XHR route in this case

@mgol mgol removed the Needs review label Jan 30, 2023
@mgol mgol merged commit 6d13644 into jquery:main Feb 1, 2023
@mgol mgol deleted the ajax-script-headers-gh-5142 branch February 1, 2023 12:40
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

jQuery.ajax doesn’t send headers when ‘dataType‘ is ‘script’ and ‘crossDomain’ is true

2 participants