Breaking change in elgg_is_xhr()? #8735

Closed
hypeJunction opened this Issue Jul 14, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@hypeJunction
Contributor

hypeJunction commented Jul 14, 2015

Can't track down the commit that changed the behaviour of elgg_is_xhr(). I have noticed this in 1.12 branch, but this could have happened earlier.
This function used to check the query parameter as well as the headers, now it only checks the header using Symfony request. This breaks the output for some JS that was passing X-Requested-With as a URL query parameter.

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Jul 14, 2015

Contributor

In case anyone else encounters this problem while using jquery.form plugin: removing iframe option rectifies the issue in modern browsers, as the plugin is capable of handling file uploads without using iframes.

Contributor

hypeJunction commented Jul 14, 2015

In case anyone else encounters this problem while using jquery.form plugin: removing iframe option rectifies the issue in modern browsers, as the plugin is capable of handling file uploads without using iframes.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Jul 14, 2015

Member

If jquery.form uses iframes by default, I think we should issue a fix to add query checking back in.

Member

mrclay commented Jul 14, 2015

If jquery.form uses iframes by default, I think we should issue a fix to add query checking back in.

@mrclay mrclay added this to the Elgg 1.12.x milestone Oct 23, 2015

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Oct 23, 2015

Member

It used to check all input: 6e05a5d

Member

mrclay commented Oct 23, 2015

It used to check all input: 6e05a5d

@mrclay mrclay added the regression label Oct 23, 2015

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Jan 26, 2016

Member

FYI elgg.embed.submit() sets X-Requested-With in the data (specifically for iframe handling), so we shouldn't have removed this.

Member

mrclay commented Jan 26, 2016

FYI elgg.embed.submit() sets X-Requested-With in the data (specifically for iframe handling), so we shouldn't have removed this.

@mrclay mrclay modified the milestones: Elgg 2.0.x, Elgg 1.12.x Jan 26, 2016

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 4, 2016

fix(ajax): iframe-based submissions can again be recognized as XHR re…
…quests

We were relying on this behavior so we return it to 1.x behavior and add tests
for this and the other Request methods.

Fixes #8735
@mrclay

This comment has been minimized.

Show comment
Hide comment
Member

mrclay commented Mar 4, 2016

PR #9443

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