reqwest JSONP doesn't work

I'm trying to get twitter shares count using Reqwest, it seems that JSONP from Reqwest doesn't work well.

Here're more details http://stackoverflow.com/questions/22372364/get-twitter-and-facebook-shares-using-reqwest-js

Mar 13 '14 11:03 Waterlog

that's odd. our tests are passing just fine. will have a look

Mar 13 '14 17:03 ded

have a look at our tests here https://github.com/ded/reqwest/blob/master/tests/tests.js

there's quite a few for JSONP so make sure you're implementing it correctly

Mar 13 '14 17:03 ded

I've tried multiple cases, nothing works.

Here's a demo http://jsbin.com/jaxuniya/2/edit?js,console,output Facebook code works well, but the one for Twitter quits with error 501 (Not Implemented):

XMLHttpRequest cannot load http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=?. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://run.jsbin.io' is therefore not allowed access.

Mar 14 '14 11:03 Waterlog

ok i can reproduce. not sure how this is became broken. in the meantime you can do this:

ajax({
  type: 'jsonp',
  url: 'http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=foo',
  jsonpCallback: 'foo'
})
function foo (data) {
  console.log(data)
}

Mar 16 '14 18:03 ded

@ded thanks, but your code also doesn't work http://jsbin.com/jaxuniya/3/edit?js,output

http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=foo 501 (Not Implemented) 
http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=foo No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://run.jsbin.io' is therefore not allowed access.
XMLHttpRequest cannot load http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=foo. No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://run.jsbin.io' is therefore not allowed access.

Mar 16 '14 18:03 Waterlog

the link you posted isn't what i wrote. here is an end of end example:

<!DOCTYPE html>
<html lang="en-US">
  <head>
    <title>test</title>
  <script src='reqwest.js'></script>
    <script type="text/javascript">
reqwest({
  type: 'jsonp',
  url: 'http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=foo',
  jsonpCallback: 'foo'
})
function foo (data) {
  console.log(data)
}
    </script>
  </head>
</html>

Mar 16 '14 21:03 ded

It works. I'm not sure about the other browsers, I've tested only in latest Google Chrome.

Please don't close current issue before I confirm that this code works in other browsers too.

This code looks pretty similar, but quits with an error instead:

reqwest({
  type: 'jsonp',
  url: 'http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=?',
  complete: function(data) {
    console.log(data);
  }
});

Uncaught TypeError: Cannot read property 'responseText' of undefined
(anonymous function) 
M.h.onload.h.onreadystatechange

So a jsonpCallback is fired only on success state? What about a fail state, should I use something like this instead?

reqwest({
  type: 'jsonp',
  url: 'http://urls.api.twitter.com/1/urls/count.json?url=http://google.com&callback=foo',
  jsonpCallback: 'foo',
  fail: function(data) {
    console.log('fail');
  }
});

Mar 17 '14 08:03 Waterlog

I've seen this error as well but only in IE10 and for cached responses. From my tests it looks like the "generalCallback" doesn't fire when the response arrives and lastValue remains undefined which causes the error Waterlog described above.

Mar 17 '14 09:03 boazhachlili

I've also noticed that jsonp and JSONP aren't similar for reqwest. Probably some place for a toLowerCase().

@boazhachlili you'd be a hero if you provide a fix for that.

Mar 17 '14 09:03 Waterlog

here's how I got mine to work: Basically, a hack was used for IE versions except IE10 (this code taken from older version of reqwest):

, isIE10 = navigator.userAgent.indexOf('MSIE 10.0') !== -1

and later:

if (typeof script.onreadystatechange !== 'undefined' && !isIE10) {
      // need this for IE due to out-of-order onreadystatechange(), binding script
      // execution to an event listener gives us control over when the script
      // is executed. See http://jaubourg.net/2010/07/loading-script-as-onclick-handler-of.html
    script.event = 'onclick'
    script.htmlFor = script.id = '_reqwest_' + reqId
}

For some reason, script.event = 'onclick' is missing from the current master, not sure if by design. I noticed that I need this fix for IE8 only. So I modified the condition for using this hack:

, isIE8 = navigator.userAgent.indexOf('MSIE 8') !== -1

and:

if (typeof script.onreadystatechange !== 'undefined' && isIE8) {

see lines 121, 138 and 142 for changes from the current reqwest code (https://github.com/ded/reqwest/blob/master/reqwest.js).

@Waterlog, I'd start by adding the missing script.event = 'onclick' and see if that helps.

Mar 17 '14 15:03 boazhachlili

@boazhachlili thanks mate.

I've already moved these requests to php side, since it looks like a jsonp support isn't stable right now. Not sure about the fix, anyway, I'm a bit tired of getting an issues from reqwest, hope this issue will be tested well and closed one day.

Mar 18 '14 18:03 Waterlog