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

force .load to POST instead of GET #184

Closed
wants to merge 1 commit into from

Conversation

trwww
Copy link
Contributor

@trwww trwww commented Apr 9, 2016

AJAXifying the preview scrips with .load() via .serialize() does a GET
request with the Display.html form, and the query string can end up too
long for the server to accept it.

This commit adds a helper to serialize the form as an object instead of
query string, which tells .load to POST instead of GET. It looks like
theres no built in jQuery interface to serialize a form as an object so
the helper is needed.

Fixes issues#31874

AJAXifying the preview scrips with .load() via .serialize() does a GET
request with the Display.html form, and the query string can end up too
long for the server to accept it.

This commit adds a helper to serialize the form as an object instead of
query string, which tells .load to POST instead of GET. It looks like
theres no built in jQuery interface to serialize a form as an object so
the helper is needed.

Fixes issues#31874
@JvdW
Copy link

JvdW commented Apr 16, 2016

I implemented this patch and it works OK but has a unpleasant side effect.
How to reproduce:
Reply to a ticket and make sure you have the right to see the scrip preview pane
Notice that all that need to recieve email are having a checkmark!
Click into the message body reply field, type some text, not really needed
Click outside the text field on a place that is NOT a other form element such as the Owner dropdown.
Notice that the checkmarks disappear :-(

I implented it by copying the 2 files to the correct locations in rt4/local/... and applying the patch there.
I applied with the -b option so for testing I mv'ed the patched files to *.patched and restarted httpd making sure to cleanout the mason cache. The above mention problem disappeared for me.

@alexmv
Copy link
Contributor

alexmv commented Apr 16, 2016

I commented on the issues ticket, but not here. 4.4/preview-scrips-post fixes both of these issues.

@trwww
Copy link
Contributor Author

trwww commented Apr 17, 2016

Closing pull request as @alexmv's branch is much cleaner.

@trwww trwww closed this Apr 17, 2016
@trwww trwww deleted the 4.4/POST-preview-scrips branch April 21, 2016 13:54
@sartak
Copy link
Contributor

sartak commented Apr 21, 2016

Hi Todd, JvdW, Alex. Just to close the loop on this, I merged Alex's 4.4/preview-scrips-post branch to 4.4-trunk as 63944d7. Thanks everybody. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants