From 23ec02bb930b8723f6464b61666deba1bff86748 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 28 Jan 2016 13:02:17 +0100 Subject: [PATCH 1/3] Remove the "Page: " label and replace it with a tooltip The following reasoning was used for deciding to remove the "Page: " label, and replace it with a tooltip, from the main toolbar: - We have no other visible labels in the *main* toolbar (e.g. the Zoom dropdown doesn't have a label, but only a tooltip). - We already hide the "Page: " label when the viewer is narrow. - The varying width of the "Page: " label in different locales is already causing issues for many languages, with overlap in the main toolbar as a result. Trying to create responsive CSS styles that works well in all locales is already difficult, and if we add support for page labels that will only further compound the issues. - Some PDF viewers (e.g. Adobe Reader, pdfium in Chrome) doesn't show labels in the UI by default. --- l10n/en-US/viewer.properties | 9 ++++----- l10n/sv-SE/viewer.properties | 9 ++++----- web/viewer.css | 2 +- web/viewer.html | 5 ++--- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/l10n/en-US/viewer.properties b/l10n/en-US/viewer.properties index 731a1f228..90c09aa99 100644 --- a/l10n/en-US/viewer.properties +++ b/l10n/en-US/viewer.properties @@ -18,11 +18,10 @@ previous_label=Previous next.title=Next Page next_label=Next -# LOCALIZATION NOTE (page_label, page_of): -# These strings are concatenated to form the "Page: X of Y" string. -# Do not translate "{{pageCount}}", it will be substituted with a number -# representing the total number of pages. -page_label=Page: +# LOCALIZATION NOTE (page.title): The tooltip for the pageNumber input. +page.title=Page +# LOCALIZATION NOTE (page_of): "{{pageCount}}" will be replaced by a number +# representing the total number of pages in the document. page_of=of {{pageCount}} zoom_out.title=Zoom Out diff --git a/l10n/sv-SE/viewer.properties b/l10n/sv-SE/viewer.properties index 4487addbd..514c32d4b 100644 --- a/l10n/sv-SE/viewer.properties +++ b/l10n/sv-SE/viewer.properties @@ -18,11 +18,10 @@ previous_label=Föregående next.title=Nästa sida next_label=Nästa -# LOCALIZATION NOTE (page_label, page_of): -# These strings are concatenated to form the "Page: X of Y" string. -# Do not translate "{{pageCount}}", it will be substituted with a number -# representing the total number of pages. -page_label=Sida: +# LOCALIZATION NOTE (page.title): The tooltip for the pageNumber input. +page.title=Sida +# LOCALIZATION NOTE (page_of): "{{pageCount}}" will be replaced by a number +# representing the total number of pages in the document. page_of=av {{pageCount}} zoom_out.title=Zooma ut diff --git a/web/viewer.css b/web/viewer.css index 4fedfe591..630d54b78 100644 --- a/web/viewer.css +++ b/web/viewer.css @@ -1939,7 +1939,7 @@ html[dir='rtl'] #documentPropertiesOverlay .row > * { } @media all and (max-width: 510px) { - #scaleSelectContainer, #pageNumberLabel { + #scaleSelectContainer { display: none; } } diff --git a/web/viewer.html b/web/viewer.html index 566770ae4..50df35d5a 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -194,8 +194,7 @@ See https://github.com/adobe-type-tools/cmap-resources Next - - +
@@ -236,7 +235,7 @@ See https://github.com/adobe-type-tools/cmap-resources
- From f461fd64aa221255cce87669a9cf3911acceac93 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 23 Jan 2016 13:56:56 +0100 Subject: [PATCH 2/3] Add support for PageLabels in the viewer This patch implements the page label functionality in a similar way as Adobe Reader. For documents with page labels, if a non-existent page label is entered we'll try to fallback to the page number instead. The patch also includes a preference (`disablePageLabels`), to make it easy to opt-out of using page labels if the user/implementor so wishes. The way that `get/set currentPageLabel` is implemented in `PDFViewer`, is as wrappers for the corresponding `get/set currentPageNumber` functions, since that seemed like the cleanest solution. The page labels are purposely *only* added to the page controls in the viewer UI, and not stored in e.g. the `ViewHistory`. Since doing so would mean adding unnecessary code complexity, without any real added value, and would also mean delaying the inital loading of PDF documents. Note that this patch will ignore page labels if they are identical to standard page numbering, since in this case displaying the page labels adds no value (but only UI noise). The reason for handling this case specially, is that in practice a surprising number of PDF files include "pointless" page labels. --- extensions/chromium/preferences_schema.json | 4 ++ l10n/en-US/viewer.properties | 8 ++- l10n/sv-SE/viewer.properties | 8 ++- web/app.js | 64 +++++++++++++++++++-- web/default_preferences.json | 3 +- web/pdf_thumbnail_viewer.js | 19 ++++++ web/pdf_viewer.js | 42 ++++++++++++++ 7 files changed, 137 insertions(+), 11 deletions(-) diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json index baa3300c4..9bff7d805 100644 --- a/extensions/chromium/preferences_schema.json +++ b/extensions/chromium/preferences_schema.json @@ -89,6 +89,10 @@ ], "default": 0 }, + "disablePageLabels": { + "type": "boolean", + "default": false + }, "disableTelemetry": { "title": "Disable telemetry", "type": "boolean", diff --git a/l10n/en-US/viewer.properties b/l10n/en-US/viewer.properties index 90c09aa99..2775ae19b 100644 --- a/l10n/en-US/viewer.properties +++ b/l10n/en-US/viewer.properties @@ -20,9 +20,13 @@ next_label=Next # LOCALIZATION NOTE (page.title): The tooltip for the pageNumber input. page.title=Page -# LOCALIZATION NOTE (page_of): "{{pageCount}}" will be replaced by a number +# LOCALIZATION NOTE (of_pages): "{{pagesCount}}" will be replaced by a number # representing the total number of pages in the document. -page_of=of {{pageCount}} +of_pages=of {{pagesCount}} +# LOCALIZATION NOTE (page_of_pages): "{{pageNumber}}" and "{{pagesCount}}" +# will be replaced by a number representing the currently visible page, +# respectively a number representing the total number of pages in the document. ++page_of_pages=({{pageNumber}} of {{pagesCount}}) zoom_out.title=Zoom Out zoom_out_label=Zoom Out diff --git a/l10n/sv-SE/viewer.properties b/l10n/sv-SE/viewer.properties index 514c32d4b..9ada2c333 100644 --- a/l10n/sv-SE/viewer.properties +++ b/l10n/sv-SE/viewer.properties @@ -20,9 +20,13 @@ next_label=Nästa # LOCALIZATION NOTE (page.title): The tooltip for the pageNumber input. page.title=Sida -# LOCALIZATION NOTE (page_of): "{{pageCount}}" will be replaced by a number +# LOCALIZATION NOTE (of_pages): "{{pagesCount}}" will be replaced by a number # representing the total number of pages in the document. -page_of=av {{pageCount}} +of_pages=av {{pagesCount}} +# LOCALIZATION NOTE (page_of_pages): "{{pageNumber}}" and "{{pagesCount}}" +# will be replaced by a number representing the currently active visible page, +# respectively a number representing the total number of pages in the document. +page_of_pages=({{pageNumber}} av {{pagesCount}}) zoom_out.title=Zooma ut zoom_out_label=Zooma ut diff --git a/web/app.js b/web/app.js index 73cc29911..a2fbfb884 100644 --- a/web/app.js +++ b/web/app.js @@ -178,10 +178,12 @@ var PDFViewerApplication = { preferencePdfBugEnabled: false, preferenceShowPreviousViewOnLoad: true, preferenceDefaultZoomValue: '', + preferenceDisablePageLabels: false, isViewerEmbedded: (window.parent !== window), url: '', baseUrl: '', externalServices: DefaultExernalServices, + hasPageLabels: false, // called once when the document is loaded initialize: function pdfViewInitialize(appConfig) { @@ -380,6 +382,9 @@ var PDFViewerApplication = { // before the various viewer components are initialized. self.pdfViewer.renderInteractiveForms = value; }), + Preferences.get('disablePageLabels').then(function resolved(value) { + self.preferenceDisablePageLabels = value; + }), // TODO move more preferences and other async stuff here ]).catch(function (reason) { }); @@ -567,6 +572,7 @@ var PDFViewerApplication = { } this.store = null; this.isInitialViewSet = false; + this.hasPageLabels = false; this.pdfSidebar.reset(); this.pdfOutlineViewer.reset(); @@ -879,7 +885,8 @@ var PDFViewerApplication = { this.pageRotation = 0; - this.pdfThumbnailViewer.setDocument(pdfDocument); + var pdfThumbnailViewer = this.pdfThumbnailViewer; + pdfThumbnailViewer.setDocument(pdfDocument); firstPagePromise.then(function(pdfPage) { downloadedPromise.then(function () { @@ -959,6 +966,33 @@ var PDFViewerApplication = { }); }); + pdfDocument.getPageLabels().then(function (labels) { + if (!labels || self.preferenceDisablePageLabels) { + return; + } + var i = 0, numLabels = labels.length; + if (numLabels !== self.pagesCount) { + console.error('The number of Page Labels does not match ' + + 'the number of pages in the document.'); + return; + } + // Ignore page labels that correspond to standard page numbering. + while (i < numLabels && labels[i] === (i + 1).toString()) { + i++; + } + if (i === numLabels) { + return; + } + + pdfViewer.setPageLabels(labels); + pdfThumbnailViewer.setPageLabels(labels); + + self.hasPageLabels = true; + self._updateUIToolbar({ + resetNumPages: true, + }); + }); + pagesPromise.then(function() { if (self.supportsPrinting) { pdfDocument.getJavaScript().then(function(javaScript) { @@ -1186,6 +1220,7 @@ var PDFViewerApplication = { /** * @typedef UpdateUIToolbarParameters * @property {number} pageNumber + * @property {string} pageLabel * @property {string} scaleValue * @property {number} scale * @property {boolean} resetNumPages @@ -1226,11 +1261,25 @@ var PDFViewerApplication = { var pagesCount = this.pagesCount; if (resetNumPages) { - toolbarConfig.numPages.textContent = - mozL10n.get('page_of', { pageCount: pagesCount }, 'of {{pageCount}}'); + if (this.hasPageLabels) { + toolbarConfig.pageNumber.type = 'text'; + } else { + toolbarConfig.pageNumber.type = 'number'; + toolbarConfig.numPages.textContent = mozL10n.get('of_pages', + { pagesCount: pagesCount }, 'of {{pagesCount}}'); + } toolbarConfig.pageNumber.max = pagesCount; } - toolbarConfig.pageNumber.value = pageNumber; + + if (this.hasPageLabels) { + toolbarConfig.pageNumber.value = params.pageLabel || + this.pdfViewer.currentPageLabel; + toolbarConfig.numPages.textContent = mozL10n.get('page_of_pages', + { pageNumber: pageNumber, pagesCount: pagesCount }, + '({{pageNumber}} of {{pagesCount}})'); + } else { + toolbarConfig.pageNumber.value = pageNumber; + } toolbarConfig.previous.disabled = (pageNumber <= 1); toolbarConfig.next.disabled = (pageNumber >= pagesCount); @@ -1495,11 +1544,13 @@ function webViewerInitialized() { }); appConfig.toolbar.pageNumber.addEventListener('change', function() { - PDFViewerApplication.page = (this.value | 0); + var pdfViewer = PDFViewerApplication.pdfViewer; + pdfViewer.currentPageLabel = this.value; // Ensure that the page number input displays the correct value, even if the // value entered by the user was invalid (e.g. a floating point number). - if (this.value !== PDFViewerApplication.page.toString()) { + if (this.value !== pdfViewer.currentPageNumber.toString() && + this.value !== pdfViewer.currentPageLabel) { PDFViewerApplication._updateUIToolbar({}); } }); @@ -1930,6 +1981,7 @@ function webViewerPageChanging(e) { PDFViewerApplication._updateUIToolbar({ pageNumber: page, + pageLabel: e.pageLabel, }); if (PDFViewerApplication.pdfSidebar.isThumbnailViewVisible) { diff --git a/web/default_preferences.json b/web/default_preferences.json index 242c30b12..15371f6b3 100644 --- a/web/default_preferences.json +++ b/web/default_preferences.json @@ -13,5 +13,6 @@ "useOnlyCssZoom": false, "externalLinkTarget": 0, "enhanceTextSelection": false, - "renderInteractiveForms": false + "renderInteractiveForms": false, + "disablePageLabels": false } diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index aa4181195..8a9dcc663 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -133,6 +133,7 @@ var PDFThumbnailViewer = (function PDFThumbnailViewerClosure() { */ _resetView: function PDFThumbnailViewer_resetView() { this.thumbnails = []; + this._pageLabels = null; this._pagesRotation = 0; this._pagesRequests = []; @@ -179,6 +180,24 @@ var PDFThumbnailViewer = (function PDFThumbnailViewerClosure() { } }, + /** + * @param {Array|null} labels + */ + setPageLabels: function PDFThumbnailViewer_setPageLabels(labels) { + if (!this.pdfDocument) { + return; + } + if (!labels) { + this._pageLabels = null; + } else if (!(labels instanceof Array && + this.pdfDocument.numPages === labels.length)) { + this._pageLabels = null; + console.error('PDFThumbnailViewer_setPageLabels: Invalid page labels.'); + } else { + this._pageLabels = labels; + } + }, + /** * @param {PDFThumbnailView} thumbView * @returns {PDFPage} diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 16c3198b7..5f7b97c13 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -211,6 +211,7 @@ var PDFViewer = (function pdfViewer() { var arg = { source: this, pageNumber: val, + pageLabel: this._pageLabels && this._pageLabels[val - 1], }; this._currentPageNumber = val; this.eventBus.dispatch('pagechanging', arg); @@ -221,6 +222,28 @@ var PDFViewer = (function pdfViewer() { } }, + /** + * @returns {string|null} Returns the current page label, + * or `null` if no page labels exist. + */ + get currentPageLabel() { + return this._pageLabels && this._pageLabels[this._currentPageNumber - 1]; + }, + + /** + * @param {string} val - The page label. + */ + set currentPageLabel(val) { + var pageNumber = val | 0; // Fallback page number. + if (this._pageLabels) { + var i = this._pageLabels.indexOf(val); + if (i >= 0) { + pageNumber = i + 1; + } + } + this.currentPageNumber = pageNumber; + }, + /** * @returns {number} */ @@ -414,11 +437,30 @@ var PDFViewer = (function pdfViewer() { }.bind(this)); }, + /** + * @param {Array|null} labels + */ + setPageLabels: function PDFViewer_setPageLabels(labels) { + if (!this.pdfDocument) { + return; + } + if (!labels) { + this._pageLabels = null; + } else if (!(labels instanceof Array && + this.pdfDocument.numPages === labels.length)) { + this._pageLabels = null; + console.error('PDFViewer_setPageLabels: Invalid page labels.'); + } else { + this._pageLabels = labels; + } + }, + _resetView: function () { this._pages = []; this._currentPageNumber = 1; this._currentScale = UNKNOWN_SCALE; this._currentScaleValue = null; + this._pageLabels = null; this._buffer = new PDFPageViewBuffer(DEFAULT_CACHE_SIZE); this._location = null; this._pagesRotation = 0; From efb9619e53b0e12ccbac95f3d7d5c87bbae24fa0 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 27 Jan 2016 14:27:01 +0100 Subject: [PATCH 3/3] Add PageLabels to `PDFPageView` and `PDFThumbnailView` --- web/pdf_page_view.js | 14 ++++++++++++++ web/pdf_thumbnail_view.js | 31 +++++++++++++++++++++++++++++-- web/pdf_thumbnail_viewer.js | 6 ++++++ web/pdf_viewer.js | 6 ++++++ 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 8e8d2b274..14e9fd0b6 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -78,6 +78,7 @@ var PDFPageView = (function PDFPageViewClosure() { this.id = id; this.renderingId = 'page' + id; + this.pageLabel = null; this.rotation = 0; this.scale = scale || DEFAULT_SCALE; @@ -554,6 +555,19 @@ var PDFPageView = (function PDFPageViewClosure() { } return promise; }, + + /** + * @param {string|null} label + */ + setPageLabel: function PDFView_setPageLabel(label) { + this.pageLabel = (typeof label === 'string' ? label : null); + + if (this.pageLabel !== null) { + this.div.setAttribute('data-page-label', this.pageLabel); + } else { + this.div.removeAttribute('data-page-label'); + } + }, }; return PDFPageView; diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 52db26928..65d0f6cf1 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -91,6 +91,7 @@ var PDFThumbnailView = (function PDFThumbnailViewClosure() { this.id = id; this.renderingId = 'thumbnail' + id; + this.pageLabel = null; this.pdfPage = null; this.rotation = 0; @@ -120,6 +121,7 @@ var PDFThumbnailView = (function PDFThumbnailViewClosure() { linkService.page = id; return false; }; + this.anchor = anchor; var div = document.createElement('div'); div.id = 'thumbnailContainer' + id; @@ -247,7 +249,7 @@ var PDFThumbnailView = (function PDFThumbnailViewClosure() { } var id = this.renderingId; var className = 'thumbnailImage'; - var ariaLabel = mozL10n.get('thumb_page_canvas', { page: this.id }, + var ariaLabel = mozL10n.get('thumb_page_canvas', { page: this.pageId }, 'Thumbnail of Page {{page}}'); if (this.disableCanvasToImageConversion) { @@ -395,7 +397,32 @@ var PDFThumbnailView = (function PDFThumbnailViewClosure() { ctx.drawImage(reducedImage, 0, 0, reducedWidth, reducedHeight, 0, 0, canvas.width, canvas.height); this._convertCanvasToImage(); - } + }, + + get pageId() { + return (this.pageLabel !== null ? this.pageLabel : this.id); + }, + + /** + * @param {string|null} label + */ + setPageLabel: function PDFThumbnailView_setPageLabel(label) { + this.pageLabel = (typeof label === 'string' ? label : null); + + this.anchor.title = mozL10n.get('thumb_page_title', { page: this.pageId }, + 'Page {{page}}'); + + if (this.renderingState !== RenderingStates.FINISHED) { + return; + } + var ariaLabel = mozL10n.get('thumb_page_canvas', { page: this.pageId }, + 'Thumbnail of Page {{page}}'); + if (this.image) { + this.image.setAttribute('aria-label', ariaLabel); + } else if (this.disableCanvasToImageConversion && this.canvas) { + this.canvas.setAttribute('aria-label', ariaLabel); + } + }, }; return PDFThumbnailView; diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 8a9dcc663..7d7eb5dd9 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -196,6 +196,12 @@ var PDFThumbnailViewer = (function PDFThumbnailViewerClosure() { } else { this._pageLabels = labels; } + // Update all the `PDFThumbnailView` instances. + for (var i = 0, ii = this.thumbnails.length; i < ii; i++) { + var thumbnailView = this.thumbnails[i]; + var label = this._pageLabels && this._pageLabels[i]; + thumbnailView.setPageLabel(label); + } }, /** diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js index 5f7b97c13..ede997ef2 100644 --- a/web/pdf_viewer.js +++ b/web/pdf_viewer.js @@ -453,6 +453,12 @@ var PDFViewer = (function pdfViewer() { } else { this._pageLabels = labels; } + // Update all the `PDFPageView` instances. + for (var i = 0, ii = this._pages.length; i < ii; i++) { + var pageView = this._pages[i]; + var label = this._pageLabels && this._pageLabels[i]; + pageView.setPageLabel(label); + } }, _resetView: function () {