From b05f053287d04ef66c197074f6de6856d8b92071 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 5 Dec 2018 20:09:15 +0100 Subject: [PATCH 1/2] [api-minor] Add support for OpenAction destinations (issue 10332) Note that the OpenAction dictionary may contain other information besides just a destination array, e.g. instructions for auto-printing[1]. Given first of all that an arbitrary `Dict` cannot be sent from the Worker (since cloning would fail), and second of all that the data obviously needs to be validated, this patch purposely only adds support for fetching a destination from the OpenAction entry[2]. --- [1] This information is, currently in PDF.js, being included through the `getJavaScript` API method. [2] This significantly reduces the complexity of the implementation, which seems fine for now. If there's ever need for other kinds of OpenAction to be fetched, additional API methods could/should be implemented as necessary (could e.g. follow the `getOpenActionWhatever` naming scheme). --- src/core/obj.js | 22 ++++++++++++++++++++++ src/core/worker.js | 4 ++++ src/display/api.js | 13 +++++++++++++ test/unit/api_spec.js | 18 ++++++++++++++++++ 4 files changed, 57 insertions(+) diff --git a/src/core/obj.js b/src/core/obj.js index 7a875a216..700a4ca35 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -392,6 +392,28 @@ class Catalog { return shadow(this, 'pageMode', pageMode); } + get openActionDestination() { + const obj = this.catDict.get('OpenAction'); + let openActionDest = null; + + if (isDict(obj)) { + // Convert the OpenAction dictionary into a format that works with + // `parseDestDictionary`, to avoid having to re-implement those checks. + const destDict = new Dict(this.xref); + destDict.set('A', obj); + + const resultObj = { url: null, dest: null, }; + Catalog.parseDestDictionary({ destDict, resultObj, }); + + if (Array.isArray(resultObj.dest)) { + openActionDest = resultObj.dest; + } + } else if (Array.isArray(obj)) { + openActionDest = obj; + } + return shadow(this, 'openActionDestination', openActionDest); + } + get attachments() { const obj = this.catDict.get('Names'); let attachments = null; diff --git a/src/core/worker.js b/src/core/worker.js index 1d525d0da..df14cd3af 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -531,6 +531,10 @@ var WorkerMessageHandler = { return pdfManager.ensureCatalog('pageMode'); }); + handler.on('getOpenActionDestination', function(data) { + return pdfManager.ensureCatalog('openActionDestination'); + }); + handler.on('GetAttachments', function wphSetupGetAttachments(data) { return pdfManager.ensureCatalog('attachments'); diff --git a/src/display/api.js b/src/display/api.js index 6a75d8288..c89930ff1 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -650,6 +650,14 @@ class PDFDocumentProxy { return this._transport.getPageMode(); } + /** + * @return {Promise} A promise that is resolved with an {Array} containing the + * destination, or `null` when no open action is present in the PDF file. + */ + getOpenActionDestination() { + return this._transport.getOpenActionDestination(); + } + /** * @return {Promise} A promise that is resolved with a lookup table for * mapping named attachments to their content. @@ -2152,6 +2160,11 @@ class WorkerTransport { return this.messageHandler.sendWithPromise('GetPageMode', null); } + getOpenActionDestination() { + return this.messageHandler.sendWithPromise('getOpenActionDestination', + null); + } + getAttachments() { return this.messageHandler.sendWithPromise('GetAttachments', null); } diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 4ae03fbf0..021a361f9 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -650,6 +650,24 @@ describe('api', function() { }).catch(done.fail); }); + it('gets default open action destination', function(done) { + var loadingTask = getDocument(buildGetDocumentParams('tracemonkey.pdf')); + + loadingTask.promise.then(function(pdfDocument) { + return pdfDocument.getOpenActionDestination(); + }).then(function(dest) { + expect(dest).toEqual(null); + + loadingTask.destroy().then(done); + }).catch(done.fail); + }); + it('gets non-default open action destination', function(done) { + doc.getOpenActionDestination().then(function(dest) { + expect(dest).toEqual([{ num: 15, gen: 0, }, { name: 'FitH', }, null]); + done(); + }).catch(done.fail); + }); + it('gets non-existent attachments', function(done) { var promise = doc.getAttachments(); promise.then(function (data) { From a7e70a50f5e59a8673731d66825a77210542c486 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 6 Dec 2018 18:32:41 +0100 Subject: [PATCH 2/2] Add OpenAction destination support, off by default, to the viewer Given that it's really not clear to me if this is actually desired functionality in the default viewer, and considering that it doesn't fit in *great* with the way that `PDFHistory` is initialized, this feature is currently off by default[1]. --- [1] It's controlled with the `disableOpenActionDestination` Preference/AppOption. --- extensions/chromium/preferences_schema.json | 4 ++++ web/app.js | 17 ++++++++++++--- web/app_options.js | 5 +++++ web/default_preferences.json | 1 + web/interfaces.js | 2 +- web/pdf_history.js | 23 +++++++++++++++------ 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json index 696090120..019577cfd 100644 --- a/extensions/chromium/preferences_schema.json +++ b/extensions/chromium/preferences_schema.json @@ -117,6 +117,10 @@ ], "default": 0 }, + "disableOpenActionDestination": { + "type": "boolean", + "default": true + }, "disablePageLabels": { "type": "boolean", "default": false diff --git a/web/app.js b/web/app.js index a4229a81d..420125a87 100644 --- a/web/app.js +++ b/web/app.js @@ -867,7 +867,9 @@ let PDFViewerApplication = { // Since the `setInitialView` call below depends on this being resolved, // fetch it early to avoid delaying initial rendering of the PDF document. - let pageModePromise = pdfDocument.getPageMode().catch( + const pageModePromise = pdfDocument.getPageMode().catch( + function() { /* Avoid breaking initial rendering; ignoring errors. */ }); + const openActionDestPromise = pdfDocument.getOpenActionDestination().catch( function() { /* Avoid breaking initial rendering; ignoring errors. */ }); this.toolbar.setPagesCount(pdfDocument.numPages, false); @@ -923,8 +925,17 @@ let PDFViewerApplication = { }).catch(() => { /* Unable to read from storage; ignoring errors. */ }); Promise.all([ - storePromise, pageModePromise, - ]).then(async ([values = {}, pageMode]) => { + storePromise, pageModePromise, openActionDestPromise, + ]).then(async ([values = {}, pageMode, openActionDest]) => { + if (openActionDest && !this.initialBookmark && + !AppOptions.get('disableOpenActionDestination')) { + // Always let the browser history/document hash take precedence. + this.initialBookmark = JSON.stringify(openActionDest); + // TODO: Re-factor the `PDFHistory` initialization to remove this hack + // that's currently necessary to prevent weird initial history state. + this.pdfHistory.push({ explicitDest: openActionDest, + pageNumber: null, }); + } const initialBookmark = this.initialBookmark; // Initialize the default values, from user preferences. const zoom = AppOptions.get('defaultZoomValue'); diff --git a/web/app_options.js b/web/app_options.js index 220b28455..bbd8e6c82 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -48,6 +48,11 @@ const defaultOptions = { value: false, kind: OptionKind.VIEWER, }, + disableOpenActionDestination: { + /** @type {boolean} */ + value: true, + kind: OptionKind.VIEWER, + }, disablePageLabels: { /** @type {boolean} */ value: false, diff --git a/web/default_preferences.json b/web/default_preferences.json index 3389a5cf8..f41aa1487 100644 --- a/web/default_preferences.json +++ b/web/default_preferences.json @@ -16,6 +16,7 @@ "renderer": "canvas", "renderInteractiveForms": false, "enablePrintAutoRotate": false, + "disableOpenActionDestination": true, "disablePageMode": false, "disablePageLabels": false, "scrollModeOnLoad": 0, diff --git a/web/interfaces.js b/web/interfaces.js index 98fe4bd2d..a7f58a2c8 100644 --- a/web/interfaces.js +++ b/web/interfaces.js @@ -94,7 +94,7 @@ class IPDFHistory { /** * @param {Object} params */ - push({ namedDest, explicitDest, pageNumber, }) {} + push({ namedDest = null, explicitDest, pageNumber, }) {} pushCurrentPosition() {} diff --git a/web/pdf_history.js b/web/pdf_history.js index 7c02dbe34..cbf8f3869 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -158,16 +158,27 @@ class PDFHistory { * Push an internal destination to the browser history. * @param {PushParameters} */ - push({ namedDest, explicitDest, pageNumber, }) { + push({ namedDest = null, explicitDest, pageNumber, }) { if (!this.initialized) { return; } - if ((namedDest && typeof namedDest !== 'string') || - !Array.isArray(explicitDest) || - !(Number.isInteger(pageNumber) && - pageNumber > 0 && pageNumber <= this.linkService.pagesCount)) { - console.error('PDFHistory.push: Invalid parameters.'); + if (namedDest && typeof namedDest !== 'string') { + console.error('PDFHistory.push: ' + + `"${namedDest}" is not a valid namedDest parameter.`); return; + } else if (!Array.isArray(explicitDest)) { + console.error('PDFHistory.push: ' + + `"${explicitDest}" is not a valid explicitDest parameter.`); + return; + } else if (!(Number.isInteger(pageNumber) && + pageNumber > 0 && pageNumber <= this.linkService.pagesCount)) { + // Allow an unset `pageNumber` if and only if the history is still empty; + // please refer to the `this._destination.page = null;` comment above. + if (pageNumber !== null || this._destination) { + console.error('PDFHistory.push: ' + + `"${pageNumber}" is not a valid pageNumber parameter.`); + return; + } } let hash = namedDest || JSON.stringify(explicitDest);