EPUB/snapshot: Replace NavStack with History

Fixes zotero/zotero#3745

src/pdf/lib/history.js → 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,17 +74,21 @@ 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();
 		}
 	}
 
 	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();
 		}

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<unknown>}
+ */
+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) {

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;

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<State extends DOMViewState, Data> {
 	initializedPromise: Promise<void>;
@@ -82,6 +84,10 @@ abstract class DOMView<State extends DOMViewState, Data> {
 
 	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<State extends DOMViewState, Data> {
 		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<State extends DOMViewState, Data> {
 	// Abstractions over document structure
 	// ***
 
+	protected abstract _getHistoryLocation(): NavLocation | null;
+
 	protected abstract _getAnnotationFromRange(range: Range, type: AnnotationType, color?: string): NewAnnotation<WADMAnnotation> | null;
 
 	protected abstract _updateViewState(): void;
@@ -202,6 +214,18 @@ abstract class DOMView<State extends DOMViewState, Data> {
 	// 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<State extends DOMViewState, Data> {
 			}, 2000);
 		}
 	}
+
+	navigateBack() {
+		this._history.navigateBack();
+	}
+
+	navigateForward() {
+		this._history.navigateForward();
+	}
 }
 
 export type DOMViewOptions<State extends DOMViewState, Data> = {
@@ -1196,7 +1228,7 @@ export interface CustomScrollIntoViewOptions extends Omit<ScrollIntoViewOptions,
 }
 
 export interface NavigateOptions extends CustomScrollIntoViewOptions {
-	skipNavStack?: boolean;
+	skipHistory?: boolean;
 }
 
 export default DOMView;

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<EPUBViewState, EPUBViewData> {
 	protected _find: EPUBFindProcessor | null = null;
 
@@ -75,8 +74,6 @@ class EPUBView extends DOMView<EPUBViewState, EPUBViewData> {
 
 	private readonly _sectionViews: SectionView[] = [];
 
-	private readonly _navStack = new NavStack<string>();
-
 	private readonly _rangeCache = new Map<string, PersistentRange>();
 
 	private _pageProgressionRTL!: boolean;
@@ -337,16 +334,14 @@ class EPUBView extends DOMView<EPUBViewState, EPUBViewData> {
 		}
 	}
 
-	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<EPUBViewState, EPUBViewData> {
 			this.navigate(
 				{ pageNumber: beforeCFI.toString() },
 				{
-					skipNavStack: true,
+					skipHistory: true,
 					behavior: 'auto',
 					offsetY: beforeOffset ?? undefined
 				}
@@ -547,8 +542,8 @@ class EPUBView extends DOMView<EPUBViewState, EPUBViewData> {
 			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<EPUBViewState, EPUBViewData> {
 			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<EPUBViewState, EPUBViewData> {
 		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<EPUBViewState, EPUBViewData> {
 		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<EPUBViewState, EPUBViewData> {
 		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();
 		}
 	}