From b05652ca977719529df509bb5891329d0aa698c4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sun, 22 Nov 2015 13:56:52 +0100 Subject: [PATCH] [api-minor] Let `getAnnotations` fetch all annotations by default, unless an intent is specified Currently `getAnnotations` will *only* fetch annotations that are either `viewable` or `printable`. This is "hidden" inside the `core.js` file, meaning that API consumers might be confused as to why they are not recieving *all* the annotations present for a page. I thus think that the API should, by default, return *all* available annotations unless specifically told otherwise. In e.g. the default viewer, we obviously only want to display annotations that are `viewable`, hence this patch adds an `intent` parameter to `getAnnotations` that makes it possible to decide if only `viewable` or `printable` annotations should be fetched. --- src/core/core.js | 10 ++++++++-- src/core/worker.js | 4 ++-- src/display/api.js | 31 ++++++++++++++++++++++++------- test/unit/api_spec.js | 14 ++++++++++++-- web/annotations_layer_builder.js | 9 +++++++-- web/pdf_page_view.js | 4 ++-- 6 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/core/core.js b/src/core/core.js index 502081d01..984c5e91e 100644 --- a/src/core/core.js +++ b/src/core/core.js @@ -252,10 +252,16 @@ var Page = (function PageClosure() { }); }, - getAnnotationsData: function Page_getAnnotationsData() { + getAnnotationsData: function Page_getAnnotationsData(intent) { var annotations = this.annotations; var annotationsData = []; for (var i = 0, n = annotations.length; i < n; ++i) { + if (intent) { + if (!(intent === 'display' && annotations[i].viewable) && + !(intent === 'print' && annotations[i].printable)) { + continue; + } + } annotationsData.push(annotations[i].data); } return annotationsData; @@ -268,7 +274,7 @@ var Page = (function PageClosure() { for (var i = 0, n = annotationRefs.length; i < n; ++i) { var annotationRef = annotationRefs[i]; var annotation = annotationFactory.create(this.xref, annotationRef); - if (annotation && (annotation.viewable || annotation.printable)) { + if (annotation) { annotations.push(annotation); } } diff --git a/src/core/worker.js b/src/core/worker.js index dd4bd9a29..1e80d5aab 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -399,7 +399,7 @@ var WorkerMessageHandler = PDFJS.WorkerMessageHandler = { handler.on('GetDestination', function wphSetupGetDestination(data) { - return pdfManager.ensureCatalog('getDestination', [ data.id ]); + return pdfManager.ensureCatalog('getDestination', [data.id]); } ); @@ -447,7 +447,7 @@ var WorkerMessageHandler = PDFJS.WorkerMessageHandler = { handler.on('GetAnnotations', function wphSetupGetAnnotations(data) { return pdfManager.getPage(data.pageIndex).then(function(page) { - return pdfManager.ensure(page, 'getAnnotationsData', []); + return pdfManager.ensure(page, 'getAnnotationsData', [data.intent]); }); }); diff --git a/src/display/api.js b/src/display/api.js index 450483e58..c9034fb48 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -649,6 +649,16 @@ var PDFDocumentProxy = (function PDFDocumentProxyClosure() { * @property {string} fontFamily - possible font family */ +/** + * Page annotation parameters. + * + * @typedef {Object} GetAnnotationsParameters + * @param {string} intent - Determines the annotations that will be fetched, + * can be either 'display' (viewable annotations) or 'print' + * (printable annotations). + * If the parameter is omitted, all annotations are fetched. + */ + /** * Page render parameters. * @@ -737,12 +747,17 @@ var PDFPageProxy = (function PDFPageProxyClosure() { return new PDFJS.PageViewport(this.view, scale, rotate, 0, 0); }, /** + * @param {GetAnnotationsParameters} params - Annotation parameters. * @return {Promise} A promise that is resolved with an {Array} of the * annotation objects. */ - getAnnotations: function PDFPageProxy_getAnnotations() { - if (!this.annotationsPromise) { - this.annotationsPromise = this.transport.getAnnotations(this.pageIndex); + getAnnotations: function PDFPageProxy_getAnnotations(params) { + var intent = (params && params.intent) || null; + + if (!this.annotationsPromise || this.annotationsIntent !== intent) { + this.annotationsPromise = this.transport.getAnnotations(this.pageIndex, + intent); + this.annotationsIntent = intent; } return this.annotationsPromise; }, @@ -1470,9 +1485,11 @@ var WorkerTransport = (function WorkerTransportClosure() { return this.messageHandler.sendWithPromise('GetPageIndex', { ref: ref }); }, - getAnnotations: function WorkerTransport_getAnnotations(pageIndex) { - return this.messageHandler.sendWithPromise('GetAnnotations', - { pageIndex: pageIndex }); + getAnnotations: function WorkerTransport_getAnnotations(pageIndex, intent) { + return this.messageHandler.sendWithPromise('GetAnnotations', { + pageIndex: pageIndex, + intent: intent, + }); }, getDestinations: function WorkerTransport_getDestinations() { @@ -1480,7 +1497,7 @@ var WorkerTransport = (function WorkerTransportClosure() { }, getDestination: function WorkerTransport_getDestination(id) { - return this.messageHandler.sendWithPromise('GetDestination', { id: id } ); + return this.messageHandler.sendWithPromise('GetDestination', { id: id }); }, getAttachments: function WorkerTransport_getAttachments() { diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index fa2f6ab5c..1bfa02f86 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -400,8 +400,18 @@ describe('api', function() { expect(viewport.height).toEqual(892.92); }); it('gets annotations', function () { - var promise = page.getAnnotations(); - waitsForPromiseResolved(promise, function (data) { + var defaultPromise = page.getAnnotations(); + waitsForPromiseResolved(defaultPromise, function (data) { + expect(data.length).toEqual(4); + }); + + var displayPromise = page.getAnnotations({ intent: 'display' }); + waitsForPromiseResolved(displayPromise, function (data) { + expect(data.length).toEqual(4); + }); + + var printPromise = page.getAnnotations({ intent: 'print' }); + waitsForPromiseResolved(printPromise, function (data) { expect(data.length).toEqual(4); }); }); diff --git a/web/annotations_layer_builder.js b/web/annotations_layer_builder.js index 752e43134..11ae32d54 100644 --- a/web/annotations_layer_builder.js +++ b/web/annotations_layer_builder.js @@ -45,9 +45,10 @@ var AnnotationsLayerBuilder = (function AnnotationsLayerBuilderClosure() { /** * @param {PageViewport} viewport + * @param {string} intent (default value is 'display') */ setupAnnotations: - function AnnotationsLayerBuilder_setupAnnotations(viewport) { + function AnnotationsLayerBuilder_setupAnnotations(viewport, intent) { function bindLink(link, dest) { link.href = linkService.getDestinationHash(dest); link.onclick = function annotationsLayerBuilderLinksOnclick() { @@ -73,8 +74,12 @@ var AnnotationsLayerBuilder = (function AnnotationsLayerBuilderClosure() { var linkService = this.linkService; var pdfPage = this.pdfPage; var self = this; + var getAnnotationsParams = { + intent: (intent === undefined ? 'display' : intent), + }; - pdfPage.getAnnotations().then(function (annotationsData) { + pdfPage.getAnnotations(getAnnotationsParams).then( + function (annotationsData) { viewport = viewport.clone({ dontFlip: true }); var transform = viewport.transform; var transformStr = 'matrix(' + transform.join(',') + ')'; diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 3f1a30c27..bfa5875ae 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -275,7 +275,7 @@ var PDFPageView = (function PDFPageViewClosure() { } if (redrawAnnotations && this.annotationLayer) { - this.annotationLayer.setupAnnotations(this.viewport); + this.annotationLayer.setupAnnotations(this.viewport, 'display'); } }, @@ -507,7 +507,7 @@ var PDFPageView = (function PDFPageViewClosure() { this.annotationLayer = this.annotationsLayerFactory. createAnnotationsLayerBuilder(div, this.pdfPage); } - this.annotationLayer.setupAnnotations(this.viewport); + this.annotationLayer.setupAnnotations(this.viewport, 'display'); } div.setAttribute('data-loaded', true);