Skip to content

Commit

Permalink
Merge pull request #17813 from Snuffleupagus/initPassiveLoading-simplify
Browse files Browse the repository at this point in the history
Remove `PDFViewerApplication.initPassiveLoading` and directly invoke the `open`-method from the extension-specific code
  • Loading branch information
timvandermeij authored Mar 22, 2024
2 parents 067c49d + ea1c910 commit e7203f5
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 71 deletions.
86 changes: 29 additions & 57 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,9 @@ const PDFViewerApplication = {
this._hideViewBookmark();
}
} else if (PDFJSDev.test("MOZCENTRAL || CHROME")) {
this.initPassiveLoading(file);
this.setTitleUsingUrl(file, /* downloadUrl = */ file);

this.externalServices.initPassiveLoading();
} else {
throw new Error("Not implemented: run");
}
Expand Down Expand Up @@ -811,39 +813,6 @@ const PDFViewerApplication = {
this._caretBrowsing.moveCaret(isUp, select);
},

initPassiveLoading(file) {
if (
typeof PDFJSDev === "undefined" ||
!PDFJSDev.test("MOZCENTRAL || CHROME")
) {
throw new Error("Not implemented: initPassiveLoading");
}
this.setTitleUsingUrl(file, /* downloadUrl = */ file);

this.externalServices.initPassiveLoading({
onOpenWithTransport: range => {
this.open({ range });
},
onOpenWithData: (data, contentDispositionFilename) => {
if (isPdfFile(contentDispositionFilename)) {
this._contentDispositionFilename = contentDispositionFilename;
}
this.open({ data });
},
onOpenWithURL: (url, length, originalUrl) => {
this.open({ url, length, originalUrl });
},
onError: err => {
this.l10n.get("pdfjs-loading-error").then(msg => {
this._documentError(msg, err);
});
},
onProgress: (loaded, total) => {
this.progress(loaded / total);
},
});
},

setTitleUsingUrl(url = "", downloadUrl = null) {
this.url = url;
this.baseUrl = url.split("#", 1)[0];
Expand Down Expand Up @@ -985,10 +954,11 @@ const PDFViewerApplication = {
const workerParams = AppOptions.getAll(OptionKind.WORKER);
Object.assign(GlobalWorkerOptions, workerParams);

if (
(typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) &&
args.url
) {
if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
if (args.data && isPdfFile(args.filename)) {
this._contentDispositionFilename = args.filename;
}
} else if (args.url) {
// The Firefox built-in viewer always calls `setTitleUsingUrl`, before
// `initPassiveLoading`, and it never provides an `originalUrl` here.
this.setTitleUsingUrl(
Expand Down Expand Up @@ -1046,10 +1016,11 @@ const PDFViewerApplication = {
} else if (reason instanceof UnexpectedResponseException) {
key = "pdfjs-unexpected-response-error";
}
return this.l10n.get(key).then(msg => {
this._documentError(msg, { message: reason?.message });
throw reason;
});
return this._documentError(key, { message: reason.message }).then(
() => {
throw reason;
}
);
}
);
},
Expand Down Expand Up @@ -1130,10 +1101,13 @@ const PDFViewerApplication = {
* Report the error; used for errors affecting loading and/or parsing of
* the entire PDF document.
*/
_documentError(message, moreInfo = null) {
async _documentError(key, moreInfo = null) {
this._unblockDocumentLoadEvent();

this._otherError(message, moreInfo);
const message = await this._otherError(
key || "pdfjs-loading-error",
moreInfo
);

this.eventBus.dispatch("documenterror", {
source: this,
Expand All @@ -1144,12 +1118,15 @@ const PDFViewerApplication = {

/**
* Report the error; used for errors affecting e.g. only a single page.
* @param {string} message - A message that is human readable.
* @param {string} key - The localization key for the error.
* @param {Object} [moreInfo] - Further information about the error that is
* more technical. Should have a 'message' and
* optionally a 'stack' property.
* @returns {string} A (localized) error message that is human readable.
*/
_otherError(message, moreInfo = null) {
async _otherError(key, moreInfo = null) {
const message = await this.l10n.get(key);

const moreInfoText = [`PDF.js v${version || "?"} (build: ${build || "?"})`];
if (moreInfo) {
moreInfoText.push(`Message: ${moreInfo.message}`);
Expand All @@ -1167,6 +1144,7 @@ const PDFViewerApplication = {
}

console.error(`${message}\n\n${moreInfoText.join("\n")}`);
return message;
},

progress(level) {
Expand Down Expand Up @@ -1383,9 +1361,7 @@ const PDFViewerApplication = {
this._initializeAutoPrint(pdfDocument, openActionPromise);
},
reason => {
this.l10n.get("pdfjs-loading-error").then(msg => {
this._documentError(msg, { message: reason?.message });
});
this._documentError("pdfjs-loading-error", { message: reason.message });
}
);

Expand Down Expand Up @@ -1774,9 +1750,7 @@ const PDFViewerApplication = {
}

if (!this.supportsPrinting) {
this.l10n.get("pdfjs-printing-not-supported").then(msg => {
this._otherError(msg);
});
this._otherError("pdfjs-printing-not-supported");
return;
}

Expand Down Expand Up @@ -2238,8 +2212,8 @@ if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
throw new Error("file origin does not match viewer's");
}
} catch (ex) {
PDFViewerApplication.l10n.get("pdfjs-loading-error").then(msg => {
PDFViewerApplication._documentError(msg, { message: ex?.message });
PDFViewerApplication._documentError("pdfjs-loading-error", {
message: ex.message,
});
throw ex;
}
Expand Down Expand Up @@ -2304,9 +2278,7 @@ function webViewerPageRendered({ pageNumber, error }) {
}

if (error) {
PDFViewerApplication.l10n.get("pdfjs-rendering-error").then(msg => {
PDFViewerApplication._otherError(msg, error);
});
PDFViewerApplication._otherError("pdfjs-rendering-error", error);
}
}

Expand Down
14 changes: 7 additions & 7 deletions web/chromecom.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,11 @@ const ChromeCom = {
// Even without this check, the file load in frames is still blocked,
// but this may change in the future (https://crbug.com/550151).
if (origin && !/^file:|^chrome-extension:/.test(origin)) {
viewerApp._documentError(
`Blocked ${origin} from loading ${file}. Refused to load ` +
"a local file in a non-local page for security reasons."
);
viewerApp._documentError(null, {
message:
`Blocked ${origin} from loading ${file}. Refused to load ` +
"a local file in a non-local page for security reasons.",
});
return;
}
isAllowedFileSchemeAccess(function (isAllowedAccess) {
Expand Down Expand Up @@ -416,12 +417,11 @@ class Preferences extends BasePreferences {
}

class ExternalServices extends BaseExternalServices {
initPassiveLoading(callbacks) {
// defaultUrl is set in viewer.js
initPassiveLoading() {
ChromeCom.resolvePDFFile(
AppOptions.get("defaultUrl"),
function (url, length, originalUrl) {
callbacks.onOpenWithURL(url, length, originalUrl);
viewerApp.open({ url, length, originalUrl });
}
);
}
Expand Down
2 changes: 1 addition & 1 deletion web/external_services.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class BaseExternalServices {

updateFindMatchesCount(data) {}

initPassiveLoading(callbacks) {}
initPassiveLoading() {}

reportTelemetry(data) {}

Expand Down
12 changes: 6 additions & 6 deletions web/firefoxcom.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class ExternalServices extends BaseExternalServices {
FirefoxCom.request("updateFindMatchesCount", data);
}

initPassiveLoading(callbacks) {
initPassiveLoading() {
let pdfDataRangeTransport;

window.addEventListener("message", function windowMessage(e) {
Expand All @@ -340,7 +340,7 @@ class ExternalServices extends BaseExternalServices {
switch (args.pdfjsLoadAction) {
case "supportsRangedLoading":
if (args.done && !args.data) {
callbacks.onError();
viewerApp._documentError(null);
break;
}
pdfDataRangeTransport = new FirefoxComDataRangeTransport(
Expand All @@ -350,7 +350,7 @@ class ExternalServices extends BaseExternalServices {
args.filename
);

callbacks.onOpenWithTransport(pdfDataRangeTransport);
viewerApp.open({ range: pdfDataRangeTransport });
break;
case "range":
pdfDataRangeTransport.onDataRange(args.begin, args.chunk);
Expand All @@ -369,14 +369,14 @@ class ExternalServices extends BaseExternalServices {
pdfDataRangeTransport?.onDataProgressiveDone();
break;
case "progress":
callbacks.onProgress(args.loaded, args.total);
viewerApp.progress(args.loaded / args.total);
break;
case "complete":
if (!args.data) {
callbacks.onError(args.errorCode);
viewerApp._documentError(null, { message: args.errorCode });
break;
}
callbacks.onOpenWithData(args.data, args.filename);
viewerApp.open({ data: args.data, filename: args.filename });
break;
}
});
Expand Down

0 comments on commit e7203f5

Please sign in to comment.