Remove the disableCreateObjectURL option from web/app_options.js

Prior to PR 11601, the `disableCreateObjectURL` option was present on `getDocument` in the API, since it was (potentially) used when decoding JPEG images natively in the browser. Hence setting this option, which was done automatically using compatibility-code, were in some browsers necessary in order for e.g. JPEG images to be correctly rendered.

The downside of the `disableCreateObjectURL` option is that memory usage increases significantly, since we're forced to build and use `data:` URIs (rather than `blob:` URLs).
However, at this point in time the `disableCreateObjectURL` option is only necessary for *some* (non-essential) functionality in the default viewer; in particular:
 - The openfile functionality, used only when manually opening a new file in the default viewer.
 - The download functionality, used when downloading either the PDF document itself or its attached files (if such exists).
 - The print functionality, in the generic `PDFPrintService` implementation.

Hence neither the general PDF.js library, nor the *basic* functionality of the default viewer, depends on the `disableCreateObjectURL` option any more; which is why I'm thus proposing that we remove the option since using it is a performance footgun.

*Please note:* To not outright break currently "supported" browsers, which lack proper `URL.createObjectURL` support, this patch purposely keeps the compatibility-code to explicitly disable `URL.createObjectURL` usage *only* for browsers which are known to not work correctly.[1]

While it's certainly possible that there's additional, likely older, browsers with broken `URL.createObjectURL` support, the last time that these types of problems were reported was over *three* years ago.[2]
Hence in the *very* unlikely event that additional problems occur, as a result of these changes, we can either add a new case in the compatibility-code or simply declare the affected browser as unsupported.

---
[1] Which are IE11 (see issue 3977), and Google Chrome on iOS (see PR 8081).

[2] Given that `URL.createObjectURL` is used by default, you'd really expect more reports if these problems were widespread.
This commit is contained in:
Jonas Jenwald 2020-08-06 12:43:59 +02:00
parent 6620861c7d
commit 5b94ed5487
6 changed files with 12 additions and 27 deletions

View File

@ -72,6 +72,7 @@ import { PDFThumbnailViewer } from "./pdf_thumbnail_viewer.js";
import { PDFViewer } from "./pdf_viewer.js";
import { SecondaryToolbar } from "./secondary_toolbar.js";
import { Toolbar } from "./toolbar.js";
import { viewerCompatibilityParams } from "./viewer_compatibility.js";
import { ViewHistory } from "./view_history.js";
const DEFAULT_SCALE_DELTA = 1.1;
@ -408,9 +409,7 @@ const PDFViewerApplication = {
});
this.pdfLinkService = pdfLinkService;
const downloadManager = this.externalServices.createDownloadManager({
disableCreateObjectURL: AppOptions.get("disableCreateObjectURL"),
});
const downloadManager = this.externalServices.createDownloadManager();
this.downloadManager = downloadManager;
const findController = new PDFFindController({
@ -2225,7 +2224,7 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
}
const file = evt.fileInput.files[0];
if (!AppOptions.get("disableCreateObjectURL")) {
if (!viewerCompatibilityParams.disableCreateObjectURL) {
let url = URL.createObjectURL(file);
if (file.name) {
url = { url, originalUrl: file.name };

View File

@ -42,12 +42,6 @@ const defaultOptions = {
value: "",
kind: OptionKind.VIEWER + OptionKind.PREFERENCE,
},
disableCreateObjectURL: {
/** @type {boolean} */
value: false,
compatibility: viewerCompatibilityParams.disableCreateObjectURL,
kind: OptionKind.VIEWER,
},
disableHistory: {
/** @type {boolean} */
value: false,

View File

@ -23,9 +23,6 @@ if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("CHROME || GENERIC")) {
);
}
const DISABLE_CREATE_OBJECT_URL =
viewerCompatibilityParams.disableCreateObjectURL || false;
function download(blobUrl, filename) {
const a = document.createElement("a");
if (!a.click) {
@ -46,10 +43,6 @@ function download(blobUrl, filename) {
}
class DownloadManager {
constructor({ disableCreateObjectURL = DISABLE_CREATE_OBJECT_URL }) {
this.disableCreateObjectURL = disableCreateObjectURL;
}
downloadUrl(url, filename) {
if (!createValidAbsoluteUrl(url, "http://example.com")) {
return; // restricted/invalid URL
@ -66,7 +59,7 @@ class DownloadManager {
const blobUrl = createObjectURL(
data,
contentType,
this.disableCreateObjectURL
viewerCompatibilityParams.disableCreateObjectURL
);
download(blobUrl, filename);
}
@ -80,7 +73,7 @@ class DownloadManager {
return;
}
if (this.disableCreateObjectURL) {
if (viewerCompatibilityParams.disableCreateObjectURL) {
// URL.createObjectURL is not supported
this.downloadUrl(url, filename);
return;

View File

@ -88,10 +88,6 @@ const FirefoxCom = (function FirefoxComClosure() {
})();
class DownloadManager {
constructor(options) {
this.disableCreateObjectURL = false;
}
downloadUrl(url, filename) {
FirefoxCom.request("download", {
originalUrl: url,

View File

@ -15,6 +15,7 @@
import { createPromiseCapability, getFilenameFromUrl } from "pdfjs-lib";
import { BaseTreeViewer } from "./base_tree_viewer.js";
import { viewerCompatibilityParams } from "./viewer_compatibility.js";
/**
* @typedef {Object} PDFAttachmentViewerOptions
@ -169,7 +170,7 @@ class PDFAttachmentViewer extends BaseTreeViewer {
const element = document.createElement("a");
if (
/\.pdf$/i.test(filename) &&
!this.downloadManager.disableCreateObjectURL
!viewerCompatibilityParams.disableCreateObjectURL
) {
this._bindPdfLink(element, { content: item.content, filename });
} else {

View File

@ -15,7 +15,7 @@
import { CSS_UNITS, NullL10n } from "./ui_utils.js";
import { PDFPrintServiceFactory, PDFViewerApplication } from "./app.js";
import { AppOptions } from "./app_options.js";
import { viewerCompatibilityParams } from "./viewer_compatibility.js";
let activeService = null;
let overlayManager = null;
@ -78,7 +78,6 @@ function PDFPrintService(
this.printContainer = printContainer;
this._printResolution = printResolution || 150;
this.l10n = l10n || NullL10n;
this.disableCreateObjectURL = AppOptions.get("disableCreateObjectURL");
this.currentPage = -1;
// The temporary canvas where renderPage paints one page at a time.
this.scratchCanvas = document.createElement("canvas");
@ -188,7 +187,10 @@ PDFPrintService.prototype = {
img.style.height = printItem.height;
const scratchCanvas = this.scratchCanvas;
if ("toBlob" in scratchCanvas && !this.disableCreateObjectURL) {
if (
"toBlob" in scratchCanvas &&
!viewerCompatibilityParams.disableCreateObjectURL
) {
scratchCanvas.toBlob(function (blob) {
img.src = URL.createObjectURL(blob);
});