From 138347c8212f82e25d05f728fa1dbc5dcdc55354 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sun, 7 Jun 2026 00:27:16 -0400 Subject: [PATCH] fix(progress): use page index for fixed-layout pull sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pull path (_doGetProgress + syncToProgress) still used the reflowable-oriented nav_target selection that prefers xpointer/cfi over page. For has_pages documents, if the server returned an epubcfi (e.g., a fake CFI from the web reader's comic renderer), nav_target became a CFI string, tonumber() returned nil, and GotoPage(nil) crashed koreader when the user confirmed the sync prompt. After the crash the old position was retained because GotoPage never completed. This makes the pull path symmetric with the push path (ef129fd, which omits epubcfi for paging documents): - _doGetProgress: for has_pages, select nav_target from progress.page and show the actual page number in the sync prompt instead of computing it from server_percentage × total_pages (which could display the wrong page due to rounding differences between foliate and koreader page math). - syncToProgress: add a nil guard so a missing page number skips gracefully instead of crashing. --- main.lua | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/main.lua b/main.lua index 0ff4e71..f021548 100644 --- a/main.lua +++ b/main.lua @@ -870,7 +870,10 @@ end function Bookhoard:syncToProgress(progress, percentage) logger.dbg("Bookhoard: sync to progress", progress, percentage) if self.ui.document.info.has_pages then - self.ui:handleEvent(Event:new("GotoPage", tonumber(progress))) + local page = tonumber(progress) + if page then + self.ui:handleEvent(Event:new("GotoPage", page)) + end elseif progress and progress:match("^/body/") then self.ui:handleEvent(Event:new("GotoXPointer", progress)) elseif percentage then @@ -1052,17 +1055,21 @@ function Bookhoard:_doGetProgress(interactive) local self_older = server_percentage > percentage - local nav_target = progress.koreader_xpointer or progress.epubcfi or progress.page - + local nav_target local sync_text - if not self.ui.document.info.has_pages then + if self.ui.document.info.has_pages then + -- Fixed-layout: the page index is the canonical locator. CFI/xpointer + -- are meaningless for image-based content, so use progress.page directly. + local total = progress.total_pages or self.ui.document:getPageCount() + local target_page = progress.page + or math.min(Math.round(server_percentage * total), total) + nav_target = progress.page + sync_text = T(_("Sync to page %1 of %2 from server?"), target_page, total) + else + nav_target = progress.koreader_xpointer or progress.epubcfi or progress.page local total = self.ui.document:getPageCount() local target_page = math.min(Math.round(server_percentage * total), total) sync_text = T(_("Sync to page %1 of %2 from server?"), target_page, total) - else - sync_text = T(_("Sync to page %1 of %2 from server?"), - Math.round(server_percentage * (progress.total_pages or 1)), - progress.total_pages or "?") end if self_older then