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);