Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Do not redirect while DOING_AJAX #121

Closed
dboune opened this issue Jun 18, 2015 · 12 comments
Closed

Do not redirect while DOING_AJAX #121

dboune opened this issue Jun 18, 2015 · 12 comments
Assignees
Milestone

Comments

@dboune
Copy link

dboune commented Jun 18, 2015

I found it necessary in one circumstance to hook mlp_redirect_url and prevent redirects during an XHR.

Specifically this occurs with plugins that do not use wp-admin/admin-ajax.php to provide data to the front end. The case here was the use of the Infinite Scroll plugin, part of Jetpack.

You can see the code I am referring to here:
https://github.com/Automattic/jetpack/blob/3.5.3/modules/infinite-scroll/infinity.php#L596-L614

In any case, I believe the definitive determination of whether or not we are in an ajax call should be when DOING_AJAX is true, not is_admin(). It's possible to handle XHR outside of wp-admin/admin-ajax.php, and at the very least DOING_AJAX should be set if you care for other code to be able cope with it.

@thefuxia
Copy link

This shouldn't happen. The redirect is disabled when is_admin() is true. The file wp-admin/admin-ajax.php always runs in admin mode. Can you please provide the code to reproduce the problem?

@dboune
Copy link
Author

dboune commented Jun 19, 2015

I apologize for the thin report. I've updated the issue above. I was very glad to find the filter there, which allowed me to fashion a quick workaround, so thank you very much for having thought of that potential.

@dboune
Copy link
Author

dboune commented Jun 19, 2015

Now that I pause a moment.. I can see that you would likely need to do both tests, since you wouldn't want to redirect users within of the admin area in any case. I'm sure that was apparent to you, but I thought I'd mention that it did just now occur to me.

@thefuxia
Copy link

Oh, they are circumventing the regular API by using some custom AJAX response. Dirty, but yes, we can fix that.

The regular admin area will never be redirected.

@dboune
Copy link
Author

dboune commented Jun 19, 2015

Yes, I assume they have their reasons, though I can't say what they are. It's an automatic project, so I'll grudgingly give them the benefit of the doubt.

Very good. That certainly continues to be desirable.

@tfrommen
Copy link
Member

Hi Damian,

I just pushed a commit to a separate issue branch.
By default, AJAX requests will not be redirected anymore, no matter if admin-ajax.php or not (as long as DOING_AJAX is set to TRUE).
In case one actually would want the request redirected, there's a new filter mlp_redirect_frontend_ajax.

Testing welcome. :)

@thefuxia
Copy link

@tfrommen I cannot think of any case where an AJAX request should be redirected. Better wait until someone provides a use case before you widen the API.

@tfrommen
Copy link
Member

@toscho so you would just leave out the filter/variable, right?

@thefuxia
Copy link

Yes. The API should cover known use cases only. That makes it easier to change the code later.

tfrommen added a commit that referenced this issue Jun 19, 2015
@tfrommen tfrommen added this to the 2.2.0 milestone Jun 19, 2015
@dboune
Copy link
Author

dboune commented Jun 19, 2015

Thanks for the quick action! I'll test out the feature banch later today.

@dboune
Copy link
Author

dboune commented Jun 19, 2015

Tested and working!

@tfrommen
Copy link
Member

Hi Damian,

thanks for testing/confirming. The branch has been merged into master already.

Closed with 1c2731e.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants