8000 Enable POSTing JSON objects with casper.open() by sorenlouv · Pull Request #832 · casperjs/casperjs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jun 19, 2020. It is now read-only.

Enable POSTing JSON objects with casper.open() #832

Merged
merged 1 commit into from
Feb 20, 2014
Merged

Enable POSTing JSON objects with casper.open() #832

merged 1 commit into from
Feb 20, 2014

Conversation

sorenlouv
Copy link
Contributor

Use JSON.stringify for encoding the request body when Content-Type is application/json.

@sorenlouv
Copy link
Contributor Author

This should fix #196

I've run casperjs selftest and everything is green :)

@mickaelandrieu
Copy link
Member

Hi @sqren

Can you please add a test for your specific case ?

Regards,

@sorenlouv
Copy link
Contributor Author

Added a test.

@mickaelandrieu
Copy link
Member

For me 👍 when you will fix some typos.

Let's wait @n1k0 review ;)

Use JSON.stringify for encoding the request body when Content-Type is "application/json".
Ref #196
@sorenlouv
Copy link
Contributor Author

Thanks for the comments. I've fixed the style issues and squashed everything into a single commit.

@n1k0
Copy link
Member
n1k0 commented Feb 17, 2014

Is JSON encoding parameters the default behavior we want when using application/json? Shouldn't we just add documentation for such a case, so people may use JSON.stringify explicitely?

@sorenlouv
Copy link
Contributor Author

The most important thing is, that we don't run qs.encode on the body if the Content-Type is application/json.
I'm not sure why someone would want to send data with a Content-Type of application/json but not encode with JSON.stringify, which is why I think it makes sense to add it.

@n1k0
Copy link
Member
n1k0 commented Feb 20, 2014

Hmm, you're kinda right :) Merging.

n1k0 added a commit that referenced this pull request Feb 20, 2014
Enable POSTing JSON objects with casper.open()
@n1k0 n1k0 merged commit 89cfda7 into casperjs:master Feb 20, 2014
@n1k0
Copy link
Member
n1k0 commented Feb 20, 2014

Thanks for contributing.

r8k added a commit to r8k/casperjs that referenced this pull request Feb 23, 2014
@r8k r8k mentioned this pull request Feb 23, 2014
r8k added a commit to r8k/casperjs that referenced this pull request Feb 23, 2014
r8k added a commit to r8k/casperjs that referenced this pull request Feb 23, 2014
r8k added a commit to r8k/casperjs that referenced this pull request Mar 8, 2014
this fixes casperjs#832 #196 in the following scenarios ..

  content-type can be
  * application/json
  * application/json; charset=utf-8
n1k0 added a commit that referenced this pull request Mar 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0