From 940bedf75ff7cb5cfe057493bb4d33c453b92ffa Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 7 Jul 2015 21:48:57 +0200 Subject: [PATCH 1/2] Add a unit-test that attempts to fetch a non-existent named destination Doing this helped uncover an issue with the `getDestination` implementation. Currently if a named destination doesn't exist, the method (in `obj.js`) may return `undefined` which leads to the promise being stuck in a pending state. *Note:* returning `null` for this case is consistent with other methods, e.g. `getOutline` and `getAttachments`. --- src/core/obj.js | 2 +- test/unit/api_spec.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/obj.js b/src/core/obj.js index 7e3192987..bcb9c9054 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -485,7 +485,7 @@ var Catalog = (function CatalogClosure() { } var xref = this.xref; - var dest, nameTreeRef, nameDictionaryRef; + var dest = null, nameTreeRef, nameDictionaryRef; var obj = this.catDict.get('Names'); if (obj && obj.has('Dests')) { nameTreeRef = obj.getRaw('Dests'); diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 5064e3f27..895039beb 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -129,6 +129,12 @@ describe('api', function() { 0, 841.89, null]); }); }); + it('gets a non-existent destination', function() { + var promise = doc.getDestination('non-existent-named-destination'); + waitsForPromiseResolved(promise, function(data) { + expect(data).toEqual(null); + }); + }); it('gets attachments', function() { var promise = doc.getAttachments(); waitsForPromiseResolved(promise, function (data) { From 7df78f997ec67e6e3810b14d57669c1fb97861e1 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 8 Jul 2015 15:31:06 +0200 Subject: [PATCH 2/2] Slightly more efficient `getDestination` For named destinations that are contained in a `Dict`, as opposed to a `NameTree`, we currently iterate through the *entire* dictionary just to fetch *one* destination. This code appears to simply have been copy-pasted from the `get destinations` method, but in its current form it's quite unnecessary/inefficient since can just get the required destination directly instead. --- src/core/obj.js | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/core/obj.js b/src/core/obj.js index bcb9c9054..f2bfdb635 100644 --- a/src/core/obj.js +++ b/src/core/obj.js @@ -493,17 +493,11 @@ var Catalog = (function CatalogClosure() { nameDictionaryRef = this.catDict.get('Dests'); } - if (nameDictionaryRef) { - // reading simple destination dictionary - obj = nameDictionaryRef; - obj.forEach(function catalogForEach(key, value) { - if (!value) { - return; - } - if (key === destinationId) { - dest = fetchDestination(value); - } - }); + if (nameDictionaryRef) { // Simple destination dictionary. + var value = nameDictionaryRef.get(destinationId); + if (value) { + dest = fetchDestination(value); + } } if (nameTreeRef) { var nameTree = new NameTree(nameTreeRef, xref);