From e0ae1575827f2d86f9fe6186f82b74ec34a197cc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 17 Mar 2018 17:10:37 +0100 Subject: [PATCH] [api-minor] Fix various issues related to the pageSize information The `getPageSizeInches` method was implemented on `PDFDocumentProxy`, which seems conceptually wrong since the size property isn't global to the document but rather specific to each page. Hence the method is moved into `PDFPageProxy`, as `get pageSizeInches` instead to address this. Despite the fact that new API functionality was implemented, no unit-tests were added. To prevent issues later on, we should *always* ensure that new functionality has at least some test-coverage; something that this patch also takes care of. The new `PDFDocumentProperties._parsePageSize` method seemed unnecessary convoluted. Furthermore, in the "no data provided"-case it even returned incorrect data (an array, rather than the expected object). Finally, the fallback strings didn't actually agree with the `en-US` locale. This inconsistency doesn't look too great, and it's thus addressed here as well. --- src/display/api.js | 31 ++++++++++------------ test/unit/api_spec.js | 8 ++++++ web/pdf_document_properties.js | 47 +++++++++++++++++----------------- 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/src/display/api.js b/src/display/api.js index 0d2e316fe..203dbe10f 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -692,23 +692,6 @@ var PDFDocumentProxy = (function PDFDocumentProxyClosure() { getMetadata: function PDFDocumentProxy_getMetadata() { return this.transport.getMetadata(); }, - /** - * @param {number} pageNumber The page number to get the page size from. - * The first page is 1, which is also the default page used. - * @return {Promise} A promise that is resolved with an dict containing the - * width and height in inches. - */ - getPageSizeInches(pageNumber) { - pageNumber = pageNumber || 1; - return this.getPage(pageNumber).then((page) => { - const [x1, y1, x2, y2] = page.view; - // convert values from user units to inches - return { - width: (x2 - x1) / 72 * page.userUnit, - height: (y2 - y1) / 72 * page.userUnit, - }; - }); - }, /** * @return {Promise} A promise that is resolved with a TypedArray that has * the raw data from the PDF. @@ -890,6 +873,20 @@ var PDFPageProxy = (function PDFPageProxyClosure() { get view() { return this.pageInfo.view; }, + + /** + * The size of the current page, converted from PDF units to inches. + * @return {Object} An Object containing the properties: {number} `width` + * and {number} `height`, given in inches. + */ + get pageSizeInches() { + const [x1, y1, x2, y2] = this.view, userUnit = this.userUnit; + return { + width: (x2 - x1) / 72 * userUnit, + height: (y2 - y1) / 72 * userUnit, + }; + }, + /** * @param {number} scale The desired scale of the viewport. * @param {number} rotate Degrees to rotate the viewport. If omitted this diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 35e729283..ed4a809ea 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -964,6 +964,14 @@ describe('api', function() { it('gets view', function () { expect(page.view).toEqual([0, 0, 595.28, 841.89]); }); + + it('gets page size (in inches)', function() { + const { width, height, } = page.pageSizeInches; + + expect(+width.toPrecision(3)).toEqual(8.27); + expect(+height.toPrecision(4)).toEqual(11.69); + }); + it('gets viewport', function () { var viewport = page.getViewport(1.5, 90); expect(viewport.viewBox).toEqual(page.view); diff --git a/web/pdf_document_properties.js b/web/pdf_document_properties.js index 7f351592b..b4987b376 100644 --- a/web/pdf_document_properties.js +++ b/web/pdf_document_properties.js @@ -80,13 +80,12 @@ class PDFDocumentProperties { this._parseFileSize(this.maybeFileSize), this._parseDate(info.CreationDate), this._parseDate(info.ModDate), - this.pdfDocument.getPageSizeInches().then((pageSizeInches) => { - return this._parsePageSize(pageSizeInches); + this.pdfDocument.getPage(1).then((pdfPage) => { + return this._parsePageSize(pdfPage.pageSizeInches); }), - ]); - }).then(([info, metadata, fileName, fileSize, - creationDate, modDate, pageSize]) => { + }).then(([info, metadata, fileName, fileSize, creationDate, modDate, + pageSizes]) => { freezeFieldData({ 'fileName': fileName, 'fileSize': fileSize, @@ -100,8 +99,8 @@ class PDFDocumentProperties { 'producer': info.Producer, 'version': info.PDFFormatVersion, 'pageCount': this.pdfDocument.numPages, - 'pageSizeInch': pageSize.inch, - 'pageSizeMM': pageSize.mm, + 'pageSizeInch': pageSizes.inch, + 'pageSizeMM': pageSizes.mm, }); this._updateUI(); @@ -224,25 +223,25 @@ class PDFDocumentProperties { */ _parsePageSize(pageSizeInches) { if (!pageSizeInches) { - return Promise.resolve([undefined, undefined]); + return Promise.resolve({ inch: undefined, mm: undefined, }); } - const sizes_two_units = { - 'width_in': Math.round(pageSizeInches.width * 100) / 100, - 'height_in': Math.round(pageSizeInches.height * 100) / 100, - // 1in = 25.4mm; no need to round to 2 decimals for mm - 'width_mm': Math.round(pageSizeInches.width * 25.4 * 10) / 10, - 'height_mm': Math.round(pageSizeInches.height * 25.4 * 10) / 10, - }; + const { width, height, } = pageSizeInches; + return Promise.all([ - this.l10n.get('document_properties_page_size_in', - sizes_two_units, '{{width_in}} in × {{height_in}} in'), - this.l10n.get('document_properties_page_size_mm', - sizes_two_units, '{{width_mm}} mm × {{height_mm}} mm'), - ]).then(([parsedPageSizeInches, parsedPageSizeMM]) => { - return Promise.resolve({ - inch: parsedPageSizeInches, - mm: parsedPageSizeMM, - }); + this.l10n.get('document_properties_page_size_in', { + width_in: Math.round(width * 100) / 100, + height_in: Math.round(height * 100) / 100, + }, '{{width_in}}in × {{height_in}}in'), + // 1in = 25.4mm; no need to round to 2 decimals for millimeters. + this.l10n.get('document_properties_page_size_mm', { + width_mm: Math.round(width * 25.4 * 10) / 10, + height_mm: Math.round(height * 25.4 * 10) / 10, + }, '{{width_mm}}mm × {{height_mm}}mm'), + ]).then((sizes) => { + return { + inch: sizes[0], + mm: sizes[1], + }; }); }