Skip to content

Commit

Permalink
Remove the sourceEventType from the viewer (bug 1757771 follow-up)
Browse files Browse the repository at this point in the history
After the changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1757771, that simplified the MOZCENTRAL downloading code, the `sourceEventType` is now completely unused and should thus be removed (in my opinion).

Furthermore, with these changes, we also no longer need a *separate* internal "save"-event and can instead just use the older "download"-event everywhere.
  • Loading branch information
Snuffleupagus committed May 15, 2022
1 parent 60e9065 commit 4f1cd6a
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 29 deletions.
27 changes: 11 additions & 16 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ const PDFViewerApplication = {
) {
try {
// Trigger saving, to prevent data loss in forms; see issue 12257.
await this.save({ sourceEventType: "save" });
await this.save();
} catch (reason) {
// Ignoring errors, to ensure that document closing won't break.
}
Expand Down Expand Up @@ -964,7 +964,7 @@ const PDFViewerApplication = {
throw new Error("PDF document not downloaded.");
},

async download({ sourceEventType = "download" } = {}) {
async download() {
const url = this._downloadUrl,
filename = this._docFilename;
try {
Expand All @@ -973,15 +973,15 @@ const PDFViewerApplication = {
const data = await this.pdfDocument.getData();
const blob = new Blob([data], { type: "application/pdf" });

await this.downloadManager.download(blob, url, filename, sourceEventType);
await this.downloadManager.download(blob, url, filename);
} catch (reason) {
// When the PDF document isn't ready, or the PDF file is still
// downloading, simply download using the URL.
await this.downloadManager.downloadUrl(url, filename);
}
},

async save({ sourceEventType = "download" } = {}) {
async save() {
if (this._saveInProgress) {
return;
}
Expand All @@ -996,23 +996,23 @@ const PDFViewerApplication = {
const data = await this.pdfDocument.saveDocument();
const blob = new Blob([data], { type: "application/pdf" });

await this.downloadManager.download(blob, url, filename, sourceEventType);
await this.downloadManager.download(blob, url, filename);
} catch (reason) {
// When the PDF document isn't ready, or the PDF file is still
// downloading, simply fallback to a "regular" download.
console.error(`Error when saving the document: ${reason.message}`);
await this.download({ sourceEventType });
await this.download();
} finally {
await this.pdfScriptingManager.dispatchDidSave();
this._saveInProgress = false;
}
},

downloadOrSave(options) {
downloadOrSave() {
if (this.pdfDocument?.annotationStorage.size > 0) {
this.save(options);
this.save();
} else {
this.download(options);
this.download();
}
},

Expand Down Expand Up @@ -1875,7 +1875,6 @@ const PDFViewerApplication = {
eventBus._on("presentationmode", webViewerPresentationMode);
eventBus._on("print", webViewerPrint);
eventBus._on("download", webViewerDownload);
eventBus._on("save", webViewerSave);
eventBus._on("firstpage", webViewerFirstPage);
eventBus._on("lastpage", webViewerLastPage);
eventBus._on("nextpage", webViewerNextPage);
Expand Down Expand Up @@ -1973,7 +1972,6 @@ const PDFViewerApplication = {
eventBus._off("presentationmode", webViewerPresentationMode);
eventBus._off("print", webViewerPrint);
eventBus._off("download", webViewerDownload);
eventBus._off("save", webViewerSave);
eventBus._off("firstpage", webViewerFirstPage);
eventBus._off("lastpage", webViewerLastPage);
eventBus._off("nextpage", webViewerNextPage);
Expand Down Expand Up @@ -2330,7 +2328,7 @@ function webViewerNamedAction(evt) {
break;

case "SaveAs":
webViewerSave();
PDFViewerApplication.downloadOrSave();
break;
}
}
Expand Down Expand Up @@ -2461,10 +2459,7 @@ function webViewerPrint() {
PDFViewerApplication.triggerPrinting();
}
function webViewerDownload() {
PDFViewerApplication.downloadOrSave({ sourceEventType: "download" });
}
function webViewerSave() {
PDFViewerApplication.downloadOrSave({ sourceEventType: "save" });
PDFViewerApplication.downloadOrSave();
}
function webViewerFirstPage() {
if (PDFViewerApplication.pdfDocument) {
Expand Down
8 changes: 1 addition & 7 deletions web/download_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,7 @@ class DownloadManager {
return false;
}

/**
* @param sourceEventType {string} Used to signal what triggered the download.
* The version of PDF.js integrated with Firefox uses this to to determine
* which dialog to show. "save" triggers "save as" and "download" triggers
* the "open with" dialog.
*/
download(blob, url, filename, sourceEventType = "download") {
download(blob, url, filename) {
const blobUrl = URL.createObjectURL(blob);
download(blobUrl, filename);
}
Expand Down
5 changes: 2 additions & 3 deletions web/firefoxcom.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,13 @@ class DownloadManager {
return false;
}

download(blob, url, filename, sourceEventType = "download") {
download(blob, url, filename) {
const blobUrl = URL.createObjectURL(blob);

FirefoxCom.requestAsync("download", {
blobUrl,
originalUrl: url,
filename,
sourceEventType,
}).then(error => {
if (error) {
// If downloading failed in `PdfStreamConverter.jsm` it's very unlikely
Expand Down Expand Up @@ -275,7 +274,7 @@ class MozL10n {
if (!PDFViewerApplication.initialized) {
return;
}
PDFViewerApplication.eventBus.dispatch(type, { source: window });
PDFViewerApplication.eventBus.dispatch("download", { source: window });
};

window.addEventListener("save", handleEvent);
Expand Down
3 changes: 1 addition & 2 deletions web/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,8 @@ class IDownloadManager {
* @param {Blob} blob
* @param {string} url
* @param {string} filename
* @param {string} [sourceEventType]
*/
download(blob, url, filename, sourceEventType = "download") {}
download(blob, url, filename) {}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion web/pdf_scripting_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ class PDFScriptingManager {
this._pdfViewer.currentScaleValue = value;
break;
case "SaveAs":
this._eventBus.dispatch("save", { source: this });
this._eventBus.dispatch("download", { source: this });
break;
case "FirstPage":
this._pdfViewer.currentPageNumber = 1;
Expand Down

0 comments on commit 4f1cd6a

Please sign in to comment.