From 72f68b23147aa657abf0c2bedadb9332866abccd Mon Sep 17 00:00:00 2001 From: chy5301 Date: Sun, 30 Aug 2026 15:36:57 +0800 Subject: [PATCH] =?UTF-8?q?fix(reader):=20=E4=BF=AE=E6=AD=A3=E6=8C=89?= =?UTF-8?q?=E7=B4=A2=E5=BC=95=E8=B7=B3=E7=AB=A0=E5=9B=A0=20decodeURICompon?= =?UTF-8?q?ent=20=E9=9D=99=E9=BB=98=E5=A4=B1=E8=B4=A5=20(#747)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit view.goTo 无条件对 target 执行 decodeURIComponent，而该函数会先把参数 ToString：数字 5 变成 "5"，对象 { index: 5 } 变成 "[object Object]"。 于是 resolveNavigation 的 typeof target === "number" 分支永远走不到， 最终落到 book.resolveHref 找不到目标，renderer.goTo 读 resolved.index 抛错又被 goTo 自己的 catch 吞掉，只在 console 留下一行 Could not go to。 用户侧表现为「点了没反应，也没有任何报错」。 受影响的是所有按索引跳章的入口：桌面端 FoliateViewer.goToIndex（TTS 章节切换、AI 引用在缺少 CFI 时的兜底、目录 page: 前缀跳转），以及移动端 经 WebView 桥接的 window.goToSection。 改为只对字符串解码，href / CFI 路径行为完全不变。数字 target 本就是这个 库的一等公民——view.js 自己就在 pushState(0) 里存数字，popstate 也是直接 把它交给 resolveNavigation，不经过解码。这一并修好了索引跳转后的前进/ 后退：此前存进 history 的是解码后的 "5"，回退时同样解析不出来。 移动端 window.goToSection 此前会先自行 resolveNavigation、再把结果对象 交给 goTo，但该对象同样会被解码破坏，所以这个绕过从未生效；它的 try/catch 也永远不会触发，因为 goTo 自己吞掉了异常。现直接把索引交给 goTo。 同步重建 assets/reader/reader.html。 --- packages/app-expo/assets/reader/reader.html | 202 +++++++++--------- .../assets/reader/reader.template.html | 14 +- .../section-navigation-contract.test.ts | 39 ++++ packages/foliate-js/view.js | 7 +- 4 files changed, 149 insertions(+), 113 deletions(-) create mode 100644 packages/app-expo/src/screens/reader/section-navigation-contract.test.ts diff --git a/packages/app-expo/assets/reader/reader.html b/packages/app-expo/assets/reader/reader.html index 03d2d186a..78c9449f3 100644 --- a/packages/app-expo/assets/reader/reader.html +++ b/packages/app-expo/assets/reader/reader.html @@ -343,17 +343,29 @@ } } - function getMobileReflowMaxInlineSize() { - const viewportHeight = window.innerHeight || document.documentElement.clientHeight || 720; - const viewportWidth = window.innerWidth || document.documentElement.clientWidth || 393; - return Math.ceil(Math.max(720, viewportHeight, viewportWidth)); - } - function applyMobileReflowRendererSizing(renderer) { if (!renderer) return; renderer.setAttribute('max-column-count', '1'); renderer.setAttribute('max-block-size', '1440px'); - renderer.setAttribute('max-inline-size', getMobileReflowMaxInlineSize() + 'px'); + } + + // Mobile is always single-page (no spread), so there is no column gutter to + // balance — the foliate gap only contributes the page's internal side + // padding. Same derivation as the desktop reader: paper = 99% of the window + // (breathing room even at margin 0), outer text inset = margin + 0.5%·H. + // All against renderer.clientWidth (foliate resolves the gap % against it). + function applyMobileReflowMargins(renderer, margin) { + if (!renderer) return; + const windowWidth = + renderer.clientWidth || window.innerWidth || document.documentElement.clientWidth || 393; + const paper = windowWidth * 0.99; + const g = margin / paper; + renderer.setAttribute('margin-top', margin + 'px'); + renderer.setAttribute('margin-bottom', margin + 'px'); + renderer.setAttribute('margin-left', '0px'); + renderer.setAttribute('margin-right', '0px'); + renderer.setAttribute('gap', g * 100 + '%'); + renderer.setAttribute('max-inline-size', paper - margin + g * windowWidth + 'px'); } function applyRendererFlowMode(renderer) { @@ -1781,11 +1793,8 @@ const renderer = el.renderer; if (renderer && !el.isFixedLayout) { applyMobileReflowRendererSizing(renderer); - // Default gap/margin - const margin = msg.pageMargin || 16; - const gapPercent = Math.max(1, Math.round((margin / 393) * 100)); - renderer.setAttribute('gap', gapPercent + '%'); - renderer.setAttribute('margin', margin + 'px'); + // Default margins (synchronized with the desktop reader formula) + applyMobileReflowMargins(renderer, msg.pageMargin || 16); } else if (renderer && el.isFixedLayout) { const paginatedLayout = msg.paginatedLayout === 'single' || msg.paginatedLayout === 'double' @@ -2047,9 +2056,7 @@ const BASELINE_FONT_SIZE = 16; const layoutScale = currentFontSize / BASELINE_FONT_SIZE; const margin = Math.round(settings.pageMargin * layoutScale); - const gapPercent = Math.max(1, Math.round((margin / 393) * 100)); - renderer.setAttribute('gap', gapPercent + '%'); - renderer.setAttribute('margin', margin + 'px'); + applyMobileReflowMargins(renderer, margin); } if (view && view.isFixedLayout) { @@ -4148,18 +4155,14 @@ window.goLeft = function () { if (view) view.goLeft(); }; window.goRight = function () { if (view) view.goRight(); }; window.goToHref = function (href) { if (view) view.goTo(href); }; - window.goToSection = async function (sectionIndex) { + window.goToSection = function (sectionIndex) { if (!view) return; const index = Number(sectionIndex); if (!Number.isInteger(index)) return; - try { - const resolved = typeof view.resolveNavigation === 'function' - ? await Promise.resolve(view.resolveNavigation(index)) - : { index }; - return view.goTo(resolved); - } catch (e) { - try { return view.goTo({ index }); } catch (_) {} - } + // Hand the index straight to view.goTo: pre-resolving it here produced an + // object that goTo could not handle, and its try/catch never fired + // because goTo swallows navigation errors itself. + return view.goTo(index); }; window.goToCFI = function (cfi) { if (view) view.goTo(cfi); }; window.goToProgress = function (p) { if (view) view.goToFraction(p); }; @@ -5045,8 +5048,8 @@ diff --git a/packages/app-expo/assets/reader/reader.template.html b/packages/app-expo/assets/reader/reader.template.html index 8f0cf51f0..d920ee0d5 100644 --- a/packages/app-expo/assets/reader/reader.template.html +++ b/packages/app-expo/assets/reader/reader.template.html @@ -4108,18 +4108,14 @@ window.goLeft = function () { if (view) view.goLeft(); }; window.goRight = function () { if (view) view.goRight(); }; window.goToHref = function (href) { if (view) view.goTo(href); }; - window.goToSection = async function (sectionIndex) { + window.goToSection = function (sectionIndex) { if (!view) return; const index = Number(sectionIndex); if (!Number.isInteger(index)) return; - try { - const resolved = typeof view.resolveNavigation === 'function' - ? await Promise.resolve(view.resolveNavigation(index)) - : { index }; - return view.goTo(resolved); - } catch (e) { - try { return view.goTo({ index }); } catch (_) {} - } + // Hand the index straight to view.goTo: pre-resolving it here produced an + // object that goTo could not handle, and its try/catch never fired + // because goTo swallows navigation errors itself. + return view.goTo(index); }; window.goToCFI = function (cfi) { if (view) view.goTo(cfi); }; window.goToProgress = function (p) { if (view) view.goToFraction(p); }; diff --git a/packages/app-expo/src/screens/reader/section-navigation-contract.test.ts b/packages/app-expo/src/screens/reader/section-navigation-contract.test.ts new file mode 100644 index 000000000..f48726146 --- /dev/null +++ b/packages/app-expo/src/screens/reader/section-navigation-contract.test.ts @@ -0,0 +1,39 @@ +import { readFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const repositoryRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../../../.."); + +function readSource(relativePath: string): string { + return readFileSync(resolve(repositoryRoot, relativePath), "utf8"); +} + +describe("section index navigation", () => { + it("decodes only string targets so numeric indices survive goTo", () => { + const view = readSource("packages/foliate-js/view.js"); + + expect(view).toContain('if (typeof target === "string") target = decodeURIComponent(target);'); + expect(view).not.toMatch(/^\s*target = decodeURIComponent\(target\);$/m); + expect(view).toContain('if (typeof target === "number") return { index: target };'); + }); + + it("documents why an unguarded decode broke index navigation", () => { + // decodeURIComponent coerces its argument with ToString, which is what + // turned a section index into a href that resolves to nothing. + expect(decodeURIComponent(5 as unknown as string)).toBe("5"); + expect(decodeURIComponent({ index: 5 } as unknown as string)).toBe("[object Object]"); + }); + + it("passes the raw section index from the WebView bridge", () => { + const template = readSource("packages/app-expo/assets/reader/reader.template.html"); + const builtReader = readSource("packages/app-expo/assets/reader/reader.html"); + + for (const source of [template, builtReader]) { + expect(source).toMatch( + /window\.goToSection = function \(sectionIndex\) \{[\s\S]*?return view\.goTo\(index\);[\s\S]*?\};/, + ); + expect(source).not.toContain("await Promise.resolve(view.resolveNavigation(index))"); + } + }); +}); diff --git a/packages/foliate-js/view.js b/packages/foliate-js/view.js index 62739bdee..0a24225b5 100644 --- a/packages/foliate-js/view.js +++ b/packages/foliate-js/view.js @@ -512,7 +512,12 @@ export class View extends HTMLElement { } } async goTo(target) { - target = decodeURIComponent(target); + // Only hrefs and CFIs are percent-encoded. Decoding unconditionally would + // stringify a section index (5 -> "5") or a resolved target + // ({ index: 5 } -> "[object Object]"), so resolveNavigation could never + // take its `typeof target === "number"` branch and the jump failed + // silently. + if (typeof target === "string") target = decodeURIComponent(target); const resolved = this.resolveNavigation(target); try { await this.renderer.goTo(resolved);