downloads API: Allow extensions a way to download ignoring HTTP errors
Categories
(WebExtensions :: General, enhancement)
Tracking
(firefox71 fixed)
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: nmaier, Assigned: nmaier)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
16.14 KB,
patch
|
Details | Diff | Splinter Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Bug #1576333 will bring Firefox to parity with Chrome in regards to handling HTTP errors and cancelling downloads on HTTP errors.
A concern arose that some extensions might in fact want to continue with downloads even though the server returned an error such as 404 Not Found
.
This is currently unsupported in Chrome, and implementing this in Firefox would be a new feature.
I therefore propose the following changes:
- Add a new, optional boolean flag
allowHttpErrors
to theoptions
ofdownload.download()
, which defaults tofalse
. - When the flag is
false
(the default) in calls todownload.download()
, then there should be no change in behavior, and downloads should continue to be cancelled upon encountering HTTP errors. - When the flag is
true
in calls todownload.download()
, then upon encountering errors the download should be allowed to proceed regardless, butDownloadItem.error
should be set to the appropriateInterruptReason
, such asSERVER_BAD_CONTENT
, as usual, and special care should be taken that any.onChanged
listeners are notified of the changedDownloadItem.error
value. - When the flag is
true
in calls todownload.download()
, when the download finishes and changes its state tocomplete
, theDownloadItem.error
should still reflect theInterruptReason
corresponding to HTTP error code encountered earlier. - When the flag is
true
in calls todownload.download()
and there is a later error, such as writing the file to disk failed, then theDownloadItem.error
should reflect theInterruptReason
of this later error.
Assignee | ||
Comment 1•5 years ago
|
||
Preliminary patch, demonstrating the idea, tho I am not married to the details and open to suggestions.
It is supposed to stack on the patches from bug 1576333 of course.
Assignee | ||
Comment 2•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d12f40be9fc
Add allowHttpErrors feature to downloads API r=robwu
Comment 4•5 years ago
|
||
bugherder |
Updated•5 years ago
|
Comment 5•5 years ago
|
||
The following documentation has been updated:
- compatibility table data, the addition of the
allowHttpErrors
property, see pull request https://github.com/mdn/browser-compat-data/pull/5042. - downloads.download() added documentation of the
allowHttpErrors
property. - Release notes noted the addition of the
allowHttpErrors
property with basic description of features.
Please let me know if there are any changes or additions needed. Thank you!
Assignee | ||
Comment 6•5 years ago
|
||
Seems something went wrong with your downloads.dowload() change?
https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/downloads/download$compare?locale=en-US&to=1588216&from=1588068
Comment 7•5 years ago
|
||
Not sure what went wrong there, but fortunately I had my changes preserved in the preview page: downloads.dowload() Is now as I intended.
Assignee | ||
Comment 8•5 years ago
|
||
We eventually opted to not report at all HTTP errors when allowHttpErrors is true, i.e. the errors that bug 1576333 added are ignored. Other errors, such as FILE_* and NETWORK_* errors will be reported.
I'd replace the following phrase and the listing that follows:
Errors encountered during the download are reported: [...]
with something like:
HTTP server errors such as
SERVER_BAD_CONTENT
will be ignored entirely and are not reported, and the download is allowed to proceed, however other errors unrelated to HTTP server errors - such asFILE_FAILED
orNETWORK_FAILED
- still get reported and fail the download, as usual. Using this flag e.g. allows to deliberately download server error pages.
And I'd remove the following sentence entirely:
All .onChanged listeners are notified of the changed DownloadItem.error value.
Sorry, I forgot to mention this before and created extra work.
Comment 9•5 years ago
|
||
No worries, I have updated downloads.download. Please let me know if this is OK. Thanks!
Comment 11•5 years ago
|
||
(clearing needinfo, Nils already provided feedback on the updated MDN api docs).
Updated•5 years ago
|
Description
•