diff --git a/app/public/epub/epub.html b/app/public/epub/epub.html index bf7c1012..73f1289a 100644 --- a/app/public/epub/epub.html +++ b/app/public/epub/epub.html @@ -267,9 +267,10 @@ either expressed or implied, of the FreeBSD Project. --> return Math.min((start.index + withinSection) / sections, 1); } - // Tell the WROLPi page that embeds this viewer where the reader is, so it can save their place. + // Tell the WROLPi page that embeds this viewer where the reader is, so it can save their place. Nothing is + // posted at the start of the book: that is where a finished book opens, and saving it would un-finish it. function postLocation(location) { - if (window.parent === window || !location || !location.start) { + if (window.parent === window || !location || !location.start || location.atStart) { return; } window.parent.postMessage({ diff --git a/app/src/components/CbzViewer.js b/app/src/components/CbzViewer.js index 3f526751..3b3da07d 100644 --- a/app/src/components/CbzViewer.js +++ b/app/src/components/CbzViewer.js @@ -35,6 +35,8 @@ export function CbzViewer({path, progressPath = null, fileGroup = null}) { // Read when the pages load; a later save must not move the reader. const initialPageRef = useRef(initialPage); initialPageRef.current = initialPage; + // The page shown on opening is where the reader already is (or the cover of a finished comic); it is not saved. + const justOpenedRef = useRef(true); const [rtl, setRtl] = useLocalStorage('cbzViewerRtl', false); const [fullscreen, setFullscreen] = useState(false); const [error, setError] = useState(null); @@ -51,6 +53,7 @@ export function CbzViewer({path, progressPath = null, fileGroup = null}) { .filter(name => IMAGE_EXTENSIONS.test(name)) .sort(); const urls = images.map(member => getArchiveMemberUrl(path, member)); + justOpenedRef.current = true; setPages(urls); setCurrentPage(Math.min(initialPageRef.current, Math.max(urls.length - 1, 0))); } catch (e) { @@ -61,9 +64,14 @@ export function CbzViewer({path, progressPath = null, fileGroup = null}) { }, [path]); useEffect(() => { - if (pages && pages.length > 0) { - onPageChange(currentPage, pages.length); + if (!pages || pages.length === 0) { + return; } + if (justOpenedRef.current) { + justOpenedRef.current = false; + return; + } + onPageChange(currentPage, pages.length); }, [currentPage, pages, onPageChange]); const goNext = useCallback(() => { diff --git a/app/src/components/CbzViewer.test.js b/app/src/components/CbzViewer.test.js index faedf83b..61ba6576 100644 --- a/app/src/components/CbzViewer.test.js +++ b/app/src/components/CbzViewer.test.js @@ -62,4 +62,23 @@ describe('CbzViewer progress', () => { unmount(); expect(saves()).toEqual([]); }); + + test('opening a comic saves nothing, so a finished comic stays finished', async () => { + const fileGroup = {progress: 1, position: null, viewed: new Date(9000).toISOString()}; + const {unmount} = renderWithProviders(); + expect(await screen.findByText('1 / 4')).toBeInTheDocument(); + unmount(); + expect(saves()).toEqual([]); + }); + + test('peeking at the cover keeps the reader\'s place', async () => { + const fileGroup = {progress: 0.75, position: {kind: 'page', page: 2, updated_at: 1000}}; + const {unmount} = renderWithProviders(); + expect(await screen.findByText('3 / 4')).toBeInTheDocument(); + fireEvent.click(screen.getByLabelText('Previous page')); + fireEvent.click(screen.getByLabelText('Previous page')); + expect(screen.getByText('1 / 4')).toBeInTheDocument(); + unmount(); + expect(saves()).toEqual([expect.objectContaining({position: expect.objectContaining({page: 1})})]); + }); }); diff --git a/app/src/hooks/useFileProgress.js b/app/src/hooks/useFileProgress.js index 9f2b35f3..9a05f5ba 100644 --- a/app/src/hooks/useFileProgress.js +++ b/app/src/hooks/useFileProgress.js @@ -17,6 +17,17 @@ const CACHE_PREFIX = 'wrolpi-progress:'; const FINISHED_PROGRESS = 0.95; const MINIMUM_PROGRESS = 0.01; const MINIMUM_SECONDS = 10; +// The most a reader who has not reached the end can be through a file. The server drops the position of a +// file at FINISHED_PROGRESS, so the last pages before the end stay resumable. +const BEFORE_FINISHED = 0.94; + +/** + * Progress to save for a reader who is past the start and not at the end: kept between the server's floor (below + * which it discards the position) and its finished line. + */ +export function resumableProgress(fraction) { + return Math.min(Math.max(fraction, MINIMUM_PROGRESS), BEFORE_FINISHED); +} function readCache(path) { try { @@ -233,10 +244,11 @@ export function useEpubProgress(path, fileGroup, {enabled = true} = {}) { return; } const {type, cfi, progress} = event.data || {}; - if (type !== 'wrolpi:epub-location' || typeof cfi !== 'string' || typeof progress !== 'number') { + if (type !== 'wrolpi:epub-location' || typeof cfi !== 'string' || !Number.isFinite(progress)) { return; } - report({progress, position: {kind: 'epub', cfi}}); + // The viewer posts nothing at the start of the book, so a finished book opened there stays finished. + report({progress: progress >= 1 ? 1 : resumableProgress(progress), position: {kind: 'epub', cfi}}); }; window.addEventListener('message', onMessage); return () => window.removeEventListener('message', onMessage); @@ -256,9 +268,13 @@ export function usePageProgress(path, fileGroup, {enabled = true} = {}) { const resume = useResumePosition(trackedPath, fileGroup); const onPageChange = React.useCallback((page, pageCount) => { - if (pageCount > 0) { - report({progress: (page + 1) / pageCount, position: {kind: 'page', page}}); + // The cover is not a place to resume; saving it would clear the reader's place, so a peek at it keeps + // the last page saved. + if (pageCount <= 0 || page <= 0) { + return; } + const progress = page >= pageCount - 1 ? 1 : resumableProgress((page + 1) / pageCount); + report({progress, position: {kind: 'page', page}}); }, [report]); return {initialPage: resume && resume.kind === 'page' ? resume.page : 0, onPageChange}; diff --git a/app/src/hooks/useFileProgress.test.js b/app/src/hooks/useFileProgress.test.js index 53be5fec..53507c3d 100644 --- a/app/src/hooks/useFileProgress.test.js +++ b/app/src/hooks/useFileProgress.test.js @@ -277,6 +277,7 @@ describe('useEpubProgress', () => { test('ignores a malformed message', () => { const {post, unmount} = renderEpub(); post({type: 'wrolpi:epub-location', cfi: 42, progress: 0.45}); + post({type: 'wrolpi:epub-location', cfi: 'epubcfi(/6/10)', progress: NaN}); post({type: 'something-else', cfi: 'epubcfi(/6/10)', progress: 0.45}); unmount(); expect(sent()).toEqual([]); @@ -304,6 +305,26 @@ describe('usePageProgress', () => { })]); }); + test.each([ + // [page, pages, progress]: progress stays where the server keeps the page, until the last page. + [1, 500, 0.01], + [9, 20, 0.5], + [37, 40, 0.94], + [39, 40, 1], + ])('page %i of %i is saved as %f', (page, pageCount, progress) => { + const {result, unmount} = renderHook(() => usePageProgress(PATH, null)); + act(() => result.current.onPageChange(page, pageCount)); + unmount(); + expect(sent()).toEqual([expect.objectContaining({progress})]); + }); + + test('the cover is not saved', () => { + const {result, unmount} = renderHook(() => usePageProgress(PATH, null)); + act(() => result.current.onPageChange(0, 20)); + unmount(); + expect(sent()).toEqual([]); + }); + test('a new comic opens at the first page', () => { const {result} = renderHook(() => usePageProgress(PATH, {progress: null, position: null})); expect(result.current.initialPage).toBe(0);