Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 11292041: Use XMLHttpRequest.status instead of nsIHttpChannel.responseStatus(same value, better cross-browse… (Closed)

Created:
July 29, 2013, 1:45 p.m. by Wladimir Palant
Modified:
Nov. 4, 2013, 11:46 a.m.
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

Use XMLHttpRequest.status instead of nsIHttpChannel.responseStatus (same value, better cross-browse…

Patch Set 1 #

Total comments: 2

Patch Set 2 : Removed exception handling #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -5 lines) Patch
M lib/downloader.js View 1 1 chunk +1 line, -5 lines 0 comments Download

Messages

Total messages: 3
Wladimir Palant
July 29, 2013, 1:45 p.m. (2013-07-29 13:45:02 UTC) #1
Thomas Greiner
LGTM with that change http://codereview.adblockplus.org/11292041/diff/1/lib/downloader.js File lib/downloader.js (right): http://codereview.adblockplus.org/11292041/diff/1/lib/downloader.js#newcode224 lib/downloader.js:224: responseStatus = request.status; You don't ...
July 31, 2013, 12:34 p.m. (2013-07-31 12:34:45 UTC) #2
Wladimir Palant
July 31, 2013, 1:31 p.m. (2013-07-31 13:31:26 UTC) #3
http://codereview.adblockplus.org/11292041/diff/1/lib/downloader.js
File lib/downloader.js (right):

http://codereview.adblockplus.org/11292041/diff/1/lib/downloader.js#newcode224
lib/downloader.js:224: responseStatus = request.status;
On 2013/07/31 12:34:45, Thomas Greiner wrote:
> You don't need a try/catch here anymore. You can simply do
> let responseStatus = request.status || -1;

I verified that the Mozilla implementation indeed doesn't throw exceptions, I
assume that other browsers don't do it either. Left out the || -1 part however -
if request.status is 0 then that's what we should report. -1 was a placeholder
for "cannot retrieve".

Powered by Google App Engine
This is Rietveld