From 7c98bab43bb5354587e7f6a82645fa17f8203055 Mon Sep 17 00:00:00 2001 From: schroda <50052685+schroda@users.noreply.github.com> Date: Mon, 30 Dec 2024 20:51:29 +0100 Subject: [PATCH] Prevent TypeError and incorrect state updates for "pageLoadStates" Page load state updates of an already closed chapter caused TypeErrors or incorrect state updates. --- .../desktop/ReaderNavBarDesktopActions.tsx | 5 ++++- .../reader/components/viewer/ReaderPage.tsx | 9 +++++++-- .../reader/components/viewer/ReaderViewer.tsx | 20 ++++++++++++++----- .../state/ReaderStatePagesContextProvider.tsx | 4 +++- src/modules/reader/screens/Reader.tsx | 10 ++++++---- src/modules/reader/types/Reader.types.ts | 4 ++-- .../reader/types/ReaderProgressBar.types.ts | 4 ++-- src/modules/reader/utils/Reader.utils.ts | 19 ++++++++++++++---- .../reader/utils/ReaderPager.utils.tsx | 5 +++++ 9 files changed, 59 insertions(+), 21 deletions(-) diff --git a/src/modules/reader/components/overlay/navigation/desktop/ReaderNavBarDesktopActions.tsx b/src/modules/reader/components/overlay/navigation/desktop/ReaderNavBarDesktopActions.tsx index 9177a958..46f00534 100644 --- a/src/modules/reader/components/overlay/navigation/desktop/ReaderNavBarDesktopActions.tsx +++ b/src/modules/reader/components/overlay/navigation/desktop/ReaderNavBarDesktopActions.tsx @@ -104,7 +104,10 @@ const BaseReaderNavBarDesktopActions = memo( { setPageLoadStates((statePageLoadStates) => - statePageLoadStates.map((pageLoadState) => ({ loaded: pageLoadState.loaded })), + statePageLoadStates.map((pageLoadState) => ({ + url: pageLoadState.url, + loaded: pageLoadState.loaded, + })), ); setRetryFailedPagesKeyPrefix(`${pageRetryKeyPrefix.current}`); pageRetryKeyPrefix.current = (pageRetryKeyPrefix.current + 1) % 1000; diff --git a/src/modules/reader/components/viewer/ReaderPage.tsx b/src/modules/reader/components/viewer/ReaderPage.tsx index 339da444..4b0f1edc 100644 --- a/src/modules/reader/components/viewer/ReaderPage.tsx +++ b/src/modules/reader/components/viewer/ReaderPage.tsx @@ -88,10 +88,15 @@ const BaseReaderPage = ({ onError: ReaderPagerProps['onError']; setRef?: (pagesIndex: number, ref: HTMLElement | null) => void; }) => { + const { src } = props; + const isTabletWidth = MediaQuery.useIsTabletWidth(); - const handleLoad = useCallback(() => onLoad?.(pagesIndex, isPrimaryPage), [onLoad, pagesIndex, isPrimaryPage]); - const handleError = useCallback(() => onError?.(pageIndex), [onError, pageIndex]); + const handleLoad = useCallback( + () => onLoad?.(pagesIndex, src, isPrimaryPage), + [onLoad, pagesIndex, src, isPrimaryPage], + ); + const handleError = useCallback(() => onError?.(pageIndex, src), [onError, pageIndex, src]); const updateRef = useCallback((element: HTMLElement | null) => setRef?.(pagesIndex, element), [pagesIndex, setRef]); if (!display && !shouldLoad) { diff --git a/src/modules/reader/components/viewer/ReaderViewer.tsx b/src/modules/reader/components/viewer/ReaderViewer.tsx index fe3c44e4..f691a6c2 100644 --- a/src/modules/reader/components/viewer/ReaderViewer.tsx +++ b/src/modules/reader/components/viewer/ReaderViewer.tsx @@ -28,7 +28,7 @@ import { TReaderScrollbarContext, } from '@/modules/reader/types/Reader.types.ts'; import { userReaderStatePagesContext } from '@/modules/reader/contexts/state/ReaderStatePagesContext.tsx'; -import { getDoublePageModePages } from '@/modules/reader/utils/ReaderPager.utils.tsx'; +import { getDoublePageModePages, isPageOfOutdatedPageLoadStates } from '@/modules/reader/utils/ReaderPager.utils.tsx'; import { useReaderScrollbarContext } from '@/modules/reader/contexts/ReaderScrollbarContext.tsx'; import { MediaQuery } from '@/modules/core/utils/MediaQuery.tsx'; import { ReaderControls } from '@/modules/reader/services/ReaderControls.ts'; @@ -171,10 +171,20 @@ const BaseReaderViewer = forwardRef( [actualPages, readingMode], ); - const onError = useCallback((pageIndex: number) => { - setPageLoadStates((statePageLoadStates) => - statePageLoadStates.toSpliced(pageIndex, 1, { loaded: false, error: true }), - ); + const onError = useCallback((pageIndex: number, url: string) => { + setPageLoadStates((statePageLoadStates) => { + const pageLoadState = statePageLoadStates[pageIndex]; + + if (isPageOfOutdatedPageLoadStates(url, pageLoadState)) { + return statePageLoadStates; + } + + return statePageLoadStates.toSpliced(pageIndex, 1, { + ...pageLoadState, + loaded: false, + error: true, + }); + }); }, []); // reset spread state diff --git a/src/modules/reader/contexts/state/ReaderStatePagesContextProvider.tsx b/src/modules/reader/contexts/state/ReaderStatePagesContextProvider.tsx index db9c3f73..2e901919 100644 --- a/src/modules/reader/contexts/state/ReaderStatePagesContextProvider.tsx +++ b/src/modules/reader/contexts/state/ReaderStatePagesContextProvider.tsx @@ -17,7 +17,9 @@ export const ReaderStatePagesContextProvider = ({ children }: { children: ReactN const [currentPageIndex, setCurrentPageIndex] = useState(0); const [pageToScrollToIndex, setPageToScrollToIndex] = useState(null); const [pageUrls, setPageUrls] = useState([]); - const [pageLoadStates, setPageLoadStates] = useState([{ loaded: false }]); + const [pageLoadStates, setPageLoadStates] = useState([ + { url: '', loaded: false }, + ]); const [pages, setPages] = useState([createPageData('', 0)]); const [transitionPageMode, setTransitionPageMode] = useState( ReaderTransitionPageMode.NONE, diff --git a/src/modules/reader/screens/Reader.tsx b/src/modules/reader/screens/Reader.tsx index a4237b87..734c5ae3 100644 --- a/src/modules/reader/screens/Reader.tsx +++ b/src/modules/reader/screens/Reader.tsx @@ -203,7 +203,7 @@ const BaseReader = ({ setTotalPages(0); setPages([createPageData('', 0)]); setPageUrls([]); - setPageLoadStates([{ loaded: false }]); + setPageLoadStates([{ url: '', loaded: false }]); setIsOverlayVisible(false); @@ -224,11 +224,13 @@ const BaseReader = ({ newPages.length - 1, ); + const newPageData = createPagesData(newPages); + setArePagesFetched(true); setTotalPages(pagesPayload.chapter.pageCount); - setPages(createPagesData(newPages)); + setPages(newPageData); setPageUrls(newPages); - setPageLoadStates(newPages.map(() => ({ loaded: false }))); + setPageLoadStates(newPageData.map(({ primary: { url } }) => ({ url, loaded: false }))); setCurrentPageIndex(initialReaderPageIndex); setPageToScrollToIndex(initialReaderPageIndex); } else { @@ -238,7 +240,7 @@ const BaseReader = ({ setTotalPages(0); setPages([createPageData('', 0)]); setPageUrls([]); - setPageLoadStates([{ loaded: false }]); + setPageLoadStates([{ url: '', loaded: false }]); } setTransitionPageMode(ReaderTransitionPageMode.NONE); diff --git a/src/modules/reader/types/Reader.types.ts b/src/modules/reader/types/Reader.types.ts index e2fc6b80..b570ea2c 100644 --- a/src/modules/reader/types/Reader.types.ts +++ b/src/modules/reader/types/Reader.types.ts @@ -234,8 +234,8 @@ export interface ReaderPagerProps | 'retryFailedPagesKeyPrefix' > { imageRefs: MutableRefObject<(HTMLElement | null)[]>; - onLoad?: (pagesIndex: number, isPrimary?: boolean) => void; - onError?: (pageIndex: number) => void; + onLoad?: (pagesIndex: number, url: string, isPrimary?: boolean) => void; + onError?: (pageIndex: number, url: string) => void; } export enum PageInViewportType { diff --git a/src/modules/reader/types/ReaderProgressBar.types.ts b/src/modules/reader/types/ReaderProgressBar.types.ts index 32d7d41e..4f4ceb05 100644 --- a/src/modules/reader/types/ReaderProgressBar.types.ts +++ b/src/modules/reader/types/ReaderProgressBar.types.ts @@ -31,8 +31,8 @@ export interface ReaderStatePages { setPageToScrollToIndex: React.Dispatch>; pageUrls: string[]; setPageUrls: React.Dispatch>; - pageLoadStates: { loaded: boolean; error?: boolean }[]; - setPageLoadStates: React.Dispatch>; + pageLoadStates: { url: string; loaded: boolean; error?: boolean }[]; + setPageLoadStates: React.Dispatch>; pages: PageData[]; setPages: React.Dispatch>; transitionPageMode: ReaderTransitionPageMode; diff --git a/src/modules/reader/utils/Reader.utils.ts b/src/modules/reader/utils/Reader.utils.ts index 7f1bf58c..10093801 100644 --- a/src/modules/reader/utils/Reader.utils.ts +++ b/src/modules/reader/utils/Reader.utils.ts @@ -21,6 +21,7 @@ import { createPagesData, getScrollIntoViewInlineOption, getScrollToXForReadingDirection, + isPageOfOutdatedPageLoadStates, isSpreadPage, } from '@/modules/reader/utils/ReaderPager.utils.tsx'; import { ReaderStatePages } from '@/modules/reader/types/ReaderProgressBar.types.ts'; @@ -130,9 +131,13 @@ export const createUpdateReaderPageLoadState = setPageLoadStates: ReaderStatePages['setPageLoadStates'], readingMode: ReadingMode, ) => - (pagesIndex: number, isPrimary: boolean = true) => { + (pagesIndex: number, url: string, isPrimary: boolean = true) => { + if (pagesIndex > actualPages.length - 1) { + return; + } + const page = actualPages[pagesIndex]; - const { index, url } = isPrimary ? page.primary : page.secondary!; + const { index } = isPrimary ? page.primary : page.secondary!; if (readingMode === ReadingMode.DOUBLE_PAGE) { const img = new Image(); @@ -158,11 +163,17 @@ export const createUpdateReaderPageLoadState = } setPageLoadStates((statePageLoadStates) => { - if (statePageLoadStates[index].loaded) { + const pageLoadState = statePageLoadStates[index]; + + if (isPageOfOutdatedPageLoadStates(url, pageLoadState)) { return statePageLoadStates; } - return statePageLoadStates.toSpliced(index, 1, { loaded: true }); + if (pageLoadState.loaded) { + return statePageLoadStates; + } + + return statePageLoadStates.toSpliced(index, 1, { url: pageLoadState.url, loaded: true }); }); }; diff --git a/src/modules/reader/utils/ReaderPager.utils.tsx b/src/modules/reader/utils/ReaderPager.utils.tsx index 5a422447..6f3407e4 100644 --- a/src/modules/reader/utils/ReaderPager.utils.tsx +++ b/src/modules/reader/utils/ReaderPager.utils.tsx @@ -524,3 +524,8 @@ export const getScrollToXForReadingDirection = ( return getOptionForDirection(-element.scrollWidth, 0, themeDirectionForReadingDirection); }; + +export const isPageOfOutdatedPageLoadStates = ( + url: string, + pageLoadState: ReaderStatePages['pageLoadStates'][number] | undefined, +): boolean => pageLoadState === undefined || pageLoadState.url !== url;