1745 - Fixed Cross Domain requests for new Content Type settings #1754

Closed
wants to merge 7 commits into
from

2 participants

@NTaylorMullen

Fixes #1745 and addresses the issues caused by #947.

@davidfowl davidfowl and 1 other commented on an outdated diff Mar 26, 2013
...Net.SignalR.Client.JS.Tests/Scripts/jquery.signalR.js
@@ -331,6 +333,8 @@
connection.log("Using jsonp because this browser doesn't support CORS");
}
}
+
+ connection.contentType = "application/x-www-form-urlencoded; charset=UTF-8";
@davidfowl
SignalR member
davidfowl added a line comment Mar 26, 2013

So the content type for all cross domain (jsonp or not) is formurl encoded?

@NTaylorMullen
NTaylorMullen added a line comment Mar 26, 2013

It's the general case content type, that's jquery's default.

@davidfowl
SignalR member
davidfowl added a line comment Mar 26, 2013

Should we call it defaultContentType.

@NTaylorMullen
NTaylorMullen added a line comment Mar 26, 2013

Sounds good to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davidfowl
SignalR member

Did you test jsonp? Is formurl encoded ok for JSONP?

@NTaylorMullen

ya I did, I ran the cross domain sample for ie8/ie9/ie10/chrome/opera/ff connecting to one of our dev servers

@DamianEdwards DamianEdwards commented on the diff Mar 27, 2013
...nalR.Hosting.AspNet.Samples/Scripts/jquery.signalR.js
@@ -120,6 +120,10 @@
return new signalR.fn.init(url, qs, logging);
};
+ signalR._ = {
+ defaultContentType: "application/x-www-form-urlencoded; charset=UTF-8"
@DamianEdwards
SignalR member
DamianEdwards added a line comment Mar 27, 2013

why does this have to hang off of the ctor function? Why can't it just be a private variable in the outer closure?

@NTaylorMullen
NTaylorMullen added a line comment Mar 27, 2013

It's used in the transport.common

@DamianEdwards
SignalR member
DamianEdwards added a line comment Mar 27, 2013

Never mind, see now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@DamianEdwards DamianEdwards and 1 other commented on an outdated diff Mar 27, 2013
...SignalR.Client.JS/jquery.signalR.transports.common.js
@@ -7,6 +7,7 @@
"use strict";
var signalR = $.signalR,
+ signalRPrivate = $.signalR._,
@DamianEdwards
SignalR member
DamianEdwards added a line comment Mar 27, 2013

Hate this variable name. Why do we even need it? Just use signalR._

@NTaylorMullen
NTaylorMullen added a line comment Mar 27, 2013

Makes sense also since it's only used once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@NTaylorMullen

Updated this with a JS functional test to cover #947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment