Skip to content
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

Remove ajax timeout from contribution page on behalf of #18140

Merged
merged 1 commit into from
Oct 7, 2020

Conversation

mattwire
Copy link
Contributor

Overview

You'll probably only hit this timeout when debugging / slow connection. But it's the only place (I could find) that specifies an AJAX timeout in CiviCRM templates/js and it doesn't seem needed.

Before

AJAX call civicrm/ajax/permlocation times out after 5 seconds and you see "Common.js HTTP timeout" in browser console if debugging / site is slow to load / changing payment processors is slow.

After

Timeout gone. civicrm/ajax/permlocation loads in it's own time without timeout. If there was a real problem you'd see a 500 error or similar.

Technical Details

It is possible to specify timeouts on AJAX calls but we generally don't in CiviCRM.

Comments

@civibot
Copy link

civibot bot commented Aug 13, 2020

(Standard links)

@civibot civibot bot added the master label Aug 13, 2020
@eileenmcnaughton
Copy link
Contributor

@colemanw

@agileware-justin
Copy link
Contributor

Pretty sure we need to have a timeout otherwise the browser will just hang there, waiting for a response - even if that response never arrives.

@mattwire
Copy link
Contributor Author

@algileware-justin the issue is this is the only ajax call in CiviCRM where a timeout is defined. If it doesn't return it's quite likely that the rest of the page won't work either as it suggests a network issue or similar. There is also no handling on the form for the case when the request times out.

@agileware-justin
Copy link
Contributor

@mattwire that sounds great :)

@eileenmcnaughton
Copy link
Contributor

ok-without-test as not really testable

@colemanw colemanw merged commit b695565 into civicrm:master Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants