Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Android stock browser fix for #1356 #5547

Closed
wants to merge 1 commit into from
Closed

Conversation

rafallo
Copy link
Contributor

@rafallo rafallo commented Dec 27, 2013

Android stock browser fix for #1356

@ghost ghost assigned tbosch Dec 28, 2013
@IgorMinar
Copy link
Contributor

lgtm, but we should verify this behavior before merging

@ghost ghost assigned tbosch Jan 6, 2014
@tbosch tbosch closed this in 28fc80b Jan 9, 2014
@tbosch
Copy link
Contributor

tbosch commented Jan 9, 2014

Was able to verify the bug and the solution.

@jr314159
Copy link

This caused a breaking change in our app. Status code 0 on an xhr in jQuery (and previously in Angular) was returned when the xhr request is cancelled due to the user navigating away to a different page. We display errors in our app when a request fails, except we specifically check if the failing status is 0, and in that case we know the user is just trying to navigate away so we don't display errors then.

If a request fails with a genuine 404 we still want to show errors, but now with this change we don't have any way to distinguish between a real 404 and a 404 created because the user is navigating away. I doubt this was the intended behavior of this pull request. Is there some other way to fix this Android bug without clobbering status code 0 everywhere else?

@jr314159
Copy link

Ok I seem to have found a better solution for detecting an aborted xhr due to user navigation. Instead of checking for response.status of 0, I check for empty response.headers(). I think this is probably more reliable.

I'm still not convinced you always want to be turning status 0 into 404, that seems a little heavy handed, but I'm good for now :)

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
Android 4.1 stock browser also returns status code 0 when
a template is loaded via `http` and the application is cached using
appcache.

Fixes angular#1356.
Closes angular#5547.
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
Android 4.1 stock browser also returns status code 0 when
a template is loaded via `http` and the application is cached using
appcache.

Fixes angular#1356.
Closes angular#5547.
pkozlowski-opensource added a commit to pkozlowski-opensource/angular.js that referenced this pull request Mar 1, 2014
PR angular#5547 introduced conversion of all 0 status codes to 404 for cases
where no response was recieved (previously this was done for the
file:// protocol only). But this mechanism is too eager and
masks legitimate cases where status 0 should be returned. This commits
reverts to the previous mechanism of handling 0 status code for the
file:// protocol (converting 0 to 404) while retaining the returned
status code 0 for all the protocols other than file://

Fixes angular#6074
Fixes angular#6155
pkozlowski-opensource added a commit that referenced this pull request Mar 14, 2014
PR #5547 introduced conversion of all 0 status codes to 404 for cases
where no response was recieved (previously this was done for the
file:// protocol only). But this mechanism is too eager and
masks legitimate cases where status 0 should be returned. This commits
reverts to the previous mechanism of handling 0 status code for the
file:// protocol (converting 0 to 404) while retaining the returned
status code 0 for all the protocols other than file://

Fixes #6074
Fixes #6155
pkozlowski-opensource added a commit that referenced this pull request Mar 14, 2014
PR #5547 introduced conversion of all 0 status codes to 404 for cases
where no response was recieved (previously this was done for the
file:// protocol only). But this mechanism is too eager and
masks legitimate cases where status 0 should be returned. This commits
reverts to the previous mechanism of handling 0 status code for the
file:// protocol (converting 0 to 404) while retaining the returned
status code 0 for all the protocols other than file://

Fixes #6074
Fixes #6155
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants