diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index 1bbb175d6..aaab50cd2 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -690,6 +690,66 @@ describe('ui_utils', function() { scrollOverDocument(pages, true); }); + it('handles `sortByVisibility` correctly', function() { + const scrollEl = { + scrollTop: 75, + scrollLeft: 0, + clientHeight: 750, + clientWidth: 1500, + }; + const views = makePages([ + [[100, 150]], + [[100, 150]], + [[100, 150]], + ]); + + const visible = getVisibleElements(scrollEl, views); + const visibleSorted = getVisibleElements(scrollEl, views, + /* sortByVisibility = */ true); + + const viewsOrder = [], viewsSortedOrder = []; + for (const view of visible.views) { + viewsOrder.push(view.id); + } + for (const view of visibleSorted.views) { + viewsSortedOrder.push(view.id); + } + expect(viewsOrder).toEqual([0, 1, 2]); + expect(viewsSortedOrder).toEqual([1, 2, 0]); + }); + + it('handles views being empty', function() { + const scrollEl = { + scrollTop: 10, + scrollLeft: 0, + clientHeight: 750, + clientWidth: 1500, + }; + const views = []; + + expect(getVisibleElements(scrollEl, views)).toEqual({ + first: undefined, last: undefined, views: [], + }); + }); + + it('handles all views being hidden (without errors)', function() { + const scrollEl = { + scrollTop: 100000, + scrollLeft: 0, + clientHeight: 750, + clientWidth: 1500, + }; + const views = makePages([ + [[100, 150]], + [[100, 150]], + [[100, 150]], + ]); + + expect(getVisibleElements(scrollEl, views)).toEqual({ + first: undefined, last: undefined, views: [], + }); + }); + // This sub-suite is for a notionally internal helper function for // getVisibleElements. describe('backtrackBeforeAllVisibleElements', function() { diff --git a/web/app.js b/web/app.js index cdff6173a..26da5e818 100644 --- a/web/app.js +++ b/web/app.js @@ -996,6 +996,10 @@ let PDFViewerApplication = { pdfViewer.currentScaleValue = pdfViewer.currentScaleValue; // Re-apply the initial document location. this.setInitialView(hash); + }).catch(() => { + // Ensure that the document is always completely initialized, + // even if there are any errors thrown above. + this.setInitialView(); }).then(function() { // At this point, rendering of the initial page(s) should always have // started (and may even have completed). diff --git a/web/ui_utils.js b/web/ui_utils.js index 751d9f4fb..7c1a6c110 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -413,8 +413,8 @@ function backtrackBeforeAllVisibleElements(index, views, top) { */ function getVisibleElements(scrollEl, views, sortByVisibility = false, horizontal = false) { - let top = scrollEl.scrollTop, bottom = top + scrollEl.clientHeight; - let left = scrollEl.scrollLeft, right = left + scrollEl.clientWidth; + const top = scrollEl.scrollTop, bottom = top + scrollEl.clientHeight; + const left = scrollEl.scrollLeft, right = left + scrollEl.clientWidth; // Throughout this "generic" function, comments will assume we're working with // PDF document pages, which is the most important and complex case. In this @@ -427,27 +427,26 @@ function getVisibleElements(scrollEl, views, sortByVisibility = false, // the border). Adding clientWidth/Height gets us the bottom-right corner of // the padding edge. function isElementBottomAfterViewTop(view) { - let element = view.div; - let elementBottom = + const element = view.div; + const elementBottom = element.offsetTop + element.clientTop + element.clientHeight; return elementBottom > top; } function isElementRightAfterViewLeft(view) { - let element = view.div; - let elementRight = + const element = view.div; + const elementRight = element.offsetLeft + element.clientLeft + element.clientWidth; return elementRight > left; } - let visible = [], view, element; - let currentHeight, viewHeight, viewBottom, hiddenHeight; - let currentWidth, viewWidth, viewRight, hiddenWidth; - let percentVisible; - let firstVisibleElementInd = views.length === 0 ? 0 : + const visible = [], numViews = views.length; + let firstVisibleElementInd = numViews === 0 ? 0 : binarySearchFirstItem(views, horizontal ? isElementRightAfterViewLeft : isElementBottomAfterViewTop); - if (views.length > 0 && !horizontal) { + // Please note the return value of the `binarySearchFirstItem` function when + // no valid element is found (hence the `firstVisibleElementInd` check below). + if (numViews > 0 && firstVisibleElementInd < numViews && !horizontal) { // In wrapped scrolling (or vertical scrolling with spreads), with some page // sizes, isElementBottomAfterViewTop doesn't satisfy the binary search // condition: there can be pages with bottoms above the view top between @@ -467,15 +466,13 @@ function getVisibleElements(scrollEl, views, sortByVisibility = false, // we pass `right`, without needing the code below that handles the -1 case. let lastEdge = horizontal ? right : -1; - for (let i = firstVisibleElementInd, ii = views.length; i < ii; i++) { - view = views[i]; - element = view.div; - currentWidth = element.offsetLeft + element.clientLeft; - currentHeight = element.offsetTop + element.clientTop; - viewWidth = element.clientWidth; - viewHeight = element.clientHeight; - viewRight = currentWidth + viewWidth; - viewBottom = currentHeight + viewHeight; + for (let i = firstVisibleElementInd; i < numViews; i++) { + const view = views[i], element = view.div; + const currentWidth = element.offsetLeft + element.clientLeft; + const currentHeight = element.offsetTop + element.clientTop; + const viewWidth = element.clientWidth, viewHeight = element.clientHeight; + const viewRight = currentWidth + viewWidth; + const viewBottom = currentHeight + viewHeight; if (lastEdge === -1) { // As commented above, this is only needed in non-horizontal cases. @@ -494,24 +491,22 @@ function getVisibleElements(scrollEl, views, sortByVisibility = false, continue; } - hiddenHeight = Math.max(0, top - currentHeight) + - Math.max(0, viewBottom - bottom); - hiddenWidth = Math.max(0, left - currentWidth) + - Math.max(0, viewRight - right); - percentVisible = ((viewHeight - hiddenHeight) * (viewWidth - hiddenWidth) * - 100 / viewHeight / viewWidth) | 0; - + const hiddenHeight = Math.max(0, top - currentHeight) + + Math.max(0, viewBottom - bottom); + const hiddenWidth = Math.max(0, left - currentWidth) + + Math.max(0, viewRight - right); + const percent = ((viewHeight - hiddenHeight) * (viewWidth - hiddenWidth) * + 100 / viewHeight / viewWidth) | 0; visible.push({ id: view.id, x: currentWidth, y: currentHeight, view, - percent: percentVisible, + percent, }); } - let first = visible[0]; - let last = visible[visible.length - 1]; + const first = visible[0], last = visible[visible.length - 1]; if (sortByVisibility) { visible.sort(function(a, b) {