Skip to content

Commit 1190193

Browse files
committed
Add previous/next-page functionality that takes scroll/spread-modes into account (issue 11946)
- For wrapped scrolling, we unfortunately need to do a fair bit of parsing of the *current* page layout. Compared to e.g. the spread-modes, where we can easily tell how the pages are laid out, with wrapped scrolling we cannot tell without actually checking. In particular documents with varying page sizes require some care, since we need to check all pages on the "row" of the current page are visible and that there aren't any "holes" present. Otherwise, in the general case, there's a risk that we'd skip over pages if we'd simply always advance to the previous/next "row" in wrapped scrolling. - For horizontal scrolling, this patch simply maintains the current behaviour of advancing *one* page at a time. The reason for this is to prevent inconsistent behaviour for the next and previous cases, since those cannot be handled identically. For the next-case, it'd obviously be simple to advance to the first not completely visible page. However for the previous-case, we'd only be able to go back *one* page since it's not possible to (easily) determine the page layout of non-visible pages (documents with varying page sizes being a particular issue). - For vertical scrolling, this patch maintains the current behaviour by default. When spread-modes are being used, we'll now attempt to advance to the next *spread*, rather than just the next page, whenever possible. To prevent skipping over a page, this two-page advance will only apply when both pages of the current spread are visible (to avoid breaking documents with varying page sizes) and when the second page in the current spread is fully visible *horizontally* (to handle larger zoom values). In order to reduce the performance impact of these changes, note that the previous/next-functionality will only call `getVisibleElements` for the scroll/spread-modes where that's necessary and that "normal" vertical scrolling is thus unaffected by these changes. To support these changes, the `getVisibleElements` helper function will now also include the `widthPercent` in addition to the existing `percent` property. The `PDFViewer._updateHelper` method is changed slightly w.r.t. updating the `currentPageNumber` for the non-vertical/non-spread modes, i.e. won't affect "normal" vertical scrolling, since that helped simplify the overall calculation of the page advance. Finally, these new `BaseViewer` methods also allow (some) simplification of previous/next-page functionality in various viewer components. *Please note:* There's one thing that this patch does not attempt to change, namely disabling of the previous/next toolbarButtons respectively the firstPage/lastPage secondaryToolbarButtons. The reason for this is that doing so would add quite a bit of complexity in general, and if for some reason `BaseViewer._getPageAdvance` would get things wrong we could end up incorrectly disabling the buttons. Hence it seemed overall safer to *not* touch this, and accept that the buttons won't be `disabled` despite in some edge-cases no further scrolling being possible.
1 parent 9d4bad9 commit 1190193

File tree

8 files changed

+178
-56
lines changed

8 files changed

+178
-56
lines changed

test/unit/ui_utils_spec.js

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -650,11 +650,21 @@ describe("ui_utils", function () {
650650
const hiddenWidth =
651651
Math.max(0, scrollLeft - viewLeft) +
652652
Math.max(0, viewRight - scrollRight);
653-
const visibleArea =
654-
(div.clientHeight - hiddenHeight) * (div.clientWidth - hiddenWidth);
655-
const percent =
656-
((visibleArea * 100) / div.clientHeight / div.clientWidth) | 0;
657-
views.push({ id: view.id, x: viewLeft, y: viewTop, view, percent });
653+
654+
const fractionHeight =
655+
(div.clientHeight - hiddenHeight) / div.clientHeight;
656+
const fractionWidth =
657+
(div.clientWidth - hiddenWidth) / div.clientWidth;
658+
const percent = (fractionHeight * fractionWidth * 100) | 0;
659+
660+
views.push({
661+
id: view.id,
662+
x: viewLeft,
663+
y: viewTop,
664+
view,
665+
percent,
666+
widthPercent: (fractionWidth * 100) | 0,
667+
});
658668
}
659669
}
660670
return { first: views[0], last: views[views.length - 1], views };

web/app.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2829,10 +2829,10 @@ function webViewerLastPage() {
28292829
}
28302830
}
28312831
function webViewerNextPage() {
2832-
PDFViewerApplication.page++;
2832+
PDFViewerApplication.pdfViewer.nextPage();
28332833
}
28342834
function webViewerPreviousPage() {
2835-
PDFViewerApplication.page--;
2835+
PDFViewerApplication.pdfViewer.previousPage();
28362836
}
28372837
function webViewerZoomIn() {
28382838
PDFViewerApplication.zoomIn();
@@ -3351,13 +3351,9 @@ function webViewerKeyDown(evt) {
33513351
(!turnOnlyIfPageFit || pdfViewer.currentScaleValue === "page-fit")
33523352
) {
33533353
if (turnPage > 0) {
3354-
if (PDFViewerApplication.page < PDFViewerApplication.pagesCount) {
3355-
PDFViewerApplication.page++;
3356-
}
3354+
pdfViewer.nextPage();
33573355
} else {
3358-
if (PDFViewerApplication.page > 1) {
3359-
PDFViewerApplication.page--;
3360-
}
3356+
pdfViewer.previousPage();
33613357
}
33623358
handled = true;
33633359
}

web/base_viewer.js

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,6 +1507,140 @@ class BaseViewer {
15071507
this.update();
15081508
}
15091509

1510+
/**
1511+
* @private
1512+
*/
1513+
_getPageAdvance(currentPageNumber, previous = false) {
1514+
if (this.isInPresentationMode) {
1515+
return 1;
1516+
}
1517+
switch (this._scrollMode) {
1518+
case ScrollMode.WRAPPED: {
1519+
const { views } = this._getVisiblePages(),
1520+
pageLayout = new Map();
1521+
1522+
// Determine the current (visible) page layout.
1523+
for (const { id, y, percent, widthPercent } of views) {
1524+
if (percent === 0 || widthPercent < 100) {
1525+
continue;
1526+
}
1527+
let yArray = pageLayout.get(y);
1528+
if (!yArray) {
1529+
pageLayout.set(y, (yArray ||= []));
1530+
}
1531+
yArray.push(id);
1532+
}
1533+
// Find the row of the current page.
1534+
for (const yArray of pageLayout.values()) {
1535+
const currentIndex = yArray.indexOf(currentPageNumber);
1536+
if (currentIndex === -1) {
1537+
continue;
1538+
}
1539+
const numPages = yArray.length;
1540+
if (numPages === 1) {
1541+
break;
1542+
}
1543+
// Handle documents with varying page sizes.
1544+
if (previous) {
1545+
for (let i = currentIndex - 1, ii = 0; i >= ii; i--) {
1546+
const currentId = yArray[i],
1547+
expectedId = yArray[i + 1] - 1;
1548+
if (currentId < expectedId) {
1549+
return currentPageNumber - expectedId;
1550+
}
1551+
}
1552+
} else {
1553+
for (let i = currentIndex + 1, ii = numPages; i < ii; i++) {
1554+
const currentId = yArray[i],
1555+
expectedId = yArray[i - 1] + 1;
1556+
if (currentId > expectedId) {
1557+
return expectedId - currentPageNumber;
1558+
}
1559+
}
1560+
}
1561+
// The current row is "complete", advance to the previous/next one.
1562+
if (previous) {
1563+
const firstId = yArray[0];
1564+
if (firstId < currentPageNumber) {
1565+
return currentPageNumber - firstId + 1;
1566+
}
1567+
} else {
1568+
const lastId = yArray[numPages - 1];
1569+
if (lastId > currentPageNumber) {
1570+
return lastId - currentPageNumber + 1;
1571+
}
1572+
}
1573+
break;
1574+
}
1575+
break;
1576+
}
1577+
case ScrollMode.HORIZONTAL: {
1578+
break;
1579+
}
1580+
case ScrollMode.VERTICAL: {
1581+
if (this._spreadMode === SpreadMode.NONE) {
1582+
break; // Normal vertical scrolling.
1583+
}
1584+
const parity = this._spreadMode - 1;
1585+
1586+
if (previous && currentPageNumber % 2 !== parity) {
1587+
break; // Left-hand side page.
1588+
} else if (!previous && currentPageNumber % 2 === parity) {
1589+
break; // Right-hand side page.
1590+
}
1591+
const { views } = this._getVisiblePages(),
1592+
expectedId = previous ? currentPageNumber - 1 : currentPageNumber + 1;
1593+
1594+
for (const { id, percent, widthPercent } of views) {
1595+
if (id !== expectedId) {
1596+
continue;
1597+
}
1598+
if (percent > 0 && widthPercent === 100) {
1599+
return 2;
1600+
}
1601+
break;
1602+
}
1603+
break;
1604+
}
1605+
}
1606+
return 1;
1607+
}
1608+
1609+
/**
1610+
* Go to the next page, taking scroll/spread-modes into account.
1611+
* @returns {boolean} Whether navigation occured.
1612+
*/
1613+
nextPage() {
1614+
const currentPageNumber = this._currentPageNumber,
1615+
pagesCount = this.pagesCount;
1616+
1617+
if (currentPageNumber >= pagesCount) {
1618+
return false;
1619+
}
1620+
const advance =
1621+
this._getPageAdvance(currentPageNumber, /* previous = */ false) || 1;
1622+
1623+
this.currentPageNumber = Math.min(currentPageNumber + advance, pagesCount);
1624+
return true;
1625+
}
1626+
1627+
/**
1628+
* Go to the previous page, taking scroll/spread-modes into account.
1629+
* @returns {boolean} Whether navigation occured.
1630+
*/
1631+
previousPage() {
1632+
const currentPageNumber = this._currentPageNumber;
1633+
1634+
if (currentPageNumber <= 1) {
1635+
return false;
1636+
}
1637+
const advance =
1638+
this._getPageAdvance(currentPageNumber, /* previous = */ true) || 1;
1639+
1640+
this.currentPageNumber = Math.max(currentPageNumber - advance, 1);
1641+
return true;
1642+
}
1643+
15101644
initializeScriptingEvents() {
15111645
if (!this.enableScripting || this._pageOpenPendingSet) {
15121646
return;

web/pdf_link_service.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -401,15 +401,11 @@ class PDFLinkService {
401401
break;
402402

403403
case "NextPage":
404-
if (this.page < this.pagesCount) {
405-
this.page++;
406-
}
404+
this.pdfViewer.nextPage();
407405
break;
408406

409407
case "PrevPage":
410-
if (this.page > 1) {
411-
this.page--;
412-
}
408+
this.pdfViewer.previousPage();
413409
break;
414410

415411
case "LastPage":

web/pdf_presentation_mode.js

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ class PDFPresentationMode {
138138
const totalDelta = this.mouseScrollDelta;
139139
this._resetMouseScrollState();
140140
const success =
141-
totalDelta > 0 ? this._goToPreviousPage() : this._goToNextPage();
141+
totalDelta > 0
142+
? this.pdfViewer.previousPage()
143+
: this.pdfViewer.nextPage();
142144
if (success) {
143145
this.mouseScrollTimeStamp = currentTime;
144146
}
@@ -153,32 +155,6 @@ class PDFPresentationMode {
153155
);
154156
}
155157

156-
/**
157-
* @private
158-
*/
159-
_goToPreviousPage() {
160-
const page = this.pdfViewer.currentPageNumber;
161-
// If we're at the first page, we don't need to do anything.
162-
if (page <= 1) {
163-
return false;
164-
}
165-
this.pdfViewer.currentPageNumber = page - 1;
166-
return true;
167-
}
168-
169-
/**
170-
* @private
171-
*/
172-
_goToNextPage() {
173-
const page = this.pdfViewer.currentPageNumber;
174-
// If we're at the last page, we don't need to do anything.
175-
if (page >= this.pdfViewer.pagesCount) {
176-
return false;
177-
}
178-
this.pdfViewer.currentPageNumber = page + 1;
179-
return true;
180-
}
181-
182158
/**
183159
* @private
184160
*/
@@ -315,9 +291,9 @@ class PDFPresentationMode {
315291
evt.preventDefault();
316292

317293
if (evt.shiftKey) {
318-
this._goToPreviousPage();
294+
this.pdfViewer.previousPage();
319295
} else {
320-
this._goToNextPage();
296+
this.pdfViewer.nextPage();
321297
}
322298
}
323299
}
@@ -422,9 +398,9 @@ class PDFPresentationMode {
422398
delta = dy;
423399
}
424400
if (delta > 0) {
425-
this._goToPreviousPage();
401+
this.pdfViewer.previousPage();
426402
} else if (delta < 0) {
427-
this._goToNextPage();
403+
this.pdfViewer.nextPage();
428404
}
429405
break;
430406
}

web/pdf_single_page_viewer.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ class PDFSinglePageViewer extends BaseViewer {
121121
_updateScrollMode() {}
122122

123123
_updateSpreadMode() {}
124+
125+
_getPageAdvance() {
126+
return 1;
127+
}
124128
}
125129

126130
export { PDFSinglePageViewer };

web/pdf_viewer.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
* limitations under the License.
1414
*/
1515

16+
import { ScrollMode, SpreadMode } from "./ui_utils.js";
1617
import { BaseViewer } from "./base_viewer.js";
1718
import { shadow } from "pdfjs-lib";
1819

@@ -57,7 +58,11 @@ class PDFViewer extends BaseViewer {
5758
if (page.percent < 100) {
5859
break;
5960
}
60-
if (page.id === currentId) {
61+
if (
62+
page.id === currentId &&
63+
this._scrollMode === ScrollMode.VERTICAL &&
64+
this._spreadMode === SpreadMode.NONE
65+
) {
6166
stillFullyVisible = true;
6267
break;
6368
}

web/ui_utils.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -563,17 +563,18 @@ function getVisibleElements({
563563
Math.max(0, top - currentHeight) + Math.max(0, viewBottom - bottom);
564564
const hiddenWidth =
565565
Math.max(0, left - currentWidth) + Math.max(0, viewRight - right);
566-
const percent =
567-
(((viewHeight - hiddenHeight) * (viewWidth - hiddenWidth) * 100) /
568-
viewHeight /
569-
viewWidth) |
570-
0;
566+
567+
const fractionHeight = (viewHeight - hiddenHeight) / viewHeight,
568+
fractionWidth = (viewWidth - hiddenWidth) / viewWidth;
569+
const percent = (fractionHeight * fractionWidth * 100) | 0;
570+
571571
visible.push({
572572
id: view.id,
573573
x: currentWidth,
574574
y: currentHeight,
575575
view,
576576
percent,
577+
widthPercent: (fractionWidth * 100) | 0,
577578
});
578579
}
579580

0 commit comments

Comments
 (0)