Skip to content

Commit

Permalink
Re-factor the blob-URL caching in DownloadManager.openOrDownloadData
Browse files Browse the repository at this point in the history
Cache blob-URLs on the actual data, rather than DOM elements, to reduce potential duplicates (note the updated unit-test).
  • Loading branch information
Snuffleupagus committed Oct 17, 2023
1 parent 22d6d95 commit 245b6b7
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 18 deletions.
7 changes: 1 addition & 6 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,6 @@ class LinkAnnotationElement extends AnnotationElement {
link.href = this.linkService.getAnchorUrl("");
link.onclick = () => {
this.downloadManager?.openOrDownloadData(
this.container,
attachment.content,
attachment.filename,
dest
Expand Down Expand Up @@ -2861,11 +2860,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
* Download the file attachment associated with this annotation.
*/
#download() {
this.downloadManager?.openOrDownloadData(
this.container,
this.content,
this.filename
);
this.downloadManager?.openOrDownloadData(this.content, this.filename);
}
}

Expand Down
5 changes: 5 additions & 0 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2883,6 +2883,11 @@ describe("api", function () {

expect(attachmentDest).toEqual('[0,{"name":"Fit"}]');

// Check that the attachments, which are identical, aren't duplicated.
for (let i = 1, ii = annotations.length; i < ii; i++) {
expect(annotations[i].attachment).toBe(attachment);
}

await loadingTask.destroy();
});

Expand Down
8 changes: 4 additions & 4 deletions web/download_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,18 @@ class DownloadManager {
/**
* @returns {boolean} Indicating if the data was opened.
*/
openOrDownloadData(element, data, filename, dest = null) {
openOrDownloadData(data, filename, dest = null) {
const isPdfData = isPdfFile(filename);
const contentType = isPdfData ? "application/pdf" : "";

if (
(typeof PDFJSDev === "undefined" || !PDFJSDev.test("COMPONENTS")) &&
isPdfData
) {
let blobUrl = this.#openBlobUrls.get(element);
let blobUrl = this.#openBlobUrls.get(data);
if (!blobUrl) {
blobUrl = URL.createObjectURL(new Blob([data], { type: contentType }));
this.#openBlobUrls.set(element, blobUrl);
this.#openBlobUrls.set(data, blobUrl);
}
let viewerUrl;
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
Expand All @@ -105,7 +105,7 @@ class DownloadManager {
// Release the `blobUrl`, since opening it failed, and fallback to
// downloading the PDF file.
URL.revokeObjectURL(blobUrl);
this.#openBlobUrls.delete(element);
this.#openBlobUrls.delete(data);
}
}

Expand Down
8 changes: 4 additions & 4 deletions web/firefoxcom.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,15 @@ class DownloadManager {
/**
* @returns {boolean} Indicating if the data was opened.
*/
openOrDownloadData(element, data, filename, dest = null) {
openOrDownloadData(data, filename, dest = null) {
const isPdfData = isPdfFile(filename);
const contentType = isPdfData ? "application/pdf" : "";

if (isPdfData) {
let blobUrl = this.#openBlobUrls.get(element);
let blobUrl = this.#openBlobUrls.get(data);
if (!blobUrl) {
blobUrl = URL.createObjectURL(new Blob([data], { type: contentType }));
this.#openBlobUrls.set(element, blobUrl);
this.#openBlobUrls.set(data, blobUrl);
}
// Let Firefox's content handler catch the URL and display the PDF.
let viewerUrl = blobUrl + "?filename=" + encodeURIComponent(filename);
Expand All @@ -156,7 +156,7 @@ class DownloadManager {
// Release the `blobUrl`, since opening it failed, and fallback to
// downloading the PDF file.
URL.revokeObjectURL(blobUrl);
this.#openBlobUrls.delete(element);
this.#openBlobUrls.delete(data);
}
}

Expand Down
4 changes: 2 additions & 2 deletions web/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,12 @@ class IDownloadManager {
downloadData(data, filename, contentType) {}

/**
* @param {HTMLElement} element
* @param {Uint8Array} data
* @param {string} filename
* @param {string} [dest]
* @returns {boolean} Indicating if the data was opened.
*/
openOrDownloadData(element, data, filename) {}
openOrDownloadData(data, filename, dest = null) {}

/**
* @param {Blob} blob
Expand Down
2 changes: 1 addition & 1 deletion web/pdf_attachment_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class PDFAttachmentViewer extends BaseTreeViewer {
*/
_bindLink(element, { content, filename }) {
element.onclick = () => {
this.downloadManager.openOrDownloadData(element, content, filename);
this.downloadManager.openOrDownloadData(content, filename);
return false;
};
}
Expand Down
1 change: 0 additions & 1 deletion web/pdf_outline_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ class PDFOutlineViewer extends BaseTreeViewer {
element.href = linkService.getAnchorUrl("");
element.onclick = () => {
this.downloadManager.openOrDownloadData(
element,
attachment.content,
attachment.filename
);
Expand Down

0 comments on commit 245b6b7

Please sign in to comment.