From 7aa6a1f11463de77ca42411d39485719e2e00a9f Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 29 Feb 2024 13:31:49 -0800 Subject: [PATCH] EPUB/snapshot: Replace NavStack with History Fixes zotero/zotero#3745 --- .../lib/history.js => common/lib/history.ts} | 32 +++++++-- src/common/lib/utilities.js | 24 +++++++ src/common/types.ts | 1 + src/dom/common/dom-view.tsx | 40 +++++++++-- src/dom/common/lib/nav-stack.ts | 41 ------------ src/dom/epub/epub-view.ts | 66 +++++++------------ src/dom/epub/flow.ts | 16 ++++- src/dom/snapshot/snapshot-view.ts | 38 ++++------- src/pdf/lib/utilities.js | 24 ------- src/pdf/pdf-view.js | 4 +- 10 files changed, 138 insertions(+), 148 deletions(-) rename src/{pdf/lib/history.js => common/lib/history.ts} (71%) delete mode 100644 src/dom/common/lib/nav-stack.ts diff --git a/src/pdf/lib/history.js b/src/common/lib/history.ts similarity index 71% rename from src/pdf/lib/history.js rename to src/common/lib/history.ts index ea99fa81..ad9d388e 100644 --- a/src/pdf/lib/history.js +++ b/src/common/lib/history.ts @@ -1,7 +1,23 @@ +import { NavLocation } from "../types"; + const MIN_TIME_BETWEEN_POINTS = 2000; export class History { - constructor(props) { + private readonly _onUpdate: () => void; + + private readonly _onNavigate: (location: NavLocation) => void; + + private _backStack: NavLocation[]; + + private _forwardStack: NavLocation[]; + + private _currentLocation: NavLocation | null; + + private _lastPushIsTransient: boolean; + + private _lastSaveTime: number; + + constructor(props: { onUpdate: () => void, onNavigate: (location: NavLocation) => void }) { this._onUpdate = props.onUpdate; this._onNavigate = props.onNavigate; this._backStack = []; @@ -26,7 +42,7 @@ export class History { * @param transient Whether the location is temporary and not important enough * to be saved as a standalone point in history */ - save(location, transient) { + save(location: NavLocation, transient = false) { // Check if (JSON.stringify(location) === JSON.stringify(this._currentLocation)) { return; @@ -58,8 +74,10 @@ export class History { navigateBack() { if (this.canNavigateBack) { - this._forwardStack.push(this._currentLocation); - this._currentLocation = this._backStack.pop(); + if (this._currentLocation) { + this._forwardStack.push(this._currentLocation); + } + this._currentLocation = this._backStack.pop()!; this._onNavigate(this._currentLocation); this._onUpdate(); } @@ -67,8 +85,10 @@ export class History { navigateForward() { if (this.canNavigateForward) { - this._backStack.push(this._currentLocation); - this._currentLocation = this._forwardStack.pop(); + if (this._currentLocation) { + this._backStack.push(this._currentLocation); + } + this._currentLocation = this._forwardStack.pop()!; this._onNavigate(this._currentLocation); this._onUpdate(); } diff --git a/src/common/lib/utilities.js b/src/common/lib/utilities.js index 1b9b2bf1..aaf66777 100644 --- a/src/common/lib/utilities.js +++ b/src/common/lib/utilities.js @@ -212,6 +212,30 @@ export function getAffectedAnnotations(oldAnnotations, newAnnotations, viewOnly) return { created, updated, deleted }; } +/** + * Wait until scroll is no longer being triggered + * @param {Document | Element} container Scrollable container + * @param {number} debounceTime For how long the scroll shouldn't be triggered + * @returns {Promise} + */ +export function debounceUntilScrollFinishes(container, debounceTime = 100) { + return new Promise((resolve) => { + let debounceTimeout; + let resolveAndCleanup = () => { + container.removeEventListener('scroll', scrollListener); + clearTimeout(debounceTimeout); + resolve(); + }; + let scrollListener = () => { + clearTimeout(debounceTimeout); + debounceTimeout = setTimeout(resolveAndCleanup, debounceTime); + }; + container.addEventListener('scroll', scrollListener); + // Start the debounce timeout immediately + debounceTimeout = setTimeout(resolveAndCleanup, debounceTime); + }); +} + // findLastIndex polyfill if (!Array.prototype.findLastIndex) { Array.prototype.findLastIndex = function (callback, thisArg) { diff --git a/src/common/types.ts b/src/common/types.ts index e46a32f2..ac848fe2 100644 --- a/src/common/types.ts +++ b/src/common/types.ts @@ -56,6 +56,7 @@ export type NavLocation = { annotationID?: string; position?: Position; href?: string; + scrollCoords?: [number, number]; }; export type Position = PDFPosition | Selector; diff --git a/src/dom/common/dom-view.tsx b/src/dom/common/dom-view.tsx index 5ad53b2a..a64deff6 100644 --- a/src/dom/common/dom-view.tsx +++ b/src/dom/common/dom-view.tsx @@ -1,3 +1,6 @@ +// @ts-ignore +import annotationOverlayCSS from '!!raw-loader!./stylesheets/annotation-overlay.css'; + import { Annotation, AnnotationPopupParams, @@ -32,18 +35,17 @@ import { import { getSelectionRanges } from "./lib/selection"; import { FindProcessor } from "./find"; import { SELECTION_COLOR } from "../../common/defines"; -import { isSafari } from "../../common/lib/utilities"; +import { debounceUntilScrollFinishes, isSafari } from "../../common/lib/utilities"; import { closestElement, isElement } from "./lib/nodes"; import { debounce } from "../../common/lib/debounce"; -// @ts-ignore -import annotationOverlayCSS from '!!raw-loader!./stylesheets/annotation-overlay.css'; import { getBoundingRect, rectContains } from "./lib/rect"; +import { History } from "../../common/lib/history"; abstract class DOMView { initializedPromise: Promise; @@ -82,6 +84,10 @@ abstract class DOMView { protected _overlayPopupDelayer: PopupDelayer; + protected readonly _history: History; + + protected _suspendHistorySaving = false; + protected _highlightedPosition: Selector | null = null; protected _pointerMovedWhileDown = false; @@ -111,6 +117,10 @@ abstract class DOMView { this._overlayPopup = options.overlayPopup; this._findState = options.findState; this._overlayPopupDelayer = new PopupDelayer({ open: !!this._overlayPopup }); + this._history = new History({ + onUpdate: () => this._updateViewStats(), + onNavigate: location => this.navigate(location, { skipHistory: true, behavior: 'auto' }), + }); this._iframe = document.createElement('iframe'); this._iframe.sandbox.add('allow-same-origin'); @@ -192,6 +202,8 @@ abstract class DOMView { // Abstractions over document structure // *** + protected abstract _getHistoryLocation(): NavLocation | null; + protected abstract _getAnnotationFromRange(range: Range, type: AnnotationType, color?: string): NewAnnotation | null; protected abstract _updateViewState(): void; @@ -202,6 +214,18 @@ abstract class DOMView { // Utilities - called in appropriate event handlers // *** + protected async _pushHistoryPoint(transient = false) { + if (!transient) { + this._suspendHistorySaving = true; + await debounceUntilScrollFinishes(this._iframeDocument, 100); + this._suspendHistorySaving = false; + } + + let loc = this._getHistoryLocation(); + if (!loc) return; + this._history.save(loc, transient); + } + protected _isExternalLink(link: HTMLAnchorElement): boolean { let href = link.getAttribute('href'); if (!href) { @@ -1143,6 +1167,14 @@ abstract class DOMView { }, 2000); } } + + navigateBack() { + this._history.navigateBack(); + } + + navigateForward() { + this._history.navigateForward(); + } } export type DOMViewOptions = { @@ -1196,7 +1228,7 @@ export interface CustomScrollIntoViewOptions extends Omit { - private _backStack: T[] = []; - - private _forwardStack: T[] = []; - - canPopBack(): boolean { - return !!this._backStack.length; - } - - canPopForward(): boolean { - return !!this._backStack.length; - } - - push(value: T) { - if (this._backStack.length && value === this._backStack[this._backStack.length - 1]) { - return; - } - this._backStack.push(value); - this._forwardStack.length = 0; - } - - popBack(): T { - let value = this._backStack.pop(); - if (!value) { - throw new Error('Back stack empty'); - } - this._forwardStack.push(value); - return value; - } - - popForward(): T { - let value = this._forwardStack.pop(); - if (!value) { - throw new Error('Forward stack empty'); - } - this._backStack.push(value); - return value; - } -} - -export default NavStack; diff --git a/src/dom/epub/epub-view.ts b/src/dom/epub/epub-view.ts index 08f7d442..df47252b 100644 --- a/src/dom/epub/epub-view.ts +++ b/src/dom/epub/epub-view.ts @@ -1,3 +1,10 @@ +// @ts-expect-error +import injectCSS from './stylesheets/inject.scss'; + +// The module resolver is incapable of understanding this +// @ts-ignore +import Path from "epubjs/src/utils/path"; + import { AnnotationType, ArrayRect, @@ -26,7 +33,6 @@ import { Selector } from "../common/lib/selector"; import { EPUBFindProcessor } from "./find"; -import NavStack from "../common/lib/nav-stack"; import DOMView, { DOMViewOptions, DOMViewState, @@ -51,13 +57,6 @@ import { } from "./flow"; import { RTL_SCRIPTS } from "./defines"; -// @ts-expect-error -import injectCSS from './stylesheets/inject.scss'; - -// The module resolver is incapable of understanding this -// @ts-ignore -import Path from "epubjs/src/utils/path"; - class EPUBView extends DOMView { protected _find: EPUBFindProcessor | null = null; @@ -75,8 +74,6 @@ class EPUBView extends DOMView { private readonly _sectionViews: SectionView[] = []; - private readonly _navStack = new NavStack(); - private readonly _rangeCache = new Map(); private _pageProgressionRTL!: boolean; @@ -337,16 +334,14 @@ class EPUBView extends DOMView { } } - get views(): SectionView[] { - return this._sectionViews; + protected override _getHistoryLocation(): NavLocation | null { + let cfi = this.flow.startCFI?.toString(); + if (!cfi) return null; + return { pageNumber: cfi }; } - private _pushCurrentLocationToNavStack() { - let cfi = this.flow.startCFI?.toString(); - if (cfi) { - this._navStack.push(cfi); - this._updateViewStats(); - } + get views(): SectionView[] { + return this._sectionViews; } protected _navigateToSelector(selector: Selector, options: NavigateOptions = {}) { @@ -430,7 +425,7 @@ class EPUBView extends DOMView { this.navigate( { pageNumber: beforeCFI.toString() }, { - skipNavStack: true, + skipHistory: true, behavior: 'auto', offsetY: beforeOffset ?? undefined } @@ -547,8 +542,8 @@ class EPUBView extends DOMView { canZoomIn: this.scale === undefined || this.scale < 1.5, canZoomOut: this.scale === undefined || this.scale > 0.8, canZoomReset: this.scale !== undefined && this.scale !== 1, - canNavigateBack: this._navStack.canPopBack(), - canNavigateForward: this._navStack.canPopForward(), + canNavigateBack: this._history.canNavigateBack, + canNavigateForward: this._history.canNavigateForward, canNavigateToFirstPage: canNavigateToPreviousPage, canNavigateToLastPage: canNavigateToNextPage, canNavigateToPreviousPage, @@ -769,11 +764,12 @@ class EPUBView extends DOMView { onUpdateViewState: () => this._updateViewState(), onUpdateViewStats: () => this._updateViewStats(), onViewUpdate: () => this._handleViewUpdate(), + onPushHistoryPoint: () => this._pushHistoryPoint(true), }); this.flow.setSpreadMode(this.spreadMode); if (cfiBefore) { - this.navigate({ pageNumber: cfiBefore.toString() }, { skipNavStack: true, behavior: 'auto' }); + this.navigate({ pageNumber: cfiBefore.toString() }, { skipHistory: true, behavior: 'auto' }); } this._handleViewUpdate(); } @@ -791,7 +787,7 @@ class EPUBView extends DOMView { this.spreadMode = spreadMode; this.flow?.setSpreadMode(spreadMode); if (cfiBefore) { - this.navigate({ pageNumber: cfiBefore.toString() }, { skipNavStack: true, behavior: 'auto' }); + this.navigate({ pageNumber: cfiBefore.toString() }, { skipHistory: true, behavior: 'auto' }); } this._handleViewUpdate(); } @@ -856,15 +852,12 @@ class EPUBView extends DOMView { this._iframeDocument.documentElement.style.setProperty('--content-scale', String(scale)); this.flow?.setScale(scale); if (cfiBefore) { - this.navigate({ pageNumber: cfiBefore.toString() }, { skipNavStack: true, behavior: 'auto' }); + this.navigate({ pageNumber: cfiBefore.toString() }, { skipHistory: true, behavior: 'auto' }); } } override navigate(location: NavLocation, options: NavigateOptions = {}) { console.log('Navigating to', location); - if (!options.skipNavStack) { - this._pushCurrentLocationToNavStack(); - } options.behavior ||= 'smooth'; if (location.pageNumber) { @@ -914,24 +907,9 @@ class EPUBView extends DOMView { else { super.navigate(location, options); } - } - // This is like back/forward navigation in browsers. Try Cmd-ArrowLeft and Cmd-ArrowRight in PDF view - navigateBack() { - if (this._navStack.canPopBack()) { - this.navigate({ pageNumber: this._navStack.popBack() }, { - skipNavStack: true, - behavior: 'auto', - }); - } - } - - navigateForward() { - if (this._navStack.canPopForward()) { - this.navigate({ pageNumber: this._navStack.popForward() }, { - skipNavStack: true, - behavior: 'auto', - }); + if (!options.skipHistory) { + this._pushHistoryPoint(); } } diff --git a/src/dom/epub/flow.ts b/src/dom/epub/flow.ts index afdc6567..f8ca04cb 100644 --- a/src/dom/epub/flow.ts +++ b/src/dom/epub/flow.ts @@ -71,6 +71,8 @@ abstract class AbstractFlow implements Flow { protected _onViewUpdate: () => void; + protected _onPushHistoryPoint: () => void; + protected constructor(options: Options) { this._view = options.view; this._iframe = options.iframe; @@ -80,6 +82,9 @@ abstract class AbstractFlow implements Flow { this._onUpdateViewState = options.onUpdateViewState; this._onUpdateViewStats = options.onUpdateViewStats; this._onViewUpdate = options.onViewUpdate; + this._onPushHistoryPoint = options.onPushHistoryPoint; + + this._iframeWindow.addEventListener('scroll', this._onPushHistoryPoint); let intersectionObserver = new IntersectionObserver(() => this.invalidate(), { threshold: [0, 1] @@ -184,7 +189,9 @@ abstract class AbstractFlow implements Flow { abstract setSpreadMode(spreadMode: SpreadMode): void; - abstract destroy(): void; + destroy(): void { + this._iframeWindow.removeEventListener('scroll', this._onPushHistoryPoint); + } } interface Options { @@ -193,6 +200,7 @@ interface Options { onUpdateViewState: () => void; onUpdateViewStats: () => void; onViewUpdate: () => void; + onPushHistoryPoint: () => void; } export class ScrolledFlow extends AbstractFlow { @@ -209,7 +217,8 @@ export class ScrolledFlow extends AbstractFlow { } } - destroy(): void { + override destroy(): void { + super.destroy(); this._iframe.classList.remove('flow-mode-scrolled'); this._iframeDocument.body.classList.remove('flow-mode-scrolled'); } @@ -367,7 +376,8 @@ export class PaginatedFlow extends AbstractFlow { this._iframeDocument.body.classList.add('flow-mode-paginated'); } - destroy(): void { + override destroy(): void { + super.destroy(); this._iframeDocument.removeEventListener('keydown', this._handleKeyDown, { capture: true }); this._iframeDocument.removeEventListener('pointerdown', this._handlePointerDown); this._iframeDocument.removeEventListener('pointermove', this._handlePointerMove); diff --git a/src/dom/snapshot/snapshot-view.ts b/src/dom/snapshot/snapshot-view.ts index ab0238e5..f4de99c8 100644 --- a/src/dom/snapshot/snapshot-view.ts +++ b/src/dom/snapshot/snapshot-view.ts @@ -21,7 +21,6 @@ import DOMView, { NavigateOptions } from "../common/dom-view"; import { getUniqueSelectorContaining } from "../common/lib/unique-selector"; -import NavStack from "../common/lib/nav-stack"; import { getVisibleTextNodes } from "../common/lib/nodes"; @@ -35,8 +34,6 @@ import { import injectCSS from './stylesheets/inject.scss'; class SnapshotView extends DOMView { - private readonly _navStack = new NavStack<[number, number]>(); - protected _find: DefaultFindProcessor | null = null; private _searchContext: SearchContext | null = null; @@ -280,6 +277,10 @@ class SnapshotView extends DOMView { } } + protected _getHistoryLocation(): NavLocation | null { + return { scrollCoords: [this._iframeWindow.scrollX, this._iframeWindow.scrollY] }; + } + private _getSearchContext() { if (!this._searchContext) { this._searchContext = createSearchContext(getVisibleTextNodes(this._iframeDocument.body)); @@ -295,11 +296,6 @@ class SnapshotView extends DOMView { // - annotation, selection and overlay popups are closed by calling this._onSetSomePopup() // with no arguments - _pushCurrentLocationToNavStack() { - this._navStack.push([this._iframeWindow.scrollX, this._iframeWindow.scrollY]); - this._updateViewStats(); - } - protected _navigateToSelector(selector: Selector, options: NavigateOptions = {}) { let range = this.toDisplayedRange(selector); if (range) { @@ -335,8 +331,8 @@ class SnapshotView extends DOMView { canZoomIn: this._scale === undefined || this._scale < 1.5, canZoomOut: this._scale === undefined || this._scale > 0.6, canZoomReset: this._scale !== undefined && this._scale !== 1, - canNavigateBack: this._navStack.canPopBack(), - canNavigateForward: this._navStack.canPopForward(), + canNavigateBack: this._history.canNavigateBack, + canNavigateForward: this._history.canNavigateForward, }; this._options.onChangeViewStats(viewStats); } @@ -353,6 +349,7 @@ class SnapshotView extends DOMView { protected override _handleScroll() { super._handleScroll(); this._updateViewState(); + this._pushHistoryPoint(true); } // *** @@ -456,24 +453,17 @@ class SnapshotView extends DOMView { override navigate(location: NavLocation, options: NavigateOptions = {}) { console.log('Navigating to', location); - if (!options.skipNavStack) { - this._pushCurrentLocationToNavStack(); - } options.behavior ||= 'smooth'; - super.navigate(location, options); - } - navigateBack() { - if (this._navStack.canPopBack()) { - this._iframeWindow.scrollTo(...this._navStack.popBack()); - this._handleViewUpdate(); + if (location.scrollCoords) { + this._iframeWindow.scrollTo(...location.scrollCoords); + } + else { + super.navigate(location, options); } - } - navigateForward() { - if (this._navStack.canPopForward()) { - this._iframeWindow.scrollTo(...this._navStack.popForward()); - this._handleViewUpdate(); + if (!options.skipHistory) { + this._pushHistoryPoint(); } } diff --git a/src/pdf/lib/utilities.js b/src/pdf/lib/utilities.js index 1a232a6f..84ed39f8 100644 --- a/src/pdf/lib/utilities.js +++ b/src/pdf/lib/utilities.js @@ -471,30 +471,6 @@ export function getTransformFromRects(rect1, rect2) { return matrix; } -/** - * Wait until scroll is no longer being triggered - * @param container Scrollable container - * @param debounceTime For how long the scroll shouldn't be triggered - * @returns {Promise} - */ -export function debounceUntilScrollFinishes(container, debounceTime = 100) { - return new Promise((resolve) => { - let debounceTimeout; - let resolveAndCleanup = () => { - container.removeEventListener('scroll', scrollListener); - clearTimeout(debounceTimeout); - resolve(); - }; - let scrollListener = () => { - clearTimeout(debounceTimeout); - debounceTimeout = setTimeout(resolveAndCleanup, debounceTime); - }; - container.addEventListener('scroll', scrollListener); - // Start the debounce timeout immediately - debounceTimeout = setTimeout(resolveAndCleanup, debounceTime); - }); -} - export function normalizeDegrees(degrees) { return ((degrees % 360) + 360) % 360; } diff --git a/src/pdf/pdf-view.js b/src/pdf/pdf-view.js index 8ba573ce..a58af906 100644 --- a/src/pdf/pdf-view.js +++ b/src/pdf/pdf-view.js @@ -28,10 +28,10 @@ import { getAxialAlignedBoundingBox, distanceBetweenRects, getTransformFromRects, - debounceUntilScrollFinishes, getRotationDegrees, normalizeDegrees } from './lib/utilities'; +import { debounceUntilScrollFinishes } from '../common/lib/utilities'; import { getAffectedAnnotations, isFirefox, @@ -53,7 +53,7 @@ import { eraseInk, smoothPath } from './lib/path'; -import { History } from './lib/history'; +import { History } from '../common/lib/history'; class PDFView { constructor(options) {