From 7aca53b1a4f79dadcb551b5fb5eabb4eda886a1e Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Sep 2018 16:11:59 +0200 Subject: [PATCH 1/7] Remove the find bar's dependency on the find controller Pull request #10100 removed the last usage of the find controller from the find bar, so we can drop the dependency now. --- web/app.js | 1 - web/pdf_find_bar.js | 6 ------ 2 files changed, 7 deletions(-) diff --git a/web/app.js b/web/app.js index 3ff363ba1b6b6..3f2dfc95b8dbb 100644 --- a/web/app.js +++ b/web/app.js @@ -369,7 +369,6 @@ let PDFViewerApplication = { // TODO: improve `PDFFindBar` constructor parameter passing let findBarConfig = Object.create(appConfig.findBar); - findBarConfig.findController = this.findController; findBarConfig.eventBus = eventBus; this.findBar = new PDFFindBar(findBarConfig, this.l10n); diff --git a/web/pdf_find_bar.js b/web/pdf_find_bar.js index 61362099fd26a..b1696b5c92f9f 100644 --- a/web/pdf_find_bar.js +++ b/web/pdf_find_bar.js @@ -38,15 +38,9 @@ class PDFFindBar { this.findResultsCount = options.findResultsCount || null; this.findPreviousButton = options.findPreviousButton || null; this.findNextButton = options.findNextButton || null; - this.findController = options.findController || null; this.eventBus = options.eventBus; this.l10n = l10n; - if (this.findController === null) { - throw new Error('PDFFindBar cannot be used without a ' + - 'PDFFindController instance.'); - } - // Add event listeners to the DOM elements. this.toggleButton.addEventListener('click', () => { this.toggle(); From b14c1fbc28e432bdb5c3387e2105360174121274 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Sep 2018 16:53:52 +0200 Subject: [PATCH 2/7] Use the `updatetextlayermatches` event for highlighting matches on a page This makes use of the event bus instead of requiring the PDF viewer instance to get the page view for a page and calling `updateMatches` on it. --- web/pdf_find_controller.js | 8 ++++---- web/text_layer_builder.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index 71457d9dfb43c..8a8bc89a9dc8a 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -355,10 +355,10 @@ class PDFFindController { this._pdfViewer.currentPageNumber = index + 1; } - const page = this._pdfViewer.getPageView(index); - if (page.textLayer) { - page.textLayer.updateMatches(); - } + this._eventBus.dispatch('updatetextlayermatches', { + source: this, + pageIndex: index, + }); } _nextMatch() { diff --git a/web/text_layer_builder.js b/web/text_layer_builder.js index 281f22a888ccd..1e7431b2eddb1 100644 --- a/web/text_layer_builder.js +++ b/web/text_layer_builder.js @@ -358,7 +358,7 @@ class TextLayerBuilder { } }; _boundEvents.updateTextLayerMatches = (evt) => { - if (evt.pageIndex !== -1) { + if (evt.pageIndex !== this.pageIdx && evt.pageIndex !== -1) { return; } this.updateMatches(); From e293c12afcf3a9a55669b6bf095d657e7780ba16 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Sep 2018 17:19:33 +0200 Subject: [PATCH 3/7] Implement the `setDocument` method for the find controller Now it follows the same pattern as e.g., the document properties component, which allows us to have one instance of the find controller and set a new document to search upon switching documents. Moreover, this allows us to get rid of the dependency on `pdfViewer` in order to fetch the text content for a page. This is working towards getting rid of the `pdfViewer` dependency upon initializing the component entirely in future commits. Finally, we make the `reset` method private since it's not supposed to be used from the outside anymore now that `setDocument` takes care of this, similar to other components. --- examples/components/simpleviewer.js | 3 +- examples/components/singlepageviewer.js | 3 +- examples/svgviewer/viewer.js | 3 +- web/app.js | 3 +- web/base_viewer.js | 8 --- web/pdf_document_properties.js | 2 +- web/pdf_find_controller.js | 75 ++++++++++++++++--------- 7 files changed, 59 insertions(+), 38 deletions(-) diff --git a/examples/components/simpleviewer.js b/examples/components/simpleviewer.js index 4cbf344eae921..1ca0a6e991579 100644 --- a/examples/components/simpleviewer.js +++ b/examples/components/simpleviewer.js @@ -55,7 +55,7 @@ container.addEventListener('pagesinit', function () { pdfViewer.currentScaleValue = 'page-width'; if (SEARCH_FOR) { // We can try search for things - pdfFindController.executeCommand('find', {query: SEARCH_FOR}); + pdfFindController.executeCommand('find', { query: SEARCH_FOR, }); } }); @@ -70,4 +70,5 @@ pdfjsLib.getDocument({ pdfViewer.setDocument(pdfDocument); pdfLinkService.setDocument(pdfDocument, null); + pdfFindController.setDocument(pdfDocument); }); diff --git a/examples/components/singlepageviewer.js b/examples/components/singlepageviewer.js index dbe7b03d6a4b2..c15ae24ee9f66 100644 --- a/examples/components/singlepageviewer.js +++ b/examples/components/singlepageviewer.js @@ -55,7 +55,7 @@ container.addEventListener('pagesinit', function () { pdfSinglePageViewer.currentScaleValue = 'page-width'; if (SEARCH_FOR) { // We can try search for things - pdfFindController.executeCommand('find', {query: SEARCH_FOR}); + pdfFindController.executeCommand('find', { query: SEARCH_FOR, }); } }); @@ -70,4 +70,5 @@ pdfjsLib.getDocument({ pdfSinglePageViewer.setDocument(pdfDocument); pdfLinkService.setDocument(pdfDocument, null); + pdfFindController.setDocument(pdfDocument); }); diff --git a/examples/svgviewer/viewer.js b/examples/svgviewer/viewer.js index 17d545a988bd6..bf8330b32b5b1 100644 --- a/examples/svgviewer/viewer.js +++ b/examples/svgviewer/viewer.js @@ -57,7 +57,7 @@ container.addEventListener('pagesinit', function () { pdfViewer.currentScaleValue = 'page-width'; if (SEARCH_FOR) { // We can try search for things - pdfFindController.executeCommand('find', {query: SEARCH_FOR}); + pdfFindController.executeCommand('find', { query: SEARCH_FOR, }); } }); @@ -72,4 +72,5 @@ pdfjsLib.getDocument({ pdfViewer.setDocument(pdfDocument); pdfLinkService.setDocument(pdfDocument, null); + pdfFindController.setDocument(pdfDocument); }); diff --git a/web/app.js b/web/app.js index 3f2dfc95b8dbb..cca1e034fac3d 100644 --- a/web/app.js +++ b/web/app.js @@ -592,6 +592,7 @@ let PDFViewerApplication = { if (this.pdfDocument) { this.pdfDocument = null; + this.findController.setDocument(null); this.pdfThumbnailViewer.setDocument(null); this.pdfViewer.setDocument(null); this.pdfLinkService.setDocument(null); @@ -608,7 +609,6 @@ let PDFViewerApplication = { this.pdfOutlineViewer.reset(); this.pdfAttachmentViewer.reset(); - this.findController.reset(); this.findBar.reset(); this.toolbar.reset(); this.secondaryToolbar.reset(); @@ -916,6 +916,7 @@ let PDFViewerApplication = { } else if (PDFJSDev.test('CHROME')) { baseDocumentUrl = location.href.split('#')[0]; } + this.findController.setDocument(pdfDocument); this.pdfLinkService.setDocument(pdfDocument, baseDocumentUrl); this.pdfDocumentProperties.setDocument(pdfDocument, this.url); diff --git a/web/base_viewer.js b/web/base_viewer.js index 3bd7233847850..38e819812a8ff 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -913,14 +913,6 @@ class BaseViewer { return false; } - getPageTextContent(pageIndex) { - return this.pdfDocument.getPage(pageIndex + 1).then(function(page) { - return page.getTextContent({ - normalizeWhitespace: true, - }); - }); - } - /** * @param {HTMLDivElement} textLayerDiv * @param {number} pageIndex diff --git a/web/pdf_document_properties.js b/web/pdf_document_properties.js index 329c8dddc25e5..168ac14f21bfd 100644 --- a/web/pdf_document_properties.js +++ b/web/pdf_document_properties.js @@ -184,7 +184,7 @@ class PDFDocumentProperties { * Note that the overlay will contain no information if this method * is not called. * - * @param {Object} pdfDocument - A reference to the PDF document. + * @param {PDFDocumentProxy} pdfDocument - A reference to the PDF document. * @param {string} url - The URL of the document. */ setDocument(pdfDocument, url = null) { diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index 8a8bc89a9dc8a..9dfa107ba3719 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -51,7 +51,7 @@ class PDFFindController { this.onUpdateResultsCount = null; this.onUpdateState = null; - this.reset(); + this._reset(); eventBus.on('findbarclose', () => { this._highlightMatches = false; @@ -87,8 +87,51 @@ class PDFFindController { return this._state; } - reset() { + /** + * Set a reference to the PDF document in order to search it. + * Note that searching is not possible if this method is not called. + * + * @param {PDFDocumentProxy} pdfDocument - The PDF document to search. + */ + setDocument(pdfDocument) { + if (this._pdfDocument) { + this._reset(); + } + if (!pdfDocument) { + return; + } + this._pdfDocument = pdfDocument; + } + + executeCommand(cmd, state) { + if (!this._pdfDocument) { + return; + } + + if (this._state === null || cmd !== 'findagain') { + this._dirtyMatch = true; + } + this._state = state; + this._updateUIState(FindState.PENDING); + + this._firstPagePromise.then(() => { + this._extractText(); + + clearTimeout(this._findTimeout); + if (cmd === 'find') { + // Trigger the find action with a small delay to avoid starting the + // search when the user is still typing (saving resources). + this._findTimeout = + setTimeout(this._nextMatch.bind(this), FIND_TIMEOUT); + } else { + this._nextMatch(); + } + }); + } + + _reset() { this._highlightMatches = false; + this._pdfDocument = null; this._pageMatches = []; this._pageMatchesLength = null; this._state = null; @@ -118,28 +161,6 @@ class PDFFindController { }); } - executeCommand(cmd, state) { - if (this._state === null || cmd !== 'findagain') { - this._dirtyMatch = true; - } - this._state = state; - this._updateUIState(FindState.PENDING); - - this._firstPagePromise.then(() => { - this._extractText(); - - clearTimeout(this._findTimeout); - if (cmd === 'find') { - // Trigger the find action with a small delay to avoid starting the - // search when the user is still typing (saving resources). - this._findTimeout = - setTimeout(this._nextMatch.bind(this), FIND_TIMEOUT); - } else { - this._nextMatch(); - } - }); - } - _normalize(text) { return text.replace(this._normalizationRegex, function(ch) { return CHARACTERS_TO_NORMALIZE[ch]; @@ -326,7 +347,11 @@ class PDFFindController { this._extractTextPromises[i] = extractTextCapability.promise; promise = promise.then(() => { - return this._pdfViewer.getPageTextContent(i).then((textContent) => { + return this._pdfDocument.getPage(i + 1).then((pdfPage) => { + return pdfPage.getTextContent({ + normalizeWhitespace: true, + }); + }).then((textContent) => { const textItems = textContent.items; const strBuf = []; From e0c811f2ede8183b482b94023a7b895d04494892 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Sep 2018 19:40:11 +0200 Subject: [PATCH 4/7] Use the link service for getting and setting page information This removes the dependency on a `PDFViewer` instance from the find controller, which makes it more similar to other components and makes it easier to unit test with a mock link service. Finally, we remove the search capabilities from the SVG example since it doesn't work there because there is no separate text layer. --- examples/components/simpleviewer.js | 2 +- examples/components/singlepageviewer.js | 2 +- examples/svgviewer/viewer.js | 12 ----------- web/app.js | 2 +- web/pdf_find_controller.js | 27 ++++++++++++++++--------- 5 files changed, 21 insertions(+), 24 deletions(-) diff --git a/examples/components/simpleviewer.js b/examples/components/simpleviewer.js index 1ca0a6e991579..ece7692409f7f 100644 --- a/examples/components/simpleviewer.js +++ b/examples/components/simpleviewer.js @@ -46,7 +46,7 @@ pdfLinkService.setViewer(pdfViewer); // (Optionally) enable find controller. var pdfFindController = new pdfjsViewer.PDFFindController({ - pdfViewer: pdfViewer, + linkService: pdfLinkService, }); pdfViewer.setFindController(pdfFindController); diff --git a/examples/components/singlepageviewer.js b/examples/components/singlepageviewer.js index c15ae24ee9f66..1160a6f2e0cae 100644 --- a/examples/components/singlepageviewer.js +++ b/examples/components/singlepageviewer.js @@ -46,7 +46,7 @@ pdfLinkService.setViewer(pdfSinglePageViewer); // (Optionally) enable find controller. var pdfFindController = new pdfjsViewer.PDFFindController({ - pdfViewer: pdfSinglePageViewer, + linkService: pdfLinkService, }); pdfSinglePageViewer.setFindController(pdfFindController); diff --git a/examples/svgviewer/viewer.js b/examples/svgviewer/viewer.js index bf8330b32b5b1..f6137b1f3941a 100644 --- a/examples/svgviewer/viewer.js +++ b/examples/svgviewer/viewer.js @@ -31,7 +31,6 @@ var CMAP_URL = '../../node_modules/pdfjs-dist/cmaps/'; var CMAP_PACKED = true; var DEFAULT_URL = '../../web/compressed.tracemonkey-pldi-09.pdf'; -var SEARCH_FOR = ''; // try 'Mozilla'; var container = document.getElementById('viewerContainer'); @@ -46,19 +45,9 @@ var pdfViewer = new pdfjsViewer.PDFViewer({ }); pdfLinkService.setViewer(pdfViewer); -// (Optionally) enable find controller. -var pdfFindController = new pdfjsViewer.PDFFindController({ - pdfViewer: pdfViewer, -}); -pdfViewer.setFindController(pdfFindController); - container.addEventListener('pagesinit', function () { // We can use pdfViewer now, e.g. let's change default scale. pdfViewer.currentScaleValue = 'page-width'; - - if (SEARCH_FOR) { // We can try search for things - pdfFindController.executeCommand('find', { query: SEARCH_FOR, }); - } }); // Loading document. @@ -72,5 +61,4 @@ pdfjsLib.getDocument({ pdfViewer.setDocument(pdfDocument); pdfLinkService.setDocument(pdfDocument, null); - pdfFindController.setDocument(pdfDocument); }); diff --git a/web/app.js b/web/app.js index cca1e034fac3d..63e7142a224c6 100644 --- a/web/app.js +++ b/web/app.js @@ -343,7 +343,7 @@ let PDFViewerApplication = { pdfLinkService.setHistory(this.pdfHistory); this.findController = new PDFFindController({ - pdfViewer: this.pdfViewer, + linkService: pdfLinkService, eventBus, }); this.findController.onUpdateResultsCount = (matchesCount) => { diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index 9dfa107ba3719..082eb6a630e32 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -40,12 +40,21 @@ const CHARACTERS_TO_NORMALIZE = { '\u00BE': '3/4', // Vulgar fraction three quarters }; +/** + * @typedef {Object} PDFFindControllerOptions + * @property {IPDFLinkService} linkService - The navigation/linking service. + * @property {EventBus} eventBus - The application event bus. + */ + /** * Provides search functionality to find a given string in a PDF document. */ class PDFFindController { - constructor({ pdfViewer, eventBus = getGlobalEventBus(), }) { - this._pdfViewer = pdfViewer; + /** + * @param {PDFFindControllerOptions} options + */ + constructor({ linkService, eventBus = getGlobalEventBus(), }) { + this._linkService = linkService; this._eventBus = eventBus; this.onUpdateResultsCount = null; @@ -342,7 +351,7 @@ class PDFFindController { } let promise = Promise.resolve(); - for (let i = 0, ii = this._pdfViewer.pagesCount; i < ii; i++) { + for (let i = 0, ii = this._linkService.pagesCount; i < ii; i++) { const extractTextCapability = createPromiseCapability(); this._extractTextPromises[i] = extractTextCapability.promise; @@ -375,9 +384,9 @@ class PDFFindController { _updatePage(index) { if (this._selected.pageIdx === index) { // If the page is selected, scroll the page into view, which triggers - // rendering the page, which adds the textLayer. Once the textLayer is - // build, it will scroll onto the selected match. - this._pdfViewer.currentPageNumber = index + 1; + // rendering the page, which adds the text layer. Once the text layer + // is built, it will scroll to the selected match. + this._linkService.page = index + 1; } this._eventBus.dispatch('updatetextlayermatches', { @@ -388,8 +397,8 @@ class PDFFindController { _nextMatch() { const previous = this._state.findPrevious; - const currentPageIndex = this._pdfViewer.currentPageNumber - 1; - const numPages = this._pdfViewer.pagesCount; + const currentPageIndex = this._linkService.page - 1; + const numPages = this._linkService.pagesCount; this._highlightMatches = true; @@ -501,7 +510,7 @@ class PDFFindController { _advanceOffsetPage(previous) { const offset = this._offset; - const numPages = this._extractTextPromises.length; + const numPages = this._linkService.pagesCount; offset.pageIdx = (previous ? offset.pageIdx - 1 : offset.pageIdx + 1); offset.matchIdx = null; From 38ff79186a3330666ff712f10d25e8f45cb876a0 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Sep 2018 20:00:58 +0200 Subject: [PATCH 5/7] Replace callbacks for updating the UI with dispatching events on the event bus This makes it more similar to how other components update the viewer UI and avoids the need to have extra member variables and checks. --- web/app.js | 43 +++++++++++++++++++++----------------- web/pdf_find_controller.js | 23 +++++++++----------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/web/app.js b/web/app.js index 63e7142a224c6..62b65b1578c2a 100644 --- a/web/app.js +++ b/web/app.js @@ -346,25 +346,6 @@ let PDFViewerApplication = { linkService: pdfLinkService, eventBus, }); - this.findController.onUpdateResultsCount = (matchesCount) => { - if (this.supportsIntegratedFind) { - this.externalServices.updateFindMatchesCount(matchesCount); - } else { - this.findBar.updateResultsCount(matchesCount); - } - }; - this.findController.onUpdateState = (state, previous, matchesCount) => { - if (this.supportsIntegratedFind) { - this.externalServices.updateFindControlState({ - result: state, - findPrevious: previous, - matchesCount, - }); - } else { - this.findBar.updateUIState(state, previous, matchesCount); - } - }; - this.pdfViewer.setFindController(this.findController); // TODO: improve `PDFFindBar` constructor parameter passing @@ -1343,6 +1324,8 @@ let PDFViewerApplication = { eventBus.on('documentproperties', webViewerDocumentProperties); eventBus.on('find', webViewerFind); eventBus.on('findfromurlhash', webViewerFindFromUrlHash); + eventBus.on('updatefindmatchescount', webViewerUpdateFindMatchesCount); + eventBus.on('updatefindcontrolstate', webViewerUpdateFindControlState); if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { eventBus.on('fileinputchange', webViewerFileInputChange); } @@ -1414,6 +1397,8 @@ let PDFViewerApplication = { eventBus.off('documentproperties', webViewerDocumentProperties); eventBus.off('find', webViewerFind); eventBus.off('findfromurlhash', webViewerFindFromUrlHash); + eventBus.off('updatefindmatchescount', webViewerUpdateFindMatchesCount); + eventBus.off('updatefindcontrolstate', webViewerUpdateFindControlState); if (typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) { eventBus.off('fileinputchange', webViewerFileInputChange); } @@ -1976,6 +1961,26 @@ function webViewerFindFromUrlHash(evt) { }); } +function webViewerUpdateFindMatchesCount({ matchesCount, }) { + if (PDFViewerApplication.supportsIntegratedFind) { + PDFViewerApplication.externalServices.updateFindMatchesCount(matchesCount); + } else { + PDFViewerApplication.findBar.updateResultsCount(matchesCount); + } +} + +function webViewerUpdateFindControlState({ state, previous, matchesCount, }) { + if (PDFViewerApplication.supportsIntegratedFind) { + PDFViewerApplication.externalServices.updateFindControlState({ + result: state, + findPrevious: previous, + matchesCount, + }); + } else { + PDFViewerApplication.findBar.updateUIState(state, previous, matchesCount); + } +} + function webViewerScaleChanging(evt) { PDFViewerApplication.toolbar.setPageScale(evt.presetValue, evt.scale); diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index 082eb6a630e32..a74df0893b541 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -57,9 +57,6 @@ class PDFFindController { this._linkService = linkService; this._eventBus = eventBus; - this.onUpdateResultsCount = null; - this.onUpdateState = null; - this._reset(); eventBus.on('findbarclose', () => { @@ -564,19 +561,19 @@ class PDFFindController { } _updateUIResultsCount() { - if (!this.onUpdateResultsCount) { - return; - } - const matchesCount = this._requestMatchesCount(); - this.onUpdateResultsCount(matchesCount); + this._eventBus.dispatch('updatefindmatchescount', { + source: this, + matchesCount: this._requestMatchesCount(), + }); } _updateUIState(state, previous) { - if (!this.onUpdateState) { - return; - } - const matchesCount = this._requestMatchesCount(); - this.onUpdateState(state, previous, matchesCount); + this._eventBus.dispatch('updatefindcontrolstate', { + source: this, + state, + previous, + matchesCount: this._requestMatchesCount(), + }); } } From f79fb8886433b484265735ca31b3489b23d8397f Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sat, 22 Sep 2018 19:44:13 +0200 Subject: [PATCH 6/7] Remove the find controller setter in `web/base_viewer.js` With `PDFFindController` instances no longer (directly) depending on `BaseViewer` instances, we can pass a single `findController` when initializing a viewer, similar to other components. --- examples/components/simpleviewer.js | 12 ++++++------ examples/components/singlepageviewer.js | 12 ++++++------ web/app.js | 13 +++++++------ web/base_viewer.js | 7 +++---- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/examples/components/simpleviewer.js b/examples/components/simpleviewer.js index ece7692409f7f..090acc3bc5117 100644 --- a/examples/components/simpleviewer.js +++ b/examples/components/simpleviewer.js @@ -38,17 +38,17 @@ var container = document.getElementById('viewerContainer'); // (Optionally) enable hyperlinks within PDF files. var pdfLinkService = new pdfjsViewer.PDFLinkService(); -var pdfViewer = new pdfjsViewer.PDFViewer({ - container: container, +// (Optionally) enable find controller. +var pdfFindController = new pdfjsViewer.PDFFindController({ linkService: pdfLinkService, }); -pdfLinkService.setViewer(pdfViewer); -// (Optionally) enable find controller. -var pdfFindController = new pdfjsViewer.PDFFindController({ +var pdfViewer = new pdfjsViewer.PDFViewer({ + container: container, linkService: pdfLinkService, + findController: pdfFindController, }); -pdfViewer.setFindController(pdfFindController); +pdfLinkService.setViewer(pdfViewer); container.addEventListener('pagesinit', function () { // We can use pdfViewer now, e.g. let's change default scale. diff --git a/examples/components/singlepageviewer.js b/examples/components/singlepageviewer.js index 1160a6f2e0cae..67c7a416e4c31 100644 --- a/examples/components/singlepageviewer.js +++ b/examples/components/singlepageviewer.js @@ -38,17 +38,17 @@ var container = document.getElementById('viewerContainer'); // (Optionally) enable hyperlinks within PDF files. var pdfLinkService = new pdfjsViewer.PDFLinkService(); -var pdfSinglePageViewer = new pdfjsViewer.PDFSinglePageViewer({ - container: container, +// (Optionally) enable find controller. +var pdfFindController = new pdfjsViewer.PDFFindController({ linkService: pdfLinkService, }); -pdfLinkService.setViewer(pdfSinglePageViewer); -// (Optionally) enable find controller. -var pdfFindController = new pdfjsViewer.PDFFindController({ +var pdfSinglePageViewer = new pdfjsViewer.PDFSinglePageViewer({ + container: container, linkService: pdfLinkService, + findController: pdfFindController, }); -pdfSinglePageViewer.setFindController(pdfFindController); +pdfLinkService.setViewer(pdfSinglePageViewer); container.addEventListener('pagesinit', function () { // We can use pdfSinglePageViewer now, e.g. let's change default scale. diff --git a/web/app.js b/web/app.js index 62b65b1578c2a..8c605a20ae12b 100644 --- a/web/app.js +++ b/web/app.js @@ -305,6 +305,12 @@ let PDFViewerApplication = { }); this.downloadManager = downloadManager; + const findController = new PDFFindController({ + linkService: pdfLinkService, + eventBus, + }); + this.findController = findController; + let container = appConfig.mainContainer; let viewer = appConfig.viewerContainer; this.pdfViewer = new PDFViewer({ @@ -314,6 +320,7 @@ let PDFViewerApplication = { renderingQueue: pdfRenderingQueue, linkService: pdfLinkService, downloadManager, + findController, renderer: AppOptions.get('renderer'), enableWebGL: AppOptions.get('enableWebGL'), l10n: this.l10n, @@ -342,12 +349,6 @@ let PDFViewerApplication = { }); pdfLinkService.setHistory(this.pdfHistory); - this.findController = new PDFFindController({ - linkService: pdfLinkService, - eventBus, - }); - this.pdfViewer.setFindController(this.findController); - // TODO: improve `PDFFindBar` constructor parameter passing let findBarConfig = Object.create(appConfig.findBar); findBarConfig.eventBus = eventBus; diff --git a/web/base_viewer.js b/web/base_viewer.js index 38e819812a8ff..57153057fefc1 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -49,6 +49,8 @@ const SpreadMode = { * @property {IPDFLinkService} linkService - The navigation/linking service. * @property {DownloadManager} downloadManager - (optional) The download * manager component. + * @property {PDFFindController} findController - (optional) The find + * controller component. * @property {PDFRenderingQueue} renderingQueue - (optional) The rendering * queue object. * @property {boolean} removePageBorders - (optional) Removes the border shadow @@ -142,6 +144,7 @@ class BaseViewer { this.eventBus = options.eventBus || getGlobalEventBus(); this.linkService = options.linkService || new SimpleLinkService(); this.downloadManager = options.downloadManager || null; + this.findController = options.findController || null; this.removePageBorders = options.removePageBorders || false; this.textLayerMode = Number.isInteger(options.textLayerMode) ? options.textLayerMode : TextLayerMode.ENABLE; @@ -955,10 +958,6 @@ class BaseViewer { }); } - setFindController(findController) { - this.findController = findController; - } - /** * @returns {boolean} Whether all pages of the PDF document have identical * widths and heights. From 1b402996cfa23d33177827684e1744bd97706522 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 21 Sep 2018 22:40:15 +0200 Subject: [PATCH 7/7] Implement a basic unit test for the find controller This commit shows that we can now unit test the find controller and that executing regular queries works. Note that this is only a first step and not a complete suite of unit tests for all possible options of the find controller. While writing this unit test, I found two smaller issues that I addressed directly. The first one is that in the previous find controller refactoring I forgot to rename some occurrences of a now private member variable. Fortunately this did not cause any bugs since we did have a public getter and the fetched value may be changed by reference, but it's nevertheless good to fix. The second issue is that some entries in the `test/unit/clitests.json` file were not correct, resulting in these tests not being executed on e.g., Travis CI. --- test/unit/clitests.json | 5 +- test/unit/jasmine-boot.js | 1 + test/unit/pdf_find_controller_spec.js | 99 +++++++++++++++++++++++++++ web/pdf_find_controller.js | 4 +- 4 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 test/unit/pdf_find_controller_spec.js diff --git a/test/unit/clitests.json b/test/unit/clitests.json index a0348fb7e15cf..9aa5a7b12fcc5 100644 --- a/test/unit/clitests.json +++ b/test/unit/clitests.json @@ -25,8 +25,9 @@ "network_utils_spec.js", "node_stream_spec.js", "parser_spec.js", - "pdf_find_utils.js", - "pdf_history.js", + "pdf_find_controller_spec.js", + "pdf_find_utils_spec.js", + "pdf_history_spec.js", "primitives_spec.js", "stream_spec.js", "type1_parser_spec.js", diff --git a/test/unit/jasmine-boot.js b/test/unit/jasmine-boot.js index ef87c76ee48a2..98b736e4918ce 100644 --- a/test/unit/jasmine-boot.js +++ b/test/unit/jasmine-boot.js @@ -67,6 +67,7 @@ function initializePDFJS(callback) { 'pdfjs-test/unit/network_spec', 'pdfjs-test/unit/network_utils_spec', 'pdfjs-test/unit/parser_spec', + 'pdfjs-test/unit/pdf_find_controller_spec', 'pdfjs-test/unit/pdf_find_utils_spec', 'pdfjs-test/unit/pdf_history_spec', 'pdfjs-test/unit/primitives_spec', diff --git a/test/unit/pdf_find_controller_spec.js b/test/unit/pdf_find_controller_spec.js new file mode 100644 index 0000000000000..269e6c65f5c56 --- /dev/null +++ b/test/unit/pdf_find_controller_spec.js @@ -0,0 +1,99 @@ +/* Copyright 2018 Mozilla Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { buildGetDocumentParams } from './test_utils'; +import { EventBus } from '../../web/ui_utils'; +import { getDocument } from '../../src/display/api'; +import { PDFFindController } from '../../web/pdf_find_controller'; +import { SimpleLinkService } from '../../web/pdf_link_service'; + +class MockLinkService extends SimpleLinkService { + constructor() { + super(); + + this._page = 1; + this._pdfDocument = null; + } + + setDocument(pdfDocument) { + this._pdfDocument = pdfDocument; + } + + get pagesCount() { + return this._pdfDocument.numPages; + } + + get page() { + return this._page; + } + + set page(value) { + this._page = value; + } +} + +describe('pdf_find_controller', function() { + let eventBus; + let pdfFindController; + + beforeEach(function(done) { + const loadingTask = getDocument(buildGetDocumentParams('tracemonkey.pdf')); + loadingTask.promise.then(function(pdfDocument) { + const linkService = new MockLinkService(); + linkService.setDocument(pdfDocument); + + eventBus = new EventBus(); + + pdfFindController = new PDFFindController({ + linkService, + eventBus, + }); + pdfFindController.setDocument(pdfDocument); + + eventBus.dispatch('pagesinit'); + done(); + }); + }); + + afterEach(function() { + eventBus = null; + pdfFindController = null; + }); + + it('performs a basic search', function(done) { + pdfFindController.executeCommand('find', { query: 'Dynamic', }); + + const matchesPerPage = [11, 5, 0, 3, 0, 0, 0, 1, 1, 1, 0, 3, 4, 4]; + const totalPages = matchesPerPage.length; + const totalMatches = matchesPerPage.reduce((a, b) => { + return a + b; + }); + + eventBus.on('updatefindmatchescount', + function onUpdateFindMatchesCount(evt) { + if (pdfFindController.pageMatches.length !== totalPages) { + return; + } + eventBus.off('updatefindmatchescount', onUpdateFindMatchesCount); + + expect(evt.matchesCount.total).toBe(totalMatches); + for (let i = 0; i < totalPages; i++) { + expect(pdfFindController.pageMatches[i].length) + .toEqual(matchesPerPage[i]); + } + done(); + }); + }); +}); diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index a74df0893b541..cbd50dbb4d9c2 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -526,8 +526,8 @@ class PDFFindController { if (found) { const previousPage = this._selected.pageIdx; - this.selected.pageIdx = this._offset.pageIdx; - this.selected.matchIdx = this._offset.matchIdx; + this._selected.pageIdx = this._offset.pageIdx; + this._selected.matchIdx = this._offset.matchIdx; state = (wrapped ? FindState.WRAPPED : FindState.FOUND); // Update the currently selected page to wipe out any selected matches.