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

Don't onload / abort for 204 / 205 / "as download" responses #1382

Closed
annevk opened this issue Jun 3, 2016 · 9 comments · Fixed by #6315
Closed

Don't onload / abort for 204 / 205 / "as download" responses #1382

annevk opened this issue Jun 3, 2016 · 9 comments · Fixed by #6315
Assignees

Comments

@annevk
Copy link
Member

annevk commented Jun 3, 2016

@Manishearth mentioned this in #511, but I didn't address that part of the issue, assuming it would be handled by simply aborting the navigation steps. It seems however that we are doing certain checks too late and/or aborting is happening too early. Seems like we should at least wait for the HTTP headers of a response to come back before deciding to actually continue with navigation or terminating it.

@Manishearth
Copy link

This should also happen for content-disposition: attachment files (not sure if they get a separate status, I think they just use 200 OK)

@annevk annevk changed the title Don't onload / abort for 204/205 "as download" responses Don't onload / abort for 204 / 205 / "as download" responses Jun 3, 2016
@annevk
Copy link
Member Author

annevk commented Jun 9, 2016

I basically need to write tests here I think. E.g., do we unload a document before we hear from the server? The current specification says yes, but does that match reality?

@annevk
Copy link
Member Author

annevk commented Jun 27, 2016

Actually, I'm not sure this is correct. This would suggest that synchronously doing prepare to unload and abort is wrong, whereas that's what browsers are doing. Firefox seems very inconsistent though when it comes to onbeforeunload and such. Perhaps it only does it for top-level browsing contexts?

@ericf01
Copy link

ericf01 commented Dec 2, 2016

I'm having the same problem when posting a form that responds with a file download. The post cancels any in-progress AJAX requests issued by the current page. According to the current spec, the form post initiates navigation, which cancels the in-progress AJAX requests BEFORE receiving the status 200 response containing the content-disposition: attachment header. That header cancels the navigation, but can't reinstate the canceled AJAX requests. Based on extensive testing, both Google Chrome and MS Edge appear to have tweaked their algorithms to delay cancelation of the in-progress AJAX requests until the response headers can be examined. However, Mozilla Firefox obeys the published spec, thus experiencing the AJAX cancelation problem. Worse yet, if part of an AJAX JSON response has already been received when the form is posted, the post will truncate the response, resulting in a JSON parse error being thrown. I believe the official spec should be revised to work the way that Chrome and Edge have implemented, thus resolving this problem.

@annevk
Copy link
Member Author

annevk commented May 22, 2017

I was wrong above. We unload after (from a task in history), we prompt to unload (beforeunload) before (synchronously from navigate). So it might be that everything is in order here, but should probably test again to be sure.

@annevk
Copy link
Member Author

annevk commented May 22, 2017

@ericf01 I think your issue is slightly different, though also important to sort through and test. It's more related to #1334 (in particular the last comment). Tests for that would also be good.

@ericf01
Copy link

ericf01 commented May 22, 2017 via email

@annevk
Copy link
Member Author

annevk commented May 23, 2017

That's the same comment as far as I can tell. In any event, to make progress we need tests. A pointer to the where you discussed this would also be good (if it's a public bug report or mailing list or some such).

@ericf01
Copy link

ericf01 commented May 23, 2017

Yes, it's the same comment. I thought you wanted me to move it from #1382 to #1334.

A full discussion of the issue, involving a work-around for Mozilla Firefox from @bzbarsky including his suggestion that it be posted in #1382, can be reviewed at https://bugzilla.mozilla.org/show_bug.cgi?id=1315741.

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

Successfully merging a pull request may close this issue.

3 participants