Desktop: Fixes #5708: Scroll positions are preserved (#5826)

Features:
- Scroll position is preserved when the editor layout changes.
- Scroll position is remembered when a note selection changes.

Modifications:
- The current Sync Scroll feature (in v2.6.2) is modified to use line-percent-based scroll positions.
- Scroll position translation functions, Viewer-to-Editor and Editor-to-Viewer, are separated into V2L / L2E and E2L / L2V respectively.
- The scrollmap is moved from gui/utils/SyncScrollMap.ts to note-viewer/scrollmap.js.
- IPC Protocol about the scrollmap becomes not necessary and is removed.
- Ignores non-user scroll events to avoid sync with incorrect scroll positions.
- When CodeMirror is not ready, setEditorPercentScroll() is waited.
- Fixes the bug: An incorrect scroll position is sometimes recorded.
- Since scroll positions become line-percent-based, the following incompatibilities of scroll positions are fixed:
  - Between Editor and Viewer.
  - Between Viewer Layout and Split Layout of Viewer
  - Between Editor Layout and Split Layout of Editor

.eslintignore

@@ -705,9 +705,6 @@ packages/app-desktop/gui/style/StyledTextInput.js.map
 packages/app-desktop/gui/utils/NoteListUtils.d.ts
 packages/app-desktop/gui/utils/NoteListUtils.js
 packages/app-desktop/gui/utils/NoteListUtils.js.map
-packages/app-desktop/gui/utils/SyncScrollMap.d.ts
-packages/app-desktop/gui/utils/SyncScrollMap.js
-packages/app-desktop/gui/utils/SyncScrollMap.js.map
 packages/app-desktop/gui/utils/convertToScreenCoordinates.d.ts
 packages/app-desktop/gui/utils/convertToScreenCoordinates.js
 packages/app-desktop/gui/utils/convertToScreenCoordinates.js.map

.gitignore

@@ -688,9 +688,6 @@ packages/app-desktop/gui/style/StyledTextInput.js.map
 packages/app-desktop/gui/utils/NoteListUtils.d.ts
 packages/app-desktop/gui/utils/NoteListUtils.js
 packages/app-desktop/gui/utils/NoteListUtils.js.map
-packages/app-desktop/gui/utils/SyncScrollMap.d.ts
-packages/app-desktop/gui/utils/SyncScrollMap.js
-packages/app-desktop/gui/utils/SyncScrollMap.js.map
 packages/app-desktop/gui/utils/convertToScreenCoordinates.d.ts
 packages/app-desktop/gui/utils/convertToScreenCoordinates.js
 packages/app-desktop/gui/utils/convertToScreenCoordinates.js.map

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/CodeMirror.tsx

@@ -7,7 +7,7 @@ import { commandAttachFileToBody, handlePasteEvent } from '../../utils/resourceH
 import { ScrollOptions, ScrollOptionTypes } from '../../utils/types';
 import { CommandValue } from '../../utils/types';
 import { usePrevious, cursorPositionToTextOffset } from './utils';
-import useScrollHandler, { translateScrollPercentToEditor, translateScrollPercentToViewer } from './utils/useScrollHandler';
+import useScrollHandler from './utils/useScrollHandler';
 import useElementSize from '@joplin/lib/hooks/useElementSize';
 import Toolbar from './Toolbar';
 import styles_ from './styles';
@@ -65,7 +65,8 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) {
 
 	usePluginServiceRegistration(ref);
 
-	const { resetScroll, editor_scroll, setEditorPercentScroll, setViewerPercentScroll } = useScrollHandler(editorRef, webviewRef, props.onScroll);
+	const { resetScroll, editor_scroll, setEditorPercentScroll, setViewerPercentScroll, editor_resize,
+	} = useScrollHandler(editorRef, webviewRef, props.onScroll);
 
 	const codeMirror_change = useCallback((newBody: string) => {
 		props_onChangeRef.current({ changeId: null, content: newBody });
@@ -115,10 +116,9 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) {
 					if (!webviewRef.current) return;
 					webviewRef.current.wrappedInstance.send('scrollToHash', options.value as string);
 				} else if (options.type === ScrollOptionTypes.Percent) {
-					const editorPercent = options.value as number;
+					const percent = options.value as number;
-					setEditorPercentScroll(editorPercent);
+					setEditorPercentScroll(percent);
-					const viewerPercent = translateScrollPercentToViewer(editorRef, webviewRef, editorPercent);
+					setViewerPercentScroll(percent);
-					setViewerPercentScroll(viewerPercent);
 				} else {
 					throw new Error(`Unsupported scroll options: ${options.type}`);
 				}
@@ -581,17 +581,8 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) {
 				editorRef.current.updateBody(newBody);
 			}
 		} else if (msg === 'percentScroll') {
-			const viewerPercent = arg0;
+			const percent = arg0;
-			const editorPercent = translateScrollPercentToEditor(editorRef, webviewRef, viewerPercent);
+			setEditorPercentScroll(percent);
-			setEditorPercentScroll(editorPercent);
-		} else if (msg === 'syncViewerScrollWithEditor') {
-			const force = !!arg0;
-			webviewRef.current?.wrappedInstance?.refreshSyncScrollMap(force);
-			const editorPercent = Math.max(0, Math.min(1, editorRef.current?.getScrollPercent()));
-			if (!isNaN(editorPercent)) {
-				const viewerPercent = translateScrollPercentToViewer(editorRef, webviewRef, editorPercent);
-				setViewerPercentScroll(viewerPercent);
-			}
 		} else {
 			props.onMessage(event);
 		}
@@ -644,6 +635,7 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) {
 		const options: any = {
 			pluginAssets: renderedBody.pluginAssets,
 			downloadResources: Setting.value('sync.resourceDownloadMode'),
+			markupLineCount: editorRef.current?.lineCount() || 0,
 		};
 
 		// It seems when there's an error immediately when the component is
@@ -652,7 +644,6 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) {
 		// Since we can't do much about it we just print an error.
 		if (webviewRef.current && webviewRef.current.wrappedInstance) {
 			webviewRef.current.wrappedInstance.send('setHtml', renderedBody.html, options);
-			webviewRef.current.wrappedInstance.refreshSyncScrollMap(true);
 		} else {
 			console.error('Trying to set HTML on an undefined webview ref');
 		}
@@ -829,6 +820,7 @@ function CodeMirror(props: NoteBodyEditorProps, ref: any) {
 					onScroll={editor_scroll}
 					onEditorPaste={onEditorPaste}
 					isSafeMode={props.isSafeMode}
+					onResize={editor_resize}
 				/>
 			</div>
 		);

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/Editor.tsx

@@ -93,6 +93,7 @@ export interface EditorProps {
 	onScroll: any;
 	onEditorPaste: any;
 	isSafeMode: boolean;
+	onResize: any;
 }
 
 function Editor(props: EditorProps, ref: any) {
@@ -189,6 +190,8 @@ function Editor(props: EditorProps, ref: any) {
 		cm.on('paste', editor_paste);
 		cm.on('drop', editor_drop);
 		cm.on('dragover', editor_drag);
+		cm.on('refresh', props.onResize);
+		cm.on('update', props.onResize);
 
 		// It's possible for searchMarkers to be available before the editor
 		// In these cases we set the markers asap so the user can see them as
@@ -202,6 +205,8 @@ function Editor(props: EditorProps, ref: any) {
 			cm.off('paste', editor_paste);
 			cm.off('drop', editor_drop);
 			cm.off('dragover', editor_drag);
+			cm.off('refresh', props.onResize);
+			cm.off('update', props.onResize);
 			editorParent.current.removeChild(cm.getWrapperElement());
 			setEditor(null);
 		};

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/utils/useScrollHandler.ts

@@ -1,10 +1,34 @@
 import { useCallback, useRef } from 'react';
 import shim from '@joplin/lib/shim';
-import { SyncScrollMap } from '../../../../utils/SyncScrollMap';
 
 export default function useScrollHandler(editorRef: any, webviewRef: any, onScroll: Function) {
-	const ignoreNextEditorScrollEvent_ = useRef(false);
 	const scrollTimeoutId_ = useRef<any>(null);
+	const scrollPercent_ = useRef(0);
+	const ignoreNextEditorScrollTime_ = useRef(Date.now());
+	const ignoreNextEditorScrollEventCount_ = useRef(0);
+	const delayedSetEditorPercentScrollTimeoutID_ = useRef(null);
+
+	// Ignores one next scroll event for a short time.
+	const ignoreNextEditorScrollEvent = () => {
+		const now = Date.now();
+		if (now >= ignoreNextEditorScrollTime_.current) ignoreNextEditorScrollEventCount_.current = 0;
+		if (ignoreNextEditorScrollEventCount_.current < 10) { // for safety
+			ignoreNextEditorScrollTime_.current = now + 200;
+			ignoreNextEditorScrollEventCount_.current += 1;
+		}
+	};
+
+	// Tests the next scroll event should be ignored and then decrements the count.
+	const isNextEditorScrollEventIgnored = () => {
+		if (ignoreNextEditorScrollEventCount_.current) {
+			if (Date.now() < ignoreNextEditorScrollTime_.current) {
+				ignoreNextEditorScrollEventCount_.current -= 1;
+				return true;
+			}
+			ignoreNextEditorScrollEventCount_.current = 0;
+		}
+		return false;
+	};
 
 	const scheduleOnScroll = useCallback((event: any) => {
 		if (scrollTimeoutId_.current) {
@@ -18,108 +42,128 @@ export default function useScrollHandler(editorRef: any, webviewRef: any, onScro
 		}, 10);
 	}, [onScroll]);
 
-	const setEditorPercentScroll = useCallback((p: number) => {
+	const setEditorPercentScrollInternal = (percent: number) => {
-		ignoreNextEditorScrollEvent_.current = true;
+		scrollPercent_.current = percent;
+		let retry = 0;
+		const fn = () => {
+			if (delayedSetEditorPercentScrollTimeoutID_.current) {
+				shim.clearInterval(delayedSetEditorPercentScrollTimeoutID_.current);
+				delayedSetEditorPercentScrollTimeoutID_.current = null;
+			}
+			const cm = editorRef.current;
+			if (isCodeMirrorReady(cm)) {
+				// calculates editor's GUI-dependent pixel-based raw percent
+				const newEditorPercent = translateScrollPercentL2E(cm, scrollPercent_.current);
+				const oldEditorPercent = cm.getScrollPercent();
+				if (!(Math.abs(newEditorPercent - oldEditorPercent) < 1e-8)) {
+					ignoreNextEditorScrollEvent();
+					cm.setScrollPercent(newEditorPercent);
+				}
+			} else {
+				retry += 1;
+				if (retry <= 10) {
+					delayedSetEditorPercentScrollTimeoutID_.current = shim.setTimeout(fn, 50);
+				}
+			}
+		};
+		fn();
+	};
 
+	const restoreEditorPercentScroll = () => {
+		if (isCodeMirrorReady(editorRef.current)) {
+			setEditorPercentScrollInternal(scrollPercent_.current);
+		}
+	};
+
+	const setEditorPercentScroll = useCallback((percent: number) => {
+		setEditorPercentScrollInternal(percent);
 		if (editorRef.current) {
-			editorRef.current.setScrollPercent(p);
+			scheduleOnScroll({ percent });
-
-			scheduleOnScroll({ percent: p });
 		}
 	}, [scheduleOnScroll]);
 
-	const setViewerPercentScroll = useCallback((p: number) => {
+	const setViewerPercentScroll = useCallback((percent: number) => {
 		if (webviewRef.current) {
-			webviewRef.current.wrappedInstance.send('setPercentScroll', p);
+			webviewRef.current.wrappedInstance.send('setPercentScroll', percent);
-			scheduleOnScroll({ percent: p });
+			scheduleOnScroll({ percent });
 		}
 	}, [scheduleOnScroll]);
 
 	const editor_scroll = useCallback(() => {
-		if (ignoreNextEditorScrollEvent_.current) {
+		if (isNextEditorScrollEventIgnored()) return;
-			ignoreNextEditorScrollEvent_.current = false;
-			return;
-		}
 
-		if (editorRef.current) {
+		const cm = editorRef.current;
-			const editorPercent = Math.max(0, Math.min(1, editorRef.current.getScrollPercent()));
+		if (isCodeMirrorReady(cm)) {
+			const editorPercent = Math.max(0, Math.min(1, cm.getScrollPercent()));
 			if (!isNaN(editorPercent)) {
 				// when switching to another note, the percent can sometimes be NaN
 				// this is coming from `gui/NoteEditor/NoteBody/CodeMirror/utils/useScrollUtils.ts`
 				// when CodeMirror returns scroll info with heigth == clientHeigth
 				// https://github.com/laurent22/joplin/issues/4797
-				const viewerPercent = translateScrollPercentToViewer(editorRef, webviewRef, editorPercent);
+
-				setViewerPercentScroll(viewerPercent);
+				// calculates GUI-independent line-based percent
+				const percent = translateScrollPercentE2L(cm, editorPercent);
+				scrollPercent_.current = percent;
+				setViewerPercentScroll(percent);
 			}
 		}
 	}, [setViewerPercentScroll]);
 
 	const resetScroll = useCallback(() => {
+		scrollPercent_.current = 0;
 		if (editorRef.current) {
 			editorRef.current.setScrollPercent(0);
 		}
 	}, []);
 
-	return { resetScroll, setEditorPercentScroll, setViewerPercentScroll, editor_scroll };
+	const editor_resize = useCallback((cm) => {
+		if (cm) {
+			restoreEditorPercentScroll();
+		}
+	}, []);
+
+	return {
+		resetScroll, setEditorPercentScroll, setViewerPercentScroll, editor_scroll, editor_resize,
+	};
 }
 
-const translateScrollPercent_ = (editorRef: any, webviewRef: any, percent: number, editorToViewer: boolean) => {
+const translateLE_ = (codeMirror: any, percent: number, l2e: boolean) => {
 	// If the input is out of (0,1) or not number, it is not translated.
 	if (!(0 < percent && percent < 1)) return percent;
-	const map: SyncScrollMap = webviewRef.current?.wrappedInstance.getSyncScrollMap();
+	if (!codeMirror) return percent; // No translation
-	const cm = editorRef.current;
+	const info = codeMirror.getScrollInfo();
-	if (!map || map.line.length <= 2 || !cm) return percent; // No translation
+	const height = info.height - info.clientHeight;
-	const lineCount = cm.lineCount();
+	if (height <= 1) return percent; // No translation for non-displayed CodeMirror.
-	if (map.line[map.line.length - 2] >= lineCount) {
+	const lineCount = codeMirror.lineCount();
-		// Discarded a obsolete map and use no translation.
+	let lineU = l2e ? Math.floor(percent * lineCount) : codeMirror.lineAtHeight(percent * height, 'local');
-		webviewRef.current.wrappedInstance.refreshSyncScrollMap(false);
+	lineU = Math.max(0, Math.min(lineCount - 1, lineU));
-		return percent;
+	const ePercentU = codeMirror.heightAtLine(lineU, 'local') / height;
-	}
+	const ePercentL = codeMirror.heightAtLine(lineU + 1, 'local') / height;
-	const info = cm.getScrollInfo();
+	let linInterp, result;
-	const height = Math.max(1, info.height - info.clientHeight);
+	if (l2e) {
-	let values = map.percent, target = percent;
+		linInterp = percent * lineCount - lineU;
-	if (editorToViewer) {
-		const top = percent * height;
-		const line = cm.lineAtHeight(top, 'local');
-		values = map.line;
-		target = line;
-	}
-	// Binary search (rightmost): finds where map[r-1][field] <= target < map[r][field]
-	let l = 1, r = values.length - 1;
-	while (l < r) {
-		const m = Math.floor(l + (r - l) / 2);
-		if (target < values[m]) r = m; else l = m + 1;
-	}
-	const lineU = map.line[r - 1];
-	const lineL = Math.min(lineCount, map.line[r]);
-	const ePercentU = r == 1 ? 0 : Math.min(1, cm.heightAtLine(lineU, 'local') / height);
-	const ePercentL = Math.min(1, cm.heightAtLine(lineL, 'local') / height);
-	const vPercentU = map.percent[r - 1];
-	const vPercentL = ePercentL == 1 ? 1 : map.percent[r];
-	let result;
-	if (editorToViewer) {
-		const linInterp = (percent - ePercentU) / (ePercentL - ePercentU);
-		result = vPercentU + (vPercentL - vPercentU) * linInterp;
-	} else {
-		const linInterp = (percent - vPercentU) / (vPercentL - vPercentU);
 		result = ePercentU + (ePercentL - ePercentU) * linInterp;
+	} else {
+		linInterp = Math.max(0, Math.min(1, (percent - ePercentU) / (ePercentL - ePercentU))) || 0;
+		result = (lineU + linInterp) / lineCount;
 	}
 	return Math.max(0, Math.min(1, result));
 };
 
-// translateScrollPercentToEditor() and translateScrollPercentToViewer() are
+// translateScrollPercentL2E() and translateScrollPercentE2L() are
-// the translation functions between Editor's scroll percent and Viewer's scroll
+// the translation functions between Editor's scroll percent and line-based scroll
 // percent. They are used for synchronous scrolling between Editor and Viewer.
-// They use a SyncScrollMap provided by Viewer for its translation.
 // To see the detail of synchronous scrolling, refer the following design document.
-// https://github.com/laurent22/joplin/pull/5512#issuecomment-931277022
+// <s> Replace me! https://github.com/laurent22/joplin/pull/5512#issuecomment-931277022</s>
-
+const translateScrollPercentL2E = (cm: any, lPercent: number) => {
-export const translateScrollPercentToEditor = (editorRef: any, webviewRef: any, viewerPercent: number) => {
+	return translateLE_(cm, lPercent, true);
-	const editorPercent = translateScrollPercent_(editorRef, webviewRef, viewerPercent, false);
-	return editorPercent;
 };
 
-export const translateScrollPercentToViewer = (editorRef: any, webviewRef: any, editorPercent: number) => {
+const translateScrollPercentE2L = (cm: any, ePercent: number) => {
-	const viewerPercent = translateScrollPercent_(editorRef, webviewRef, editorPercent, true);
+	return translateLE_(cm, ePercent, false);
-	return viewerPercent;
 };
+
+function isCodeMirrorReady(cm: any) {
+	const info = cm?.getScrollInfo();
+	return info && info.height - info.clientHeight > 0;
+}

packages/app-desktop/gui/NoteEditor/NoteEditor.tsx

@@ -344,7 +344,10 @@ function NoteEditor(props: NoteEditorProps) {
 	const onScroll = useCallback((event: any) => {
 		props.dispatch({
 			type: 'EDITOR_SCROLL_PERCENT_SET',
-			noteId: formNote.id,
+			// In callbacks of setTimeout()/setInterval(), props/state cannot be used
+			// to refer the current value, since they would be one or more generations old.
+			// For the purpose, useRef value should be used.
+			noteId: formNoteRef.current.id,
 			percent: event.percent,
 		});
 	}, [props.dispatch, formNote]);

packages/app-desktop/gui/NoteTextViewer.tsx

@@ -2,7 +2,6 @@ import PostMessageService, { MessageResponse, ResponderComponentType } from '@jo
 import * as React from 'react';
 const { connect } = require('react-redux');
 import { reg } from '@joplin/lib/registry';
-import { SyncScrollMap, SyncScrollMapper } from './utils/SyncScrollMap';
 
 interface Props {
 	onDomReady: Function;
@@ -168,17 +167,6 @@ class NoteTextViewerComponent extends React.Component<Props, any> {
 		}
 	}
 
-	private syncScrollMapper_ = new SyncScrollMapper;
-
-	refreshSyncScrollMap(forced: boolean) {
-		return this.syncScrollMapper_.refresh(forced);
-	}
-
-	getSyncScrollMap(): SyncScrollMap {
-		const doc = this.webviewRef_.current?.contentWindow?.document;
-		return this.syncScrollMapper_.get(doc);
-	}
-
 	// ----------------------------------------------------------------
 	// Wrap WebView functions (END)
 	// ----------------------------------------------------------------

packages/app-desktop/gui/note-viewer/index.html

@@ -42,6 +42,7 @@
 	<div id="joplin-container-markScriptContainer"></div>
 	<div id="joplin-container-content" ondragstart="return false;" ondrop="return false;"></div>
 	<script src="./lib.js"></script>
+	<script src="./scrollmap.js"></script>
 
 	<script>
 	// This is function used internally to send message from the webview to
@@ -104,23 +105,46 @@
 		// it at any time knowing that it's not going to be changed because the content height has changed.
 		let percentScroll_ = 0;
 
-		// This variable provides a	way to skip scroll events for a certain duration.
+		let ignoreNextScrollTime_ = Date.now();
-		// In general, it should be set whenever the scroll value is set explicitely (programmatically)
+		let ignoreNextScrollEventCount_ = 0;
+
+		// ignoreNextScrollEvent() provides a way to skip scroll events for a certain duration.
+		// In general, it should be called whenever the scroll value is set explicitely (programmatically)
 		// so as to differentiate scroll events generated by the user (when scrolling the view) and those
 		// generated by the application.
-		let lastScrollEventTime = 0;
+		function ignoreNextScrollEvent() {
+			const now = Date.now();
+			if (now >= ignoreNextScrollTime_) ignoreNextScrollEventCount_ = 0;
+			if (ignoreNextScrollEventCount_ < 10) { // for safety
+				ignoreNextScrollTime_ = now + 200;
+				ignoreNextScrollEventCount_ += 1;
+			}
+		};
 
-		function setPercentScroll(percent) {
+		// Tests the next scroll event should be ignored and then decrements the count.
-			percentScroll_ = percent;
+		function isNextScrollEventIgnored() {
-			contentElement.scrollTop = percentScroll_ * maxScrollTop();
+			if (ignoreNextScrollEventCount_) {
+				if (Date.now() < ignoreNextScrollTime_) {
+					ignoreNextScrollEventCount_ -= 1;
+					return true;
+				}
+				ignoreNextScrollEventCount_ = 0;
+			}
+			return false;
 		}
 
-		function percentScroll() {
+		function setPercentScroll(percent) {
-			return percentScroll_;
+			// calculates viewer's GUI-dependent pixel-based raw percent
+			const viewerPercent = scrollmap.translateL2V(percent);
+			const newScrollTop = viewerPercent * maxScrollTop();
+			if (Math.floor(contentElement.scrollTop) !== Math.floor(newScrollTop)) {
+				ignoreNextScrollEvent();
+				percentScroll_ = percent;
+				contentElement.scrollTop = newScrollTop;
+			}
 		}
 
 		function restorePercentScroll() {
-			lastScrollEventTime = Date.now();
 			setPercentScroll(percentScroll_);
 		}
 
@@ -153,25 +177,38 @@
 		}
 
 		ipc.scrollToHash = (event) => {
-			if (window.scrollToHashIID_) clearInterval(window.scrollToHashIID_);
+			let retry = 0;
-			window.scrollToHashIID_ = setInterval(() => {
+			const fn = () => {
-				if (document.readyState !== 'complete') return;
+				if (window.scrollToHashTimeoutID_) {
-				clearInterval(window.scrollToHashIID_);
+					clearInterval(window.scrollToHashTimeoutID_);
-				const hash = event.hash.toLowerCase();
+					window.scrollToHashTimeoutID_ = null;
-				const e = document.getElementById(hash);
-				if (!e) {
-					console.warn('Cannot find hash', hash);
-					return;
 				}
-				e.scrollIntoView();
+				if (document.readyState === 'complete' ||
+					// If scrollmap is present, Element.scrollIntoView() is also
+					// available when document.readyState is interactive.
+					document.readyState === 'interactive' && scrollmap.isPresent()) {
+					const hash = event.hash.toLowerCase();
+					const e = document.getElementById(hash);
+					if (e) {
+						e.scrollIntoView();
+						// It causes a scroll event, whose listener sent a new scroll
+						// position to Editor.
+					} else {
+						console.warn('Cannot find hash', hash);
+					}
+				} else {
+					retry += 1;
+					if (retry <= 10) {
+						window.scrollToHashTimeoutID_ = setTimeout(fn, 100);
+					}
+				}
+			};
+			fn();
+		}
 
-				// Make sure the editor pane is also scrolled
+		function isVisible() {
-				setTimeout(() => {
+			// See the logic of hiding viewer in CoderMirror.tsx
-					const percent = currentPercentScroll();
+			return window.innerWidth > 1;
-					setPercentScroll(percent);
-					ipcProxySendToHost('percentScroll', percent);
-				}, 10);
-			}, 100);
 		}
 
 		// https://stackoverflow.com/a/1977898/561309
@@ -182,6 +219,7 @@
 		}
 
 		function allImagesLoaded() {
+			if (!isVisible()) return true; // In the case, images would not be loaded.
 			for (const image of document.images) {
 				if (!isImageReady(image)) return false;
 			}
@@ -209,13 +247,16 @@
 		// to observe the changes of the scroll height of the content element
 		// using a callback approach.
 		function observeRendering(callback, compress = false) {
-			let previousHeight = 0;
+			let lastScrollHeight = 0;
+			let lastClientHeight = 0;
 			const fn = (cause) => {
-				const height = contentElement.scrollHeight;
+				const sh = contentElement.scrollHeight;
-				const heightChanged = height != previousHeight;
+				const ch = contentElement.clientHeight;
+				const heightChanged = (sh !== lastScrollHeight || ch !== lastClientHeight);
 				if (!compress || heightChanged) {
-					previousHeight = height;
+					lastScrollHeight = sh;
-					callback(cause, height, heightChanged);
+					lastClientHeight = ch;
+					callback(cause, sh, heightChanged);
 				}
 			};
 			// 'resized' means DOM Layout change or Window resize event
@@ -232,21 +273,39 @@
 			return { mutationObserver, resizeObserver };
 		};
 
+		// To suppress too frequent restoring of scroll positions and refreshing of the scroll map
+		let restoreAndRefreshTimeoutID_ = null;
+		let restoreAndRefreshTimeout_ = Date.now();
+
 		// A callback anonymous function invoked when the scroll height changes.
 		const onRendering = observeRendering((cause, height, heightChanged) => {
-			if (!alreadyAllImagesLoaded) {
+			if (!alreadyAllImagesLoaded && !scrollmap.isPresent()) {
 				const loaded = allImagesLoaded();
 				if (loaded) {
 					alreadyAllImagesLoaded = true;
-					ipcProxySendToHost('syncViewerScrollWithEditor', true);
+					scrollmap.refresh();
+					restorePercentScroll();
 					ipcProxySendToHost('noteRenderComplete');
 					return;
 				}
 			}
-			if (heightChanged) {
+			if (!heightChanged) return;
-				// When the scroll height changes, sync is needed.
+			const restoreAndRefresh = () => {
-				ipcProxySendToHost('syncViewerScrollWithEditor');
+				scrollmap.refresh();
+				restorePercentScroll();
+			};
+			const now = Date.now();
+			if (now < restoreAndRefreshTimeout_) {
+				if (restoreAndRefreshTimeoutID_) {
+					clearTimeout(restoreAndRefreshTimeoutID_);
+					restoreAndRefreshTimeoutID_ = null;
+				}
+				const msec = Math.min(1000, restoreAndRefreshTimeout_ - now);
+				restoreAndRefreshTimeoutID_ = setTimeout(restoreAndRefresh, msec);
+			} else {
+				restoreAndRefresh();
 			}
+			restoreAndRefreshTimeout_ = now + 200;
 		});
 
 		ipc.focus = (event) => {
@@ -267,12 +326,12 @@
 
 			updateBodyHeight();
 
+			alreadyAllImagesLoaded = false;
+
 			contentElement.innerHTML = html;
 
+			scrollmap.create(event.options.markupLineCount);
 			restorePercentScroll(); // First, a quick treatment is applied.
-			ipcProxySendToHost('syncViewerScrollWithEditor');
-
-			alreadyAllImagesLoaded = false;
 
 			addPluginAssets(event.options.pluginAssets);
 
@@ -281,12 +340,15 @@
 			}
 
 			document.dispatchEvent(new Event('joplin-noteDidUpdate'));
+
+			if (scrollmap.isPresent()) {
+				// Now, ready to receive scrollToHash/setPercentScroll from Editor.
+				ipcProxySendToHost('noteRenderComplete');
+			}
 		}
 
 		ipc.setPercentScroll = (event) => {
-			const percent = event.percent;
+			setPercentScroll(event.percent);
-			lastScrollEventTime = Date.now();
-			setPercentScroll(percent);
 		}
 
 		// HACK for Mark.js bug - https://github.com/julmot/mark.js/issues/127
@@ -391,11 +453,14 @@
 			document.getElementById('joplin-container-content').style.height = window.innerHeight + 'px';
 		}
 
-		function currentPercentScroll() {
+		function getPercentFromViewer() {
 			const m = maxScrollTop();
 			// As of 2021, if zoomFactor != 1, underlying Chrome returns scrollTop with 
 			// some numerical error. It can be more than maxScrollTop().
-			return m ? Math.min(1, contentElement.scrollTop / m) : 0;
+			const viewerPecent = m ? Math.min(1, contentElement.scrollTop / m) : 0;
+			// calculates GUI-independent line-based logical percent
+			const percent = scrollmap.translateV2L(viewerPecent);
+			return percent;
 		}
 
 		// If zoom factor is not 1, Electron/Chromium calculates scrollTop incorrectly.
@@ -477,22 +542,15 @@
 		}));
 
 		contentElement.addEventListener('scroll', webviewLib.logEnabledEventHandler(e => {
-			// If the last scroll event was done by the user, lastScrollEventTime is set and
+			lastScrollTop_ = contentElement.scrollTop;
+			// If the last scroll event was done by the application, ignoreNextScrollEvent() is called and
 			// we can use that to skip the event handling. We skip it because in that case
 			// the scroll position has already been updated. Also we add a 200ms interval
 			// because otherwise it's most likely a glitch where we called ipc.setPercentScroll
 			// but the scroll event listener has not been called.
-			if (lastScrollEventTime && Date.now() - lastScrollEventTime < 200) {
+			if (isNextScrollEventIgnored()) return;
-				lastScrollEventTime = 0;
+			percentScroll_ = getPercentFromViewer();
-				return;
+			ipcProxySendToHost('percentScroll', percentScroll_);
-			}
-
-			lastScrollEventTime = 0;
-
-			const percent = currentPercentScroll();
-			setPercentScroll(percent);
-			
-			ipcProxySendToHost('percentScroll', percent);
 		}));
 
 		ipc['postMessageService.response'] = function(event) {
@@ -568,11 +626,25 @@
 			e.preventDefault();
 		}));
 
+		let lastClientWidth_ = NaN, lastClientHeight_ = NaN, lastScrollTop_ = NaN;
+
 		window.addEventListener('resize', webviewLib.logEnabledEventHandler(() => {
 			updateBodyHeight();
 			// When zoomFactor is changed, resize event happens.
 			zoomFactorIsNotOne = false;
 			resetSmoothScroll();
+
+			// If this event resizes contentElement, ignore the scroll event caused by it.
+			const cw = contentElement.clientWidth;
+			const ch = contentElement.clientHeight;
+			const top = contentElement.scrollTop;
+			if (!(cw === lastClientWidth_ && ch === lastClientHeight_)) {
+				// Since scroll listeners are invoked before ResizeObserver and
+				// resize listeners are invoked before scroll listeners,
+				// this code should be here to ignore scroll events.
+				if (top !== lastScrollTop_) ignoreNextScrollEvent();
+				lastClientWidth_ = cw; lastClientHeight_ = ch; lastScrollTop_ = top;
+			}
 		}));
 
 		// Prevent middle-click as that would open the URL in an Electron window

packages/app-desktop/gui/note-viewer/scrollmap.js

@@ -0,0 +1,113 @@
+// scrollmap is used for synchronous scrolling between Markdown Editor and Viewer.
+// It has the mapping information between the line numbers of a Markdown text and
+// the scroll positions (percents) of the elements in the HTML document transformed
+// from the Markdown text.
+// To see the detail of synchronous scrolling, refer the following design document.
+// <s> Replace me! https://github.com/laurent22/joplin/pull/5512#issuecomment-931277022 </s>
+
+const scrollmap = {
+	map_: null,
+	lineCount_: 0,
+};
+
+scrollmap.create = (lineCount) => {
+	// Creates a translation map between editor's line number
+	// and viewer's scroll percent. Both attributes (line and percent) of
+	// the returned map are sorted respectively.
+	// For each document change, this function should be called.
+	// Since creating this map is costly for each scroll event,
+	// it is cached and re-created as needed. Whenever the layout
+	// of the document changes, it has to be invalidated by refresh().
+	scrollmap.lineCount_ = lineCount;
+	scrollmap.refresh();
+};
+
+scrollmap.refresh = () => {
+	scrollmap.map_ = null;
+};
+
+scrollmap.get_ = () => {
+	if (scrollmap.map_) return scrollmap.map_;
+	const contentElement = document.getElementById('joplin-container-content');
+	if (!contentElement) return null;
+	const height = Math.max(1, contentElement.scrollHeight - contentElement.clientHeight);
+	// Since getBoundingClientRect() returns a relative position,
+	// the offset of the origin is needed to get its aboslute position.
+	const firstElem = document.getElementById('rendered-md');
+	if (!firstElem) return null;
+	const offset = firstElem.getBoundingClientRect().top;
+	// Mapping information between editor's lines and viewer's elements is
+	// embedded into elements by the renderer.
+	// See also renderer/MdToHtml/rules/source_map.ts.
+	const elems = document.getElementsByClassName('maps-to-line');
+	if (elems.length == 0) return null;
+	const map = { line: [0], percent: [0], viewHeight: height, lineCount: 0 };
+	// Each map entry is total-ordered.
+	let last = 0;
+	for (let i = 0; i < elems.length; i++) {
+		const top = elems[i].getBoundingClientRect().top - offset;
+		const line = Number(elems[i].getAttribute('source-line'));
+		const percent = Math.max(0, Math.min(1, top / height));
+		if (map.line[last] < line && map.percent[last] < percent) {
+			map.line.push(line);
+			map.percent.push(percent);
+			last += 1;
+		}
+	}
+	const lineCount = scrollmap.lineCount_;
+	if (lineCount) {
+		map.lineCount = lineCount;
+	} else {
+		if (map.lineCount <= map.line[last]) map.lineCount = map.line[last] + 1;
+	}
+	if (map.percent[last] < 1) {
+		map.line.push(lineCount || 1e10);
+		map.percent.push(1);
+	} else {
+		map.line[last] = lineCount || 1e10;
+	}
+	scrollmap.map_ = map;
+	return map;
+};
+
+scrollmap.isPresent = () => {
+	const map = scrollmap.get_();
+	return !!map;
+};
+
+scrollmap.translateLV_ = (percent, l2v = true) => {
+	// If the input is out of (0,1) or not number, it is not translated.
+	if (!(0 < percent && percent < 1)) return percent;
+	const map = scrollmap.get_();
+	if (!map || map.line.length <= 2) return percent; // No translation
+	const lineCount = map.lineCount;
+	const values = l2v ? map.line : map.percent;
+	const target = l2v ? percent * lineCount : percent;
+	// Binary search (rightmost): finds where map[r-1][field] <= target < map[r][field]
+	let l = 1, r = values.length - 1;
+	while (l < r) {
+		const m = Math.floor(l + (r - l) / 2);
+		if (target < values[m]) r = m; else l = m + 1;
+	}
+	const lineU = map.line[r - 1];
+	const lineL = Math.min(lineCount, map.line[r]);
+	const vPercentU = map.percent[r - 1];
+	const vPercentL = map.percent[r];
+	let linInterp, result;
+	if (l2v) {
+		linInterp = (percent * lineCount - lineU) / (lineL - lineU);
+		result = vPercentU + (vPercentL - vPercentU) * linInterp;
+	} else {
+		linInterp = (percent - vPercentU) / (vPercentL - vPercentU);
+		result = (lineU + (lineL - lineU) * linInterp) / lineCount;
+	}
+	return Math.max(0, Math.min(1, result));
+};
+
+scrollmap.translateL2V = (lPercent) => {
+	return scrollmap.translateLV_(lPercent, true);
+};
+
+scrollmap.translateV2L = (vPercent) => {
+	return scrollmap.translateLV_(vPercent, false);
+};

packages/app-desktop/gui/utils/SyncScrollMap.ts

@@ -1,92 +0,0 @@
-import shim from '@joplin/lib/shim';
-
-// SyncScrollMap is used for synchronous scrolling between Markdown Editor and Viewer.
-// It has the mapping information between the line numbers of a Markdown text and
-// the scroll positions (percents) of the elements in the HTML document transformed
-// from the Markdown text.
-// To see the detail of synchronous scrolling, refer the following design document.
-// https://github.com/laurent22/joplin/pull/5512#issuecomment-931277022
-
-export interface SyncScrollMap {
-	line: number[];
-	percent: number[];
-	viewHeight: number;
-}
-
-// Map creation utility class
-export class SyncScrollMapper {
-	private map_: SyncScrollMap = null;
-	private refreshTimeoutId_: any = null;
-	private refreshTime_ = 0;
-
-	// Invalidates an outdated SyncScrollMap.
-	// For a performance reason, too frequent refresh requests are
-	// skippend and delayed. If forced is true, refreshing is immediately performed.
-	public refresh(forced: boolean) {
-		const elapsed = this.refreshTime_ ? Date.now() - this.refreshTime_ : 10 * 1000;
-		if (!forced && (elapsed < 200 || this.refreshTimeoutId_)) {
-			// to avoid too frequent recreations of a sync-scroll map.
-			if (this.refreshTimeoutId_) {
-				shim.clearTimeout(this.refreshTimeoutId_);
-				this.refreshTimeoutId_ = null;
-			}
-			this.refreshTimeoutId_ = shim.setTimeout(() => {
-				this.refreshTimeoutId_ = null;
-				this.map_ = null;
-				this.refreshTime_ = Date.now();
-			}, 200);
-		} else {
-			this.map_ = null;
-			this.refreshTime_ = Date.now();
-		}
-	}
-
-	// Creates a new SyncScrollMap or reuses an existing one.
-	public get(doc: Document): SyncScrollMap {
-		// Returns a cached translation map between editor's scroll percenet
-		// and viewer's scroll percent. Both attributes (line and percent) of
-		// the returned map are sorted respectively.
-		// Since creating this map is costly for each scroll event, it is cached.
-		// When some update events which outdate it such as switching a note or
-		// editing a note, it has to be invalidated (using refresh()),
-		// and a new map will be created at a next scroll event.
-		if (!doc) return null;
-		const contentElement = doc.getElementById('joplin-container-content');
-		if (!contentElement) return null;
-		const height = Math.max(1, contentElement.scrollHeight - contentElement.clientHeight);
-		if (this.map_) {
-			// check whether map_ is obsolete
-			if (this.map_.viewHeight === height) return this.map_;
-			this.map_ = null;
-		}
-		// Since getBoundingClientRect() returns a relative position,
-		// the offset of the origin is needed to get its aboslute position.
-		const offset = doc.getElementById('rendered-md')?.getBoundingClientRect().top;
-		if (!offset) return null;
-		// Mapping information between editor's lines and viewer's elements is
-		// embedded into elements by the renderer.
-		// See also renderer/MdToHtml/rules/source_map.ts.
-		const elems = doc.getElementsByClassName('maps-to-line');
-		const map: SyncScrollMap = { line: [0], percent: [0], viewHeight: height };
-		// Each map entry is total-ordered.
-		let last = 0;
-		for (let i = 0; i < elems.length; i++) {
-			const top = elems[i].getBoundingClientRect().top - offset;
-			const line = Number(elems[i].getAttribute('source-line'));
-			const percent = Math.max(0, Math.min(1, top / height));
-			if (map.line[last] < line && map.percent[last] < percent) {
-				map.line.push(line);
-				map.percent.push(percent);
-				last += 1;
-			}
-		}
-		if (map.percent[last] < 1) {
-			map.line.push(1e10);
-			map.percent.push(1);
-		} else {
-			map.line[last] = 1e10;
-		}
-		this.map_ = map;
-		return map;
-	}
-}