Skip to content

Enforce PAGE-scrolling for *very* large/long documents (bug 1588435, PR 11263 follow-up) #14324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 2, 2021
Merged

Enforce PAGE-scrolling for *very* large/long documents (bug 1588435, PR 11263 follow-up) #14324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f15eb63
Remove the `PDFSinglePageViewer`-specific code from `web/secondary_to…
Snuffleupagus Nov 28, 2021
6dfe4a9
Enforce PAGE-scrolling for *very* large/long documents (bug 1588435, …
Snuffleupagus Nov 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions web/base_viewer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js";

const DEFAULT_CACHE_SIZE = 10;

const PagesCountLimit = {
FORCE_SCROLL_MODE_PAGE: 15000,
FORCE_LAZY_PAGE_INIT: 7500,
};

/**
* @typedef {Object} PDFViewerOptions
* @property {HTMLDivElement} container - The container for the viewer element.
Expand Down Expand Up @@ -515,6 +520,16 @@ class BaseViewer {
// Rendering (potentially) depends on this, hence fetching it immediately.
const optionalContentConfigPromise = pdfDocument.getOptionalContentConfig();

// Given that browsers don't handle huge amounts of DOM-elements very well,
// enforce usage of PAGE-scrolling when loading *very* long/large documents.
if (pagesCount > PagesCountLimit.FORCE_SCROLL_MODE_PAGE) {
console.warn(
"Forcing PAGE-scrolling for performance reasons, given the length of the document."
);
const mode = (this._scrollMode = ScrollMode.PAGE);
this.eventBus.dispatch("scrollmodechanged", { source: this, mode });
}

this._pagesCapability.promise.then(() => {
this.eventBus.dispatch("pagesloaded", {
source: this,
Expand Down Expand Up @@ -618,7 +633,10 @@ class BaseViewer {

// In addition to 'disableAutoFetch' being set, also attempt to reduce
// resource usage when loading *very* long/large documents.
if (pdfDocument.loadingParams.disableAutoFetch || pagesCount > 7500) {
if (
pdfDocument.loadingParams.disableAutoFetch ||
pagesCount > PagesCountLimit.FORCE_LAZY_PAGE_INIT
) {
// XXX: Printing is semi-broken with auto fetch disabled.
this._pagesCapability.resolve();
return;
Expand Down Expand Up @@ -1685,6 +1703,9 @@ class BaseViewer {
if (!isValidScrollMode(mode)) {
throw new Error(`Invalid scroll mode: ${mode}`);
}
if (this.pagesCount > PagesCountLimit.FORCE_SCROLL_MODE_PAGE) {
return; // Disabled for performance reasons.
}
this._previousScrollMode = this._scrollMode;

this._scrollMode = mode;
Expand Down Expand Up @@ -1957,4 +1978,4 @@ class BaseViewer {
}
}

export { BaseViewer, PDFPageViewBuffer };
export { BaseViewer, PagesCountLimit, PDFPageViewBuffer };
31 changes: 12 additions & 19 deletions web/secondary_toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

import { SCROLLBAR_PADDING, ScrollMode, SpreadMode } from "./ui_utils.js";
import { CursorTool } from "./pdf_cursor_tools.js";
import { PDFSinglePageViewer } from "./pdf_viewer.js";
import { PagesCountLimit } from "./base_viewer.js";

/**
* @typedef {Object} SecondaryToolbarOptions
Expand Down Expand Up @@ -166,22 +166,6 @@ class SecondaryToolbar {

// Bind the event listener for adjusting the 'max-height' of the toolbar.
this.eventBus._on("resize", this._setMaxHeight.bind(this));

// Hide the Scroll/Spread mode buttons, when they're not applicable to the
// current `BaseViewer` instance (in particular `PDFSinglePageViewer`).
this.eventBus._on("baseviewerinit", evt => {
if (evt.source instanceof PDFSinglePageViewer) {
this.toolbarButtonContainer.classList.add(
"hiddenScrollModeButtons",
"hiddenSpreadModeButtons"
);
} else {
this.toolbarButtonContainer.classList.remove(
"hiddenScrollModeButtons",
"hiddenSpreadModeButtons"
);
}
});
}

/**
Expand Down Expand Up @@ -252,7 +236,7 @@ class SecondaryToolbar {
}

_bindScrollModeListener(buttons) {
function scrollModeChanged({ mode }) {
const scrollModeChanged = ({ mode }) => {
buttons.scrollPageButton.classList.toggle(
"toggled",
mode === ScrollMode.PAGE
Expand All @@ -270,13 +254,22 @@ class SecondaryToolbar {
mode === ScrollMode.WRAPPED
);

// Permanently *disable* the Scroll buttons when PAGE-scrolling is being
// enforced for *very* long/large documents; please see the `BaseViewer`.
const forceScrollModePage =
this.pagesCount > PagesCountLimit.FORCE_SCROLL_MODE_PAGE;
buttons.scrollPageButton.disabled = forceScrollModePage;
buttons.scrollVerticalButton.disabled = forceScrollModePage;
buttons.scrollHorizontalButton.disabled = forceScrollModePage;
buttons.scrollWrappedButton.disabled = forceScrollModePage;

// Temporarily *disable* the Spread buttons when horizontal scrolling is
// enabled, since the non-default Spread modes doesn't affect the layout.
const isScrollModeHorizontal = mode === ScrollMode.HORIZONTAL;
buttons.spreadNoneButton.disabled = isScrollModeHorizontal;
buttons.spreadOddButton.disabled = isScrollModeHorizontal;
buttons.spreadEvenButton.disabled = isScrollModeHorizontal;
}
};
this.eventBus._on("scrollmodechanged", scrollModeChanged);

this.eventBus._on("secondarytoolbarreset", evt => {
Expand Down
5 changes: 0 additions & 5 deletions web/viewer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,11 +625,6 @@ html[dir="rtl"] .secondaryToolbar {
margin-bottom: -4px;
}

#secondaryToolbarButtonContainer.hiddenScrollModeButtons > .scrollModeButtons,
#secondaryToolbarButtonContainer.hiddenSpreadModeButtons > .spreadModeButtons {
display: none !important;
}

.doorHanger,
.doorHangerRight {
border-radius: 2px;
Expand Down
18 changes: 9 additions & 9 deletions web/viewer.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,32 +196,32 @@

<div class="horizontalToolbarSeparator"></div>

<button id="scrollPage" class="secondaryToolbarButton scrollModeButtons scrollPage" title="Use Page Scrolling" tabindex="62" data-l10n-id="scroll_page">
<button id="scrollPage" class="secondaryToolbarButton scrollPage" title="Use Page Scrolling" tabindex="62" data-l10n-id="scroll_page">
<span data-l10n-id="scroll_page_label">Page Scrolling</span>
</button>
<button id="scrollVertical" class="secondaryToolbarButton scrollModeButtons scrollVertical toggled" title="Use Vertical Scrolling" tabindex="63" data-l10n-id="scroll_vertical">
<button id="scrollVertical" class="secondaryToolbarButton scrollVertical toggled" title="Use Vertical Scrolling" tabindex="63" data-l10n-id="scroll_vertical">
<span data-l10n-id="scroll_vertical_label">Vertical Scrolling</span>
</button>
<button id="scrollHorizontal" class="secondaryToolbarButton scrollModeButtons scrollHorizontal" title="Use Horizontal Scrolling" tabindex="64" data-l10n-id="scroll_horizontal">
<button id="scrollHorizontal" class="secondaryToolbarButton scrollHorizontal" title="Use Horizontal Scrolling" tabindex="64" data-l10n-id="scroll_horizontal">
<span data-l10n-id="scroll_horizontal_label">Horizontal Scrolling</span>
</button>
<button id="scrollWrapped" class="secondaryToolbarButton scrollModeButtons scrollWrapped" title="Use Wrapped Scrolling" tabindex="65" data-l10n-id="scroll_wrapped">
<button id="scrollWrapped" class="secondaryToolbarButton scrollWrapped" title="Use Wrapped Scrolling" tabindex="65" data-l10n-id="scroll_wrapped">
<span data-l10n-id="scroll_wrapped_label">Wrapped Scrolling</span>
</button>

<div class="horizontalToolbarSeparator scrollModeButtons"></div>
<div class="horizontalToolbarSeparator"></div>

<button id="spreadNone" class="secondaryToolbarButton spreadModeButtons spreadNone toggled" title="Do not join page spreads" tabindex="66" data-l10n-id="spread_none">
<button id="spreadNone" class="secondaryToolbarButton spreadNone toggled" title="Do not join page spreads" tabindex="66" data-l10n-id="spread_none">
<span data-l10n-id="spread_none_label">No Spreads</span>
</button>
<button id="spreadOdd" class="secondaryToolbarButton spreadModeButtons spreadOdd" title="Join page spreads starting with odd-numbered pages" tabindex="67" data-l10n-id="spread_odd">
<button id="spreadOdd" class="secondaryToolbarButton spreadOdd" title="Join page spreads starting with odd-numbered pages" tabindex="67" data-l10n-id="spread_odd">
<span data-l10n-id="spread_odd_label">Odd Spreads</span>
</button>
<button id="spreadEven" class="secondaryToolbarButton spreadModeButtons spreadEven" title="Join page spreads starting with even-numbered pages" tabindex="68" data-l10n-id="spread_even">
<button id="spreadEven" class="secondaryToolbarButton spreadEven" title="Join page spreads starting with even-numbered pages" tabindex="68" data-l10n-id="spread_even">
<span data-l10n-id="spread_even_label">Even Spreads</span>
</button>

<div class="horizontalToolbarSeparator spreadModeButtons"></div>
<div class="horizontalToolbarSeparator"></div>

<button id="documentProperties" class="secondaryToolbarButton documentProperties" title="Document Properties…" tabindex="69" data-l10n-id="document_properties">
<span data-l10n-id="document_properties_label">Document Properties…</span>
Expand Down