From d73a71fb905062111a8bf1f019935552b0508a95 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 25 Oct 2018 16:21:55 +0200 Subject: [PATCH] Small clean-up of the search related methods in `TextLayerBuilder` This patch does four things: - Change the search related methods in `TextLayerBuilder` to be "private", since there're only called from within the class itself now. - Use `const` for local variables not intended to change in the search related methods in `TextLayerBuilder`. - Finally, removes most `this.findController` checks since they are redundant. Note how both `this._convertMatches` and `this._renderMatches` are *only* ever called, from `this._updateMatches`, when `this.findController` is actually defined. Hence there's really no need to repeat those checks all over the place, especially with all the relevant methods now being marked as "private". - Always initialize the `this._pageMatchesLength` property with an empty array, to simplify the code in `TextLayerBuilder`. --- web/pdf_find_controller.js | 7 +-- web/text_layer_builder.js | 100 ++++++++++++++++--------------------- 2 files changed, 45 insertions(+), 62 deletions(-) diff --git a/web/pdf_find_controller.js b/web/pdf_find_controller.js index 5c19b4435..37b5b5e77 100644 --- a/web/pdf_find_controller.js +++ b/web/pdf_find_controller.js @@ -141,7 +141,7 @@ class PDFFindController { this._highlightMatches = false; this._pdfDocument = null; this._pageMatches = []; - this._pageMatchesLength = null; + this._pageMatchesLength = []; this._state = null; this._selected = { // Currently selected match. pageIdx: -1, @@ -291,9 +291,6 @@ class PDFFindController { } // Prepare arrays for storing the matches. - if (!this._pageMatchesLength) { - this._pageMatchesLength = []; - } this._pageMatchesLength[pageIndex] = []; this._pageMatches[pageIndex] = []; @@ -404,7 +401,7 @@ class PDFFindController { this._offset.matchIdx = null; this._resumePageIdx = null; this._pageMatches.length = 0; - this._pageMatchesLength = null; + this._pageMatchesLength.length = 0; this._matchesCountTotal = 0; for (let i = 0; i < numPages; i++) { diff --git a/web/text_layer_builder.js b/web/text_layer_builder.js index 1e7431b2e..f7cffe502 100644 --- a/web/text_layer_builder.js +++ b/web/text_layer_builder.js @@ -108,7 +108,7 @@ class TextLayerBuilder { this.textLayerRenderTask.promise.then(() => { this.textLayerDiv.appendChild(textLayerFrag); this._finishRendering(); - this.updateMatches(); + this._updateMatches(); }, function (reason) { // Cancelled or failed to render text layer; skipping errors. }); @@ -134,24 +134,25 @@ class TextLayerBuilder { this.textContent = textContent; } - convertMatches(matches, matchesLength) { - let i = 0; - let iIndex = 0; - let textContentItemsStr = this.textContentItemsStr; - let end = textContentItemsStr.length - 1; - let queryLen = (this.findController === null ? - 0 : this.findController.state.query.length); - let ret = []; + _convertMatches(matches, matchesLength) { + // Early exit if there is nothing to convert. if (!matches) { - return ret; + return []; } - for (let m = 0, len = matches.length; m < len; m++) { + const { findController, textContentItemsStr, } = this; + + let i = 0, iIndex = 0; + const end = textContentItemsStr.length - 1; + const queryLen = findController.state.query.length; + const result = []; + + for (let m = 0, mm = matches.length; m < mm; m++) { // Calculate the start position. let matchIdx = matches[m]; // Loop over the divIdxs. - while (i !== end && matchIdx >= - (iIndex + textContentItemsStr[i].length)) { + while (i !== end && + matchIdx >= (iIndex + textContentItemsStr[i].length)) { iIndex += textContentItemsStr[i].length; i++; } @@ -176,8 +177,8 @@ class TextLayerBuilder { // Somewhat the same array as above, but use > instead of >= to get // the end position right. - while (i !== end && matchIdx > - (iIndex + textContentItemsStr[i].length)) { + while (i !== end && + matchIdx > (iIndex + textContentItemsStr[i].length)) { iIndex += textContentItemsStr[i].length; i++; } @@ -186,28 +187,22 @@ class TextLayerBuilder { divIdx: i, offset: matchIdx - iIndex, }; - ret.push(match); + result.push(match); } - - return ret; + return result; } - renderMatches(matches) { + _renderMatches(matches) { // Early exit if there is nothing to render. if (matches.length === 0) { return; } + const { findController, pageIdx, textContentItemsStr, textDivs, } = this; - let textContentItemsStr = this.textContentItemsStr; - let textDivs = this.textDivs; + const isSelectedPage = (pageIdx === findController.selected.pageIdx); + const selectedMatchIdx = findController.selected.matchIdx; + const highlightAll = findController.state.highlightAll; let prevEnd = null; - let pageIdx = this.pageIdx; - let isSelectedPage = (this.findController === null ? - false : (pageIdx === this.findController.selected.pageIdx)); - let selectedMatchIdx = (this.findController === null ? - -1 : this.findController.selected.matchIdx); - let highlightAll = (this.findController === null ? - false : this.findController.state.highlightAll); let infinity = { divIdx: -1, offset: undefined, @@ -250,16 +245,14 @@ class TextLayerBuilder { let highlightSuffix = (isSelected ? ' selected' : ''); // Scroll the selected match into view. - if (this.findController) { - if (this.findController.selected.matchIdx === i && - this.findController.selected.pageIdx === pageIdx) { - const spot = { - top: MATCH_SCROLL_OFFSET_TOP, - left: MATCH_SCROLL_OFFSET_LEFT, - }; - scrollIntoView(textDivs[begin.divIdx], spot, - /* skipOverflowHiddenElements = */ true); - } + if (findController.selected.matchIdx === i && + findController.selected.pageIdx === pageIdx) { + const spot = { + top: MATCH_SCROLL_OFFSET_TOP, + left: MATCH_SCROLL_OFFSET_LEFT, + }; + scrollIntoView(textDivs[begin.divIdx], spot, + /* skipOverflowHiddenElements = */ true); } // Match inside new div. @@ -293,20 +286,18 @@ class TextLayerBuilder { } } - updateMatches() { + _updateMatches() { // Only show matches when all rendering is done. if (!this.renderingDone) { return; } - - // Clear all matches. - let matches = this.matches; - let textDivs = this.textDivs; - let textContentItemsStr = this.textContentItemsStr; + const { + findController, matches, pageIdx, textContentItemsStr, textDivs, + } = this; let clearedUntilDivIdx = -1; // Clear all current matches. - for (let i = 0, len = matches.length; i < len; i++) { + for (let i = 0, ii = matches.length; i < ii; i++) { let match = matches[i]; let begin = Math.max(clearedUntilDivIdx, match.begin.divIdx); for (let n = begin, end = match.end.divIdx; n <= end; n++) { @@ -317,21 +308,16 @@ class TextLayerBuilder { clearedUntilDivIdx = match.end.divIdx + 1; } - if (!this.findController || !this.findController.highlightMatches) { + if (!findController || !findController.highlightMatches) { return; } - - // Convert the matches on the page controller into the match format + // Convert the matches on the `findController` into the match format // used for the textLayer. - let pageMatches, pageMatchesLength; - if (this.findController !== null) { - pageMatches = this.findController.pageMatches[this.pageIdx] || null; - pageMatchesLength = (this.findController.pageMatchesLength) ? - this.findController.pageMatchesLength[this.pageIdx] || null : null; - } + const pageMatches = findController.pageMatches[pageIdx] || null; + const pageMatchesLength = findController.pageMatchesLength[pageIdx] || null; - this.matches = this.convertMatches(pageMatches, pageMatchesLength); - this.renderMatches(this.matches); + this.matches = this._convertMatches(pageMatches, pageMatchesLength); + this._renderMatches(this.matches); } /** @@ -361,7 +347,7 @@ class TextLayerBuilder { if (evt.pageIndex !== this.pageIdx && evt.pageIndex !== -1) { return; } - this.updateMatches(); + this._updateMatches(); }; eventBus.on('pagecancelled', _boundEvents.pageCancelled);