jquery-jsonp integration with cometd client

14 views
Skip to first unread message

atzoum

unread,
Jun 30, 2009, 5:10:18 AM6/30/09
to cometd-dev
It is well known that JQuery's official JSONP implementation doesn't
support error handling in jsonp requests.
However, there is an alternative solution of jquery-jsonp at
http://code.google.com/p/jquery-jsonp/ which supports error recovery.
Jquery-jsonp plugin is using iframes to do the jsonp request and that
causes problems with javascript Arrays passed from the jsonp-iframe to
the parent.

I believe that it would be better to use this plugin in the jquery
cometd implementation in order to support client reconnection with
callback-polling transport. However, as mentioned above, it requires
some modifications in cometd-javascript common code in order to handle
Array objects.

Could I contribute somehow?

Simone Bordet

unread,
Jul 1, 2009, 3:37:58 AM7/1/09
to comet...@googlegroups.com
Hi,

On Tue, Jun 30, 2009 at 11:10, atzoum<atz...@gmail.com> wrote:
>
> It is well known that JQuery's official JSONP implementation doesn't
> support error handling in jsonp requests.

I just checked that this is true for Dojo as well.

> However, there is an alternative solution of jquery-jsonp at
> http://code.google.com/p/jquery-jsonp/ which supports error recovery.
> Jquery-jsonp plugin is using iframes to do the jsonp request and that
> causes problems with javascript Arrays passed from the jsonp-iframe to
> the parent.
>
> I believe that it would be better to use this plugin in the jquery
> cometd implementation in order to support client reconnection with
> callback-polling transport. However, as mentioned above, it requires
> some modifications in cometd-javascript common code in order to handle
> Array objects.
>
> Could I contribute somehow?

Can you expand a bit on why there is this Array problem ? And how the
common code would be changed to support that ?
I took a look at the jquery-jsonp plugin, and seems to me generic
enough to try a generalization that will work for dojo as well.

Thanks,

Simon
--
http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless. Victoria Livschitz

atzoum

unread,
Jul 1, 2009, 4:17:56 AM7/1/09
to cometd-dev
Each page (iframes too) has a global context with its own "Array"
function. Therefore "instance of Array" fails.
The javascript common code uses instance of in _convertToMessages
function.
I replaced
...
if (response instanceof Array) return response;
...
with
if ( typeof(response) != "function" && typeof(response.length) ==
'number') return response; (taken from json-1.3.js)

In jquery.cometd.js I changed the ajax request in callback-polling to:

$.jsonp({
url: packet.url,
callbackParameter: 'jsonp',
data: {
// In callback-polling, the content must be sent
via the 'message' parameter
message: packet.body
},
success: packet.onSuccess,
error: function(xOption, reason) { packet.onError
(reason, reason); }
});

This seems to be working ok, but...

The problem that I haven't solved yet is that if the data object of
the bayeux message is - or contains - an Array, the instance of still
doesn't work, since it is an Array that was created within the jsonp
iframe, hence a different object.

A possible solution would be to call toJSON and evalJSON inside
_convertToMessages function so that the Arrays would be re-created
with the correct type, but I don't know how good it is (performance-
wise).

On Jul 1, 10:37 am, Simone Bordet <simone.bor...@gmail.com> wrote:
> Hi,
>
> On Tue, Jun 30, 2009 at 11:10, atzoum<atz...@gmail.com> wrote:
>
> > It is well known that JQuery's official JSONP implementation doesn't
> > support error handling in jsonp requests.
>
> I just checked that this is true for Dojo as well.
>
> > However, there is an alternative solution of jquery-jsonp at
> >http://code.google.com/p/jquery-jsonp/which supports error recovery.
> > Jquery-jsonp plugin is using iframes to do the jsonp request and that
> > causes problems with javascript Arrays passed from the jsonp-iframe to
> > the parent.
>
> > I believe that it would be better to use this plugin in the jquery
> > cometd implementation in order to support client reconnection with
> > callback-polling transport. However, as mentioned above, it requires
> > some modifications in cometd-javascript common code in order to handle
> > Array objects.
>
> > Could I contribute somehow?
>
> Can you expand a bit on why there is this Array problem ? And how the
> common code would be changed to support that ?
> I took a look at the jquery-jsonp plugin, and seems to me generic
> enough to try a generalization that will work for dojo as well.
>
> Thanks,
>
> Simon
> --http://bordet.blogspot.com

atzoum

unread,
Jul 1, 2009, 7:17:23 AM7/1/09
to cometd-dev
Got it all working fine!

I' m posting the diff for the jquery-jsonp patch.
As you' ll notice, I changed the _mixin() function of cometd.js
because of a bug that caused null values to be copied as empty
objects. As a result, the stringified json object instead of...
"value": null
...was...
"value": {}

The above should be fixed in an upcoming cometd release





Index: cometd.js
===================================================================
@@ -85,8 +85,8 @@
if (prop === target) continue;
// Do not mixin undefined values
if (prop === undefined) continue;
-
- if (deep && typeof prop === "object")
+ /* atzoum patch */
+ if (deep && typeof prop === "object" && prop)
{
result[name] = _mixin(deep, result[name], prop);
}
@@ -617,7 +617,10 @@
*/
function _convertToMessages(response)
{
- if (response === undefined) return [];
+
+ response = org.cometd.JSON.fromJSON(org.cometd.JSON.toJSON
(response));
+ if (response === undefined) return [];
+ /* atzoum patch */
if (response instanceof Array) return response;
if (response instanceof String || typeof response ==
'string') return org.cometd.JSON.fromJSON(response);
if (response instanceof Object) return [response];

Index: jquery.cometd.js
===================================================================
@@ -31,23 +31,15 @@
}
else if (transportType == 'callback-polling')
{
- $.ajax({
+ $.jsonp({
url: packet.url,
- type: 'GET',
- dataType: 'jsonp',
- jsonp: 'jsonp',
+ callbackParameter: 'jsonp',
data: {
// In callback-polling, the content must be sent
via the 'message' parameter
message: packet.body
},
- beforeSend: function(xhr)
- {
- _setHeaders(xhr, packet.headers);
- // Returning false will abort the XHR send
- return true;
- },
success: packet.onSuccess,
- error: function(xhr, reason, exception)
{ packet.onError(reason, exception); }
+ error: function(xOption, reason) { packet.onError
(reason, reason); }
});
}
else
Index: jquery.json-1.3.js
===================================================================
@@ -53,7 +53,7 @@
return '"' + string.replace(escapeable, function (a)
{
var c = meta[a];
- if (typeof c === 'string') {
+ if (typeof c == 'string') {
return c;
}
c = a.charCodeAt();
@@ -64,15 +64,16 @@
};

$.toJSON = function(o, compact)
- {
+ {
var type = typeof(o);

if (type == "undefined")
return "undefined";
else if (type == "number" || type == "boolean")
return o + "";
- else if (o === null)
- return "null";
+ else if (!o)
+ return "null";
+

// Is it a string?
if (type == "string")
@@ -85,7 +86,7 @@
return o.toJSON(compact);

// Is it an array?
- if (type != "function" && typeof(o.length) == "number")
+ if (/*type != "function" && */typeof(o.length) == "number")
{
var ret = [];
for (var i = 0; i < o.length; i++) {
@@ -99,7 +100,7 @@

// If it's a function, we have to warn somebody!
if (type == "function") {
- throw new TypeError("Unable to convert object of type
'function' to json.");
+ throw new TypeError("Unable to convert object of type
'function' to json." + o);
}

// It's probably an object, then.

Simone Bordet

unread,
Jul 2, 2009, 7:18:50 AM7/2/09
to comet...@googlegroups.com
Hi,

On Wed, Jul 1, 2009 at 13:17, atzoum<atz...@gmail.com> wrote:
>
> Got it all working fine!
>
> I' m posting the diff for the jquery-jsonp patch.
> As you' ll notice, I changed the _mixin() function of cometd.js
> because of a bug that caused null values to be copied as empty
> objects. As a result, the stringified json object instead of...
> "value": null
> ...was...
> "value": {}
>
> The above should be fixed in an upcoming cometd release

It's fixed in trunk, thanks.

As for the rest, there are things that we can do, others that we
cannot because we're not the owner of the source.

> @@ -617,7 +617,10 @@
>      */
>     function _convertToMessages(response)
>     {
> -        if (response === undefined) return [];
> +
> +       response = org.cometd.JSON.fromJSON(org.cometd.JSON.toJSON
> (response));
> +       if (response === undefined) return [];
> +        /* atzoum patch */
>         if (response instanceof Array) return response;
>         if (response instanceof String || typeof response ==
> 'string') return org.cometd.JSON.fromJSON(response);
>         if (response instanceof Object) return [response];

I am not sure about this fix. The idea is that *before* assuming
that's a string and converting it to object from json, I do the other
checks first.

About this one I am not fully sure: does it work without the patches below ?
I ask because I cannot change the jquery json plugin.

However, glad to made it working.
Does it work in IE8 as well ?

Thanks,

Simon
--

http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless. Victoria Livschitz

> Index: jquery.json-1.3.js

atzoum

unread,
Jul 2, 2009, 7:35:51 AM7/2/09
to cometd-dev
I tested the code in IE6 and Firefox and it is working.

I do not assume that it is a String, since I do org.cometd.JSON.toJSON
(response) first. However, it is better indeed performing some of the
checks before doing the conversion.

I had to change the jquery.json-1.3.js source because of the ===
operations (type and value equality) which do not work across iframes.
I believe that the changes are safe, but the owner of jquery json
plugin should judge this. Maybe we could post a change request in the
project's page.


It's nice though to have callback polling with error handling and
reconnection :)

Julian Aubourg

unread,
Jul 11, 2009, 9:47:16 PM7/11/09
to cometd-dev
Hi,

I'm the author of jquery-jsonp and I was totally unaware of this
"multiple definition" problem regarding Array(). I see this issue as a
bug in jquery-jsonp since the idea is for it to be as transparent as
humanly possible. So I have three questions:
1) Is this limited to Array() and, if not, could anyone link me to a
page listing definitions local to windows/iframes?
2) Is this behavior browser dependant?
3) Why hasn't this been posted in the jquery-jsonp issue list? :P

I have total control over the iframe code within the plugin. If all I
need to add is "Array = parent.Array;" in the iframe code, I'll be
very happy to add it so that you don't need to change a single line in
your own code (apart for the $.ajax to $.jsonp transformation).

Very nice to see the plugin in use.

Regards,

-- Julian
> > >http://code.google.com/p/jquery-jsonp/whichsupports error recovery.

atzoum

unread,
Jul 15, 2009, 2:36:20 AM7/15/09
to cometd-dev
Hello,


On Jul 12, 4:47 am, Julian Aubourg <aubourg.jul...@gmail.com> wrote:
> Hi,
>
> I'm the author of jquery-jsonp and I was totally unaware of this
> "multiple definition" problem regarding Array(). I see this issue as a
> bug in jquery-jsonp since the idea is for it to be as transparent as
> humanly possible. So I have three questions:
> 1) Is this limited to Array() and, if not, could anyone link me to a
> page listing definitions local to windows/iframes?

I don't know, I 've seen it in Arrays only (due to cometd
implementation).
However, in this book http://docstore.mik.ua/orelly/webprog/jscript/ch13_11.htm
I read the following:
"each window has an independent copy of the constructor and an
independent copy of the constructor's prototype object. For example,
each window has its own copy of the String( ) constructor and the
String.prototype object."

> 2) Is this behavior browser dependant?
No, this applies to all browsers.

> 3) Why hasn't this been posted in the jquery-jsonp issue list? :P
Too busy maybe :(
> > > >http://code.google.com/p/jquery-jsonp/whichsupportserror recovery.

Julian Aubourg

unread,
Jul 15, 2009, 2:41:28 PM7/15/09
to comet...@googlegroups.com
OK, so I've tested around a bit and the best you can do with current implementation is by making use of the dataFilter option.
Provided you have a JSON implementation (see http://www.json.org/json2.js but I've seen you have one: org.cometd.JSON iirc), something like the following should do the trick:

$.jsonp({
  url: packet.url,
  callbackParameter: 'jsonp',
  data: {
    // In callback-polling, the content must be sent
    // via the 'message' parameter
    message: packet.body
  },
  dataFilter: function(data) {
    var JSON = org.cometd.JSON;
    return JSON.fromJSON(JSON.toJSON(data));
  },
  success: packet.onSuccess,
  error: function(xOption, reason) {
    packet.onError(reason, reason);
  }
});

Of course, there's a performance penalty but I'm confident it will end up unnoticeable in the context of a network call (especially if and when org.cometd.JSON makes use of native implementation when available).

If you were to make use of the pageCache option, I could create a "prefetch" option similar to dataFilter but taking place BEFORE the storage in cache, so that the operation is only performed once per page lifecycle.

Anyway, with the snippet above, you have no need for any change in your code base (array constructors are === to the Array function of the main window).

Surely something I should add to the Tips & Tricks wiki page.

2009/7/15 atzoum <atz...@gmail.com>
Reply all
Reply to author
Forward
0 new messages