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.

Sign up for GitHub

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

Update atmosphere.js #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update atmosphere.js #265

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 44 additions & 63 deletions modules/javascript/src/main/webapp/javascript/atmosphere.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,20 @@

"use strict";

var atmosphere = {},
guid,
Copy link
Author

Choose a reason for hiding this comment

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

This variable and hasOwn below were not used.

offline = false,
var offline = false,
requests = [],
callbacks = [],
uuid = 0,
hasOwn = Object.prototype.hasOwnProperty;
uuid = 0;

atmosphere = {
/**
* {boolean} If window beforeUnload event has been called.
* Flag will be reset after 5000 ms
*
* @private
*/
var _beforeUnloadState = false;

var atmosphere = {
version: "3.0.6-javascript",
onError: function (response) {
},
Expand Down Expand Up @@ -158,13 +163,15 @@
suspend: true,
maxRequest: -1,
reconnect: true,
mrequest: undefined,
maxStreamingLength: 10000000,
lastIndex: 0,
logLevel: 'info',
requestCount: 0,
fallbackMethod: 'GET',
fallbackTransport: 'streaming',
transport: 'long-polling',
webSocketUrl: undefined,
webSocketImpl: null,
webSocketBinaryType: null,
dispatchUrl: null,
Expand Down Expand Up @@ -196,6 +203,16 @@
handleOnlineOffline: true,
maxWebsocketErrorRetries: 1,
curWebsocketErrorRetries: 0,
id: undefined,
openId: undefined,
reconnectId: undefined,
firstMessage: undefined,
isOpen: undefined,
isReopen: undefined,
closed: undefined,
ctime: undefined,
heartbeatTimer: undefined,
force: undefined,
Comment on lines +206 to +215
Copy link
Author

Choose a reason for hiding this comment

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

All these, plus mrequest and webSocketUrl above, were accessed as properties of the _request object (or copies thereof) but never declared. It is not strictly necessary, but helps to have an overview of all the properties that will be used.

onError: function (response) {
},
onClose: function (response) {
Expand Down Expand Up @@ -232,7 +249,7 @@
reasonPhrase: "OK",
responseBody: '',
messages: [],
headers: [],
headers: {},
Copy link
Author

Choose a reason for hiding this comment

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

Every use of this field was as an object.

state: "messageReceived",
transport: "polling",
error: null,
Expand Down Expand Up @@ -347,14 +364,6 @@
/** Key for connection sharing */
var _sharingKey;

/**
* {boolean} If window beforeUnload event has been called.
* Flag will be reset after 5000 ms
*
* @private
*/
var _beforeUnloadState = false;
Copy link
Author

Choose a reason for hiding this comment

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

This was never accessed. Rather, the nonexistent atmosphere._beforeUnloadState was. I moved this declaration to line 52.


// Automatic call to subscribe
_subscribe(options);

Expand Down Expand Up @@ -1372,7 +1381,7 @@

_response.transport = "websocket";

var location = _buildWebSocketUrl(_request.url);
var location = _buildWebSocketUrl();
Copy link
Author

Choose a reason for hiding this comment

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

This function takes no arguments.

if (_canLog('debug')) {
atmosphere.util.debug("Invoking executeWebSocket, using URL: " + location);
}
Expand Down Expand Up @@ -1746,7 +1755,7 @@
} else {
atmosphere.util.log(_request.logLevel, ["Websocket reconnect maximum try reached " + _requestCount]);
if (_canLog('warn')) {
atmosphere.util.warn("Websocket error, reason: " + message.reason);
atmosphere.util.warn("Websocket error, reason: " + (message || {}).reason);
Copy link
Author

Choose a reason for hiding this comment

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

message is being accessed here as a global variable, but hasn't been initialized as such. I added a default value to avoid the "NPE", but I expect this log to end with reason: undefined, so further revision is necessary.

}
_onError(0, "maxReconnectOnClose reached");
}
Expand All @@ -1763,8 +1772,6 @@

if (typeof (_request.onTransportFailure) !== 'undefined') {
_request.onTransportFailure(errorMessage, _request);
} else if (typeof (atmosphere.util.onTransportFailure) !== 'undefined') {
Copy link
Author

Choose a reason for hiding this comment

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

atmosphere.util simply does not have a field onTransportFailure, so I don't see the point of having this check.

atmosphere.util.onTransportFailure(errorMessage, _request);
}

var reconnectInterval = _request.connectTimeout === -1 ? 0 : _request.connectTimeout;
Expand Down Expand Up @@ -1929,7 +1936,7 @@
};

var reconnectF = function (force) {
if (atmosphere._beforeUnloadState) {
if (_beforeUnloadState) {
// ATMOSPHERE-JAVASCRIPT-143: Delay reconnect to avoid reconnect attempts before an actual unload (we don't know if an unload will happen, yet)
atmosphere.util.debug(new Date() + " Atmosphere: reconnectF: execution delayed due to _beforeUnloadState flag");
setTimeout(function () {
Expand Down Expand Up @@ -1972,7 +1979,7 @@
_response.error = true;
_response.ffTryingReconnect = true;
try {
_response.status = XMLHttpRequest.status;
Copy link
Author

Choose a reason for hiding this comment

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

XMLHttpRequest does not have a status field. I'm assuming we want here the status from ajaxRequest, the instance of XMLHttpRequest.

_response.status = ajaxRequest.status;
} catch (e) {
_response.status = 500;
}
Expand Down Expand Up @@ -2246,7 +2253,7 @@
status = ajaxRequest.status > 1000 ? 0 : ajaxRequest.status;
}
_response.status = status === 0 ? 204 : status;
_response.reason = status === 0 ? "Server resumed the connection or down." : "OK";
_response.reasonPhrase = status === 0 ? "Server resumed the connection or down." : "OK";
Copy link
Author

Choose a reason for hiding this comment

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

_response has reasonPhrase, but not reason...


clearTimeout(request.id);
if (request.reconnectId) {
Expand Down Expand Up @@ -2504,7 +2511,7 @@
res.innerText = "";
var skipCallbackInvocation = _trackMessageSize(text, rq, _response);
if (skipCallbackInvocation) {
return "";
return false;
Copy link
Author

Choose a reason for hiding this comment

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

Should be equivalent at run time, but it is more clear if the return type is the same across the function.

}

_prepareCallback(_response.responseBody, "messageReceived", 200, rq.transport);
Expand Down Expand Up @@ -2748,8 +2755,6 @@
if (m.id !== guid) {
if (typeof (_request.onLocalMessage) !== 'undefined') {
_request.onLocalMessage(m.event);
} else if (typeof (atmosphere.util.onLocalMessage) !== 'undefined') {
Copy link
Author

Choose a reason for hiding this comment

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

Similar to with onTransportFailure, atmosphere.util simply does not have a field onLocalMessage.

atmosphere.util.onLocalMessage(m.event);
}
}
}
Expand Down Expand Up @@ -3190,15 +3195,15 @@
return s.join("&").replace(/%20/g, "+");
},

storage: function () {
storage: (function () {
Copy link
Author

Choose a reason for hiding this comment

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

atmosphere.util.storage is being checked as a boolean and is being assigned boolean values. Declaring it as a function looked like a mistake to me.

try {
return !!(window.localStorage && window.StorageEvent);
} catch (e) {
//Firefox throws an exception here, see
//https://bugzilla.mozilla.org/show_bug.cgi?id=748620
return false;
}
},
})(),

iterate: function (fn, interval) {
var timeoutId;
Expand All @@ -3222,46 +3227,24 @@
};
},

each: function (obj, callback, args) {
each: function (obj, callback) {
Copy link
Author

Choose a reason for hiding this comment

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

atmosphere.util.each is always called with exactly 2 arguments.

if (!obj) return;
var value, i = 0, length = obj.length, isArray = atmosphere.util.isArray(obj);

if (args) {
if (isArray) {
for (; i < length; i++) {
value = callback.apply(obj[i], args);

if (value === false) {
break;
}
}
} else {
for (i in obj) {
value = callback.apply(obj[i], args);
if (isArray) {
for (; i < length; i++) {
value = callback.call(obj[i], i, obj[i]);

if (value === false) {
break;
}
if (value === false) {
break;
}
}

// A special, fast, case for the most common use of each
} else {
if (isArray) {
for (; i < length; i++) {
value = callback.call(obj[i], i, obj[i]);

if (value === false) {
break;
}
}
} else {
for (i in obj) {
value = callback.call(obj[i], i, obj[i]);
for (i in obj) {
value = callback.call(obj[i], i, obj[i]);

if (value === false) {
break;
}
if (value === false) {
break;
}
}
}
Expand Down Expand Up @@ -3360,8 +3343,6 @@
}
};

guid = atmosphere.util.now();

// Browser sniffing
(function () {
var ua = navigator.userAgent.toLowerCase(),
Expand Down Expand Up @@ -3403,10 +3384,10 @@
atmosphere.util.debug(new Date() + " Atmosphere: " + "beforeunload event");

// ATMOSPHERE-JAVASCRIPT-143: Delay reconnect to avoid reconnect attempts before an actual unload (we don't know if an unload will happen, yet)
atmosphere._beforeUnloadState = true;
_beforeUnloadState = true;
setTimeout(function () {
atmosphere.util.debug(new Date() + " Atmosphere: " + "beforeunload event timeout reached. Reset _beforeUnloadState flag");
atmosphere._beforeUnloadState = false;
_beforeUnloadState = false;
}, 5000);
},
offline: function () {
Expand Down