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

XDR ie bug fix #466

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 46 additions & 11 deletions modules/jquery/src/main/webapp/jquery/jquery.atmosphere.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* Official documentation of this library: https://github.com/Atmosphere/atmosphere/wiki/jQuery.atmosphere.js-API
*/
jQuery.atmosphere = function() {
jQuery(window).unload(function() {
jQuery(window).bind("beforeunload", function() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think the beforeunload event should not be used by the library, because window.onbeforeunload ask users whether they want to remain on the current document or not. If the user selects to remain, the connection also should be maintained.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using "unload" event caused firefox open second connection when f5 is pressed. That's why we decided to use "beforeunload". I agree, that "beforeunload" is a dirty, but using "unload" isn't clear too.

jQuery.atmosphere.unsubscribe();
});

Expand Down Expand Up @@ -557,7 +557,7 @@ jQuery.atmosphere = function() {

_sse.onerror = function(message) {

clearTimeout(_request.id)
clearTimeout(_request.id);
_response.state = 'closed';
_response.responseBody = "";
_response.status = !sseOpened ? 501 : 200;
Expand Down Expand Up @@ -895,8 +895,8 @@ jQuery.atmosphere = function() {
rq = request;
}

// CORS fake using JSONP
if ((rq.transport == 'jsonp') || ((rq.enableXDR) && (jQuery.atmosphere.checkCORSSupport()))) {
// CORS fake using JSONP
if ((rq.transport == 'jsonp') && ((rq.enableXDR) && (jQuery.atmosphere.checkCORSSupport()))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will breaks JSONP support as JSONP will only works when enableXDR is enabled. What is the reason to change it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jfarcand, we use transport "streaming", enableXDR set in true for support IE9-10, in opera "if" return true. You may be better to fix.

_jsonp(rq);
return;
}
Expand Down Expand Up @@ -1220,11 +1220,12 @@ jQuery.atmosphere = function() {
if (isJunkEnded) {
var endOfJunk = "<!-- EOD -->";
var endOfJunkLenght = endOfJunk.length;
var junkEnd = responseBody.indexOf(endOfJunk) + endOfJunkLenght;

responseBody = responseBody.substring(junkEnd + lastIndex);
var junkEnd = responseBody.indexOf(endOfJunk);
if (junkEnd !== -1) {
responseBody = responseBody.substring(junkEnd + endOfJunkLenght + lastIndex);
lastIndex += responseBody.length;
}
}
}

_prepareCallback(responseBody, "messageReceived", 200, transport);
};
Expand All @@ -1241,6 +1242,7 @@ jQuery.atmosphere = function() {
case "PHPSESSID":
return url.replace(/\?PHPSESSID=[^&]*&?|\?|$/, "?PHPSESSID=" + match[2] + "&").replace(/&$/, "");
}
return url;
};

// Handles open and message event
Expand Down Expand Up @@ -1565,6 +1567,37 @@ jQuery.atmosphere = function() {
}
}

/**
* Проверяем заканчивается ли сообщение знаком messageDelimiter иначе посл сообщение уходит в буффер и добавиться в след раз
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you translate that one :-)

* @param message
* @param request
* @param response
*/
function _buffering(message, request, response) {
function ends(string, w) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method brokes WebSocket large message support. I'm investigating why, but may rollback for 1.0.0.beta3.

return w == string.substr(string.length - w.length);
}

if (response.bufferBody)
message = response.bufferBody + message;

if (!ends(message, request.messageDelimiter)) {
var messages = message.split(_request.messageDelimiter),
lastMessage = messages.pop();

response.bufferBody = lastMessage;

if (!messages.length) return true;

response.responseBody = messages.join(request.messageDelimiter);
} else {
response.responseBody = message;
response.bufferBody = '';
}

return false
}

function _prepareCallback(messageBody, state, errorCode, transport) {

if (state == "messageReceived") {
Expand Down Expand Up @@ -1622,8 +1655,10 @@ jQuery.atmosphere = function() {
func(_response);
};

if (_response.state == 'messageReceived' && _buffering(_response.responseBody, _request, _response)) return;

var messages = typeof(_response.responseBody) == 'string' ? _response.responseBody.split(_request.messageDelimiter) : new Array(_response.responseBody);
for (i = 0; i < messages.length; i++) {
for (var i = 0; i < messages.length; i++) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'i' global variable. Include too.To exclude problems in the projects.


if (messages.length > 1 && messages[i].length == 0) {
continue;
Expand Down Expand Up @@ -1890,8 +1925,8 @@ jQuery.atmosphere = function() {
/*
* jQuery stringifyJSON
* http://github.com/flowersinthesand/jquery-stringifyJSON
*
* Copyright 2011, Donghwan Kim
*
* Copyright 2011, Donghwan Kim
* Licensed under the Apache License, Version 2.0
* http://www.apache.org/licenses/LICENSE-2.0
*/
Expand Down