From 775d45b36ac6b75795f55c143e21034ff4370a9a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 2 Jan 2021 12:02:58 +0100 Subject: [PATCH] Remove the `DownloadManager.onerror` functionality, since its only usage is unlikely to be helpful Note how the `onerror` functionality is not being used in the GENERIC `DownloadManager`, since we have no way of knowing if downloading succeeded. Hence this functionality is only *possibly* useful in MOZCENTRAL builds, however as outlined in the existing comments it's unlikely to be helpful in practice. Generally speaking, if downloading failed once in [`PdfStreamConverter.jsm`](https://searchfox.org/mozilla-central/rev/809ac3660845fef6faf18ec210232fdadc0f1ad9/toolkit/components/pdfjs/content/PdfStreamConverter.jsm#294-406) it seems very likely that it would fail again; all-in-all I'm thus suggesting that we just remove the `onerror` functionality altogether here. --- web/app.js | 10 ---------- web/download_manager.js | 1 - web/firefoxcom.js | 6 ++++-- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/web/app.js b/web/app.js index 2166e8f21..1c44dc548 100644 --- a/web/app.js +++ b/web/app.js @@ -993,11 +993,6 @@ const PDFViewerApplication = { const downloadManager = this.downloadManager, url = this.baseUrl, filename = this._docFilename; - downloadManager.onerror = err => { - // This error won't really be helpful because it's likely the - // fallback won't work either (or is already open). - this.error(`PDF failed to download: ${err}`); - }; // When the PDF document isn't ready, or the PDF file is still downloading, // simply download using the URL. @@ -1023,11 +1018,6 @@ const PDFViewerApplication = { const downloadManager = this.downloadManager, url = this.baseUrl, filename = this._docFilename; - downloadManager.onerror = err => { - // This error won't really be helpful because it's likely the - // fallback won't work either (or is already open). - this.error(`PDF failed to be saved: ${err}`); - }; // When the PDF document isn't ready, or the PDF file is still downloading, // simply download using the URL. diff --git a/web/download_manager.js b/web/download_manager.js index 3c287f856..0261e4580 100644 --- a/web/download_manager.js +++ b/web/download_manager.js @@ -71,7 +71,6 @@ class DownloadManager { this.downloadUrl(url, filename); return; } - const blobUrl = URL.createObjectURL(blob); download(blobUrl, filename); } diff --git a/web/firefoxcom.js b/web/firefoxcom.js index afca70122..5ea019c61 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -130,8 +130,10 @@ class DownloadManager { filename, sourceEventType, }).then(error => { - if (error && this.onerror) { - this.onerror(error); + if (error) { + // If downloading failed in `PdfStreamConverter.jsm` it's very unlikely + // that attempting to fallback and re-download would be helpful here. + console.error("`ChromeActions.download` failed."); } URL.revokeObjectURL(blobUrl); });