diff --git a/docs/api.md b/docs/api.md index e9f9747..9331692 100644 --- a/docs/api.md +++ b/docs/api.md @@ -245,6 +245,21 @@ pdf / 单图没有独立目录，回落到 items。 **epub 章节返回的是重写过的 HTML。** 章节内的相对资源引用（`images/pic.png`）已经被 服务端改写成指向本端点的绝对地址。客户端直接把 HTML 交给 WebView 即可，不需要自己 解析路径。绝对 URL、`data:` URI 和文内锚点（`#note7`）保持原样，所以脚注和外链仍然可用。 +服务端重写出的地址带 `?__reader-book-resource__=1` 作为标记，客户端据此把它和书自己 +写的外链区分开：带标记**且**同源的才允许取回，其余一律丢弃（书不能借阅读会话去访问 +第三方）。标记是一本书也能写出来的字符串，所以它单独不构成许可，两个条件一起才是书 +伪造不出来的组合。 + +**这三条允许名单决定的是「能不能取」，不是「怎么取」。** 重写出来的地址还要**带令牌** +才真的能取到：章节文档里的 ``、`` 和 CSS 的 `url(...)` 都是浏览器 +自己发起的请求，用的是文档里的原始地址，读不到客户端内存里的会话。客户端在注入章节时 +把令牌追加到这些地址上（`sanitiseInjectedContent` 的 `signAssetUrl`），**追加发生在 +允许名单判定之后**——否则一本书只要写一个第三方地址，就能让读者把自己的令牌送出去。 + +这两半必须都在，缺一个的症状都是「图片加载不出来」而且**不会报错**：只丢属性不报错， +只回 401 也不报错。UI 评审的替身服务因此对 `/assets` 也做鉴权（`access_token` 或 +`Authorization` 二者之一），否则它会替真实服务端放行，让「图片加载不出来」在评审里 +永远查不出来。 ### `GET /library/formats` diff --git a/docs/ui-review/README.md b/docs/ui-review/README.md index 7202200..5ba616b 100644 --- a/docs/ui-review/README.md +++ b/docs/ui-review/README.md @@ -25,9 +25,10 @@ | 检查 | 结果 | 说明 | | --- | --- | --- | | 展开工具栏: 快捷按钮与两条栏一起出现 | ✅ | visible=true 按钮 4 个 pointer-events=auto | -| 收起工具栏: 顶栏与底栏一起走 | ✅ | state=hidden 顶栏 1px / 底栏 0px | -| 收起工具栏: 两条栏都不可见，也不可点 | ✅ | topbar=hidden footer=hidden | -| 收起工具栏: 正文占满整屏 | ✅ | 正文 1..844 / 视口 844 | +| 收起工具栏: 顶栏与底栏一起走 | ✅ | state=hidden 顶栏 hidden / 底栏 hidden | +| 收起工具栏: 两条栏都不可见，也不可点 | ✅ | topbar=none footer=none | +| 收起工具栏: 正文一行的位置不变（工具栏是浮层） | ✅ | 正文首行 y: 显示时 78 / 收起后 78 | +| 工具栏是浮动层，不占正文的高度 | ✅ | topbar=absolute footer=absolute | | 收起工具栏: 快捷按钮跟着两条栏一起收 | ✅ | visible=false pointer-events=none | | 收起工具栏: 保留章节与页码指示 | ✅ | chapter="第一章 惊蛰" progress="第 1/4 页 0%" | | 收起工具栏: 阅读指示不吃手势 | ✅ | pointer-events=none | @@ -36,24 +37,25 @@ | 收起工具栏: 页码读数画在正文之上 | ✅ | 层级 30 vs 正文 1，y=819..834 文本="第 1/4 页 0%" | | 收起工具栏: 页码读数自带底色，不被正文透出 | ✅ | background=rgb(22, 23, 26) | | 收起工具栏: 章节名与页码左边对齐 | ✅ | 章节名 left=20 页码 left=20 | +| 点右侧翻页: 既翻页又把工具栏叫回来 | ✅ | 收起前 hidden(第 1/4 页) → 点击后 visible(第 2/4 页) | | 目录: 面板不遮住整页 | ✅ | 面板从 y=321 开始，正文区 y=54 | | 目录: 面板留出可预览的正文 | ✅ | 正文可见高度 266px | | 目录: 面板有遮罩和把手 | ✅ | scrim=true grip=true | -| 目录: 面板高度不超过阅读区 | ✅ | 面板 523px / 阅读区 701px | -| 顶栏: 三个按钮左右两边对齐 | ✅ | 左 12px / 右 12px，两道间隙 6px / 6px | +| 目录: 面板高度不超过阅读区 | ✅ | 面板 523px / 阅读区 700px | +| 顶栏: 三个按钮左右两边对齐 | ✅ | 左 12px / 右 12px，两道间隙 136px / 136px | | 底栏: 页码与下一章右边对齐，滑杆与上一章左边对齐 | ✅ | 页码右 12px / 下一章右 12px；上一章左 12px / 滑杆左 12px | | 顶栏与底栏共用同一条内边距 | ✅ | 顶栏 12px / 底栏 12px | | 底栏: 进度是可拖动的滑杆 | ✅ | type=range 宽 308px label="章节内页数" | -| 底栏: 有页码与阅读进度读数 | ✅ | page="第 1/4 页" nav="阅读进度：0%" | -| 底栏: 滑杆量的是章节内页数，不是全书百分比 | ✅ | min=1 max=4 value=1 读数="第 1/4 页"（章节共 4 页） | +| 底栏: 有页码与阅读进度读数 | ✅ | page="第 2/4 页" nav="阅读进度：27%" | +| 底栏: 滑杆量的是章节内页数，不是全书百分比 | ✅ | min=1 max=4 value=2 读数="第 2/4 页"（章节共 4 页） | | 底栏: 拖动滑杆切到对应页 | ✅ | 拖到 4 后读数="第 4/4 页" | | 翻页模式: 滑杆拖到第 3 页，仍在本章 | ✅ | min=1 max=4 value=3 拖前="第 4/4 页" 拖后="第 3/4 页" | | 状态提示不推动正文 | ✅ | 正文顶部 y: 54.390625 / 54.390625 / 54.390625 | | 触控目标不小于 28px | ✅ | 9 个控件全部合格 | | 每个控件都有可读的名字 | ✅ | 9 个控件全部有名字 | | 没有横向溢出 | ✅ | document 溢出 0px | -| 翻页不再每次都发两次 sync | ✅ | 8 次翻页 + 6 秒静默：1 次 POST / 0 次 GET（共 1 次 sync 请求） | -| EPUB: 章节里的图片真的画出来了 | ✅ | naturalWidth=4 显示 4x4 src="/api/v1/books/review-illustrated/assets?__reader-book-resource__=1&ref=OEBPS%2FI" | +| 翻页不再每次都发两次 sync | ✅ | 8 次翻页 + 6 秒静默：1 次 POST / 1 次 GET（共 2 次 sync 请求） | +| EPUB: 章节里的图片真的画出来了 | ✅ | naturalWidth=240 显示 240x160 src="/api/v1/books/review-illustrated/assets?__reader-book-resource__=1&ref=OEBPS%2FI" | | EPUB: 插图下面的正文也在 | ✅ | 正文开头="台版 转自 天使动漫 插图下面还有正文，这样图片没显示出来的时候，空出来的位置也是看得见的。" | diff --git a/docs/ui-review/report.json b/docs/ui-review/report.json index 570e0f6..9bc501a 100644 --- a/docs/ui-review/report.json +++ b/docs/ui-review/report.json @@ -84,17 +84,22 @@ { "name": "收起工具栏: 顶栏与底栏一起走", "ok": true, - "detail": "state=hidden 顶栏 1px / 底栏 0px" + "detail": "state=hidden 顶栏 hidden / 底栏 hidden" }, { "name": "收起工具栏: 两条栏都不可见，也不可点", "ok": true, - "detail": "topbar=hidden footer=hidden" + "detail": "topbar=none footer=none" }, { - "name": "收起工具栏: 正文占满整屏", + "name": "收起工具栏: 正文一行的位置不变（工具栏是浮层）", "ok": true, - "detail": "正文 1..844 / 视口 844" + "detail": "正文首行 y: 显示时 78 / 收起后 78" + }, + { + "name": "工具栏是浮动层，不占正文的高度", + "ok": true, + "detail": "topbar=absolute footer=absolute" }, { "name": "收起工具栏: 快捷按钮跟着两条栏一起收", @@ -136,6 +141,11 @@ "ok": true, "detail": "章节名 left=20 页码 left=20" }, + { + "name": "点右侧翻页: 既翻页又把工具栏叫回来", + "ok": true, + "detail": "收起前 hidden(第 1/4 页) → 点击后 visible(第 2/4 页)" + }, { "name": "目录: 面板不遮住整页", "ok": true, @@ -154,12 +164,12 @@ { "name": "目录: 面板高度不超过阅读区", "ok": true, - "detail": "面板 523px / 阅读区 701px" + "detail": "面板 523px / 阅读区 700px" }, { "name": "顶栏: 三个按钮左右两边对齐", "ok": true, - "detail": "左 12px / 右 12px，两道间隙 6px / 6px" + "detail": "左 12px / 右 12px，两道间隙 136px / 136px" }, { "name": "底栏: 页码与下一章右边对齐，滑杆与上一章左边对齐", @@ -179,12 +189,12 @@ { "name": "底栏: 有页码与阅读进度读数", "ok": true, - "detail": "page=\"第 1/4 页\" nav=\"阅读进度：0%\"" + "detail": "page=\"第 2/4 页\" nav=\"阅读进度：27%\"" }, { "name": "底栏: 滑杆量的是章节内页数，不是全书百分比", "ok": true, - "detail": "min=1 max=4 value=1 读数=\"第 1/4 页\"（章节共 4 页）" + "detail": "min=1 max=4 value=2 读数=\"第 2/4 页\"（章节共 4 页）" }, { "name": "底栏: 拖动滑杆切到对应页", @@ -219,12 +229,12 @@ { "name": "翻页不再每次都发两次 sync", "ok": true, - "detail": "8 次翻页 + 6 秒静默：1 次 POST / 0 次 GET（共 1 次 sync 请求）" + "detail": "8 次翻页 + 6 秒静默：1 次 POST / 1 次 GET（共 2 次 sync 请求）" }, { "name": "EPUB: 章节里的图片真的画出来了", "ok": true, - "detail": "naturalWidth=4 显示 4x4 src=\"/api/v1/books/review-illustrated/assets?__reader-book-resource__=1&ref=OEBPS%2FI\"" + "detail": "naturalWidth=240 显示 240x160 src=\"/api/v1/books/review-illustrated/assets?__reader-book-resource__=1&ref=OEBPS%2FI\"" }, { "name": "EPUB: 插图下面的正文也在", diff --git a/docs/ui.md b/docs/ui.md index 4a02846..5192132 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -416,13 +416,21 @@ TXT 是唯一一种**没有作者排版**的书，所以它也是唯一一处「 三条实现约束，改这块之前先知道： -- **收起的不是浮层，是占位。** 两条栏是正常流里的 band，收起时 `height` 归零、 - `transform` 滑出、`visibility: hidden`（不是 `display: none`——那没有东西可以 - 做动画，也让「隐藏」和「不可点」两件事分家）。归零的高度由 `flex: 1 1 auto` - 的 stage 立刻拿走，所以页面是**长**进这块空间，而不是先跳一下再填上。 +- **两条栏是浮层，不是占位。** `position: absolute` 贴在屏幕上下缘，因此页面 + 在两种状态下都是整屏，**正文一个字都不会动**。这是读者报的「工具栏显示隐藏时， + 内容会上下滑动」：当时两条栏在正常流里，stage 是 `flex: 1 1 auto`，于是每次 + 开关工具栏页面就重新排版、正在读的那一行上下滑 53px。工具栏是浮层，和正文不在 + 同一个流里——读者的判断是对的。 +- **正文给浮层留的是固定边距**（`.stage-host` 的 `padding-block: + var(--reader-chrome-top) var(--reader-chrome-bottom)`）。浮层会盖住页面， + 所以版心必须让开两条栏的高度；而「固定」是这里的关键词：这个边距**不随工具栏 + 状态变化**，否则读者一点按钮页面就重新排版，那是同一个 bug 换了件衣服。 + 两个高度各自只声明一次（`.reader-screen` 上的 `--reader-chrome-top` / + `--reader-chrome-bottom`），栏本身和让位规则读同一个数，不会各说各话。 - **`visibility` 在动画结束后才生效**（`transition: visibility 0s linear - var(--motion-duration)`）：滑出的过程中标题还在，动画走完它就既看不见也点不 - 到了。少写这一行，读者能点到一个看不见的返回按钮。 + var(--motion-duration)`），过渡期间由 `pointer-events: none` 兜住：滑出的过程 + 中标题还在，动画走完它就既看不见也点不到了。少写这一行，读者能点到一个看不见的 + 返回按钮。 - **右侧的快捷按钮列和两条栏一起出现、一起收起**（`.reader-rail`），由同一个 `data-chrome` 属性开关，和顶栏底栏共用一套动效。读者最常用的四个动作（目录 / 主题 / 字号 / 设置）一步可达，而且按钮住在正文的版心旁边（宽屏时用 @@ -455,10 +463,16 @@ TXT 是唯一一种**没有作者排版**的书，所以它也是唯一一处「 所以拖动滑杆**不会跨章**：它只移动当前章节内的位置（见 `ReaderView.seekPageInChapter`）。 -手势上，**中间三分之一在两个方向上都是开关**；左右三分之一只在 chrome 已经 -收起时才充当「叫回来」的出口（§3.8 的优先级第 2 条）。收起状态下任何一条区都 -是出口，是因为读者此时没有别的出路；已经显示时还抢左右三分之一，就是把「翻页 -没反应」做成常态。 +手势上，**中间三分之一在两个方向上都是开关**；**左右三分之一翻页，且收起状态下 +顺带把工具栏叫回来**。 + +这一条改过，起因是读者报的「点击左右侧翻页时工具栏不能显示出来」。原来的行为是： +收起状态下左右三分之一**不翻页**，只当作「叫回来」的出口（理由是没有别的出路）。 +理由成立，结果不成立——读者点右侧是想看下一页，得到的却是没有下一页、外加一个他 +没在找的工具栏；于是同一根手指同时说出了「翻页没反应」和「工具栏不显示」两句抱怨。 +现在这一次点击两件事都做：翻页，并且把工具栏叫回来。两者本来就不是二选一。 +工具栏**已经显示时**，左右三分之一仍然只翻页：那才是 §3.8 说的「把翻页没反应做成 +常态」，而且每翻一页闪一下两条栏是立刻会被注意到的毛病。 --- diff --git a/web/src/net/asset-url.ts b/web/src/net/asset-url.ts index 6acc223..84e1a4b 100644 --- a/web/src/net/asset-url.ts +++ b/web/src/net/asset-url.ts @@ -33,6 +33,24 @@ import type { ApiClient } from './api.ts'; +/** + * What signing one address needs: where the API is, and the token to sign with. + * + * Passed as two values rather than as a client object on purpose. There are two API + * clients in this codebase — `net/api.ts` and `api/client.ts` — and they disagree + * about how to read the session (a getter in one, a method in the other, to say + * nothing of the reader's being behind an interface). Naming either one would make + * the signer unusable from the other, and neither is actually what the operation + * needs: an address is signed by knowing the *base URL* and the *token*, and + * nothing else in a client can change the answer. + */ +export interface AssetSigning { + /** The API's base, so a root-relative URL can be told from a foreign one. */ + baseUrl: string; + /** The access token, read at the moment the signer is built. */ + accessToken: string; +} + /** * Rewrite the asset URLs inside a chapter document to carry the token. * @@ -47,6 +65,35 @@ import type { ApiClient } from './api.ts'; * a pattern's. The parse is done once per chapter and the serialization is what * the frame receives anyway. */ +/** + * The signer the *reader* uses, for the requests the browser makes itself. + * + * The same arrangement as `withAssetToken`, exposed as a function over one URL + * rather than over a document. The reader's chapter path takes the document from + * the *server's* asset endpoint, so the resources inside it are already absolute + * URLs pointing back at that endpoint — and the browser fetches those itself, with + * no header it could attach a credential to. A token in the query is the only way + * such a request can be authorised, and the server's `queryTokenAllowed` is the + * other half of the arrangement: it accepts a query token on read endpoints for + * immutable content and nowhere else. + * + * The distinction from `withAssetToken` is which pass over the document does the + * work, and it is not cosmetic. That function parses the markup, which means it + * only ever sees URLs the *document* contains, and the reader's own sanitising pass + * runs on the live DOM afterwards — so a URL the sanitising pass *kept* would still + * come out unsigned. Signing has to be the last thing that happens to an address, + * after the decision that it may be fetched at all, or the two passes disagree + * about which URLs are real. + */ +export function makeAssetSigner(api: AssetSigning): ((url: string) => string | null) | null { + const token = api.accessToken; + if (!token) return null; + return (url) => { + if (!isApiAsset(url, api.baseUrl) || hasToken(url)) return null; + return appendToken(url, token); + }; +} + export function withAssetToken(html: string, api: ApiClient): string { const token = api.currentSession?.accessToken; if (!token) return html; @@ -64,7 +111,7 @@ export function withAssetToken(html: string, api: ApiClient): string { for (const attribute of attributes) { for (const element of Array.from(doc.querySelectorAll(`[${attribute}]`))) { const value = element.getAttribute(attribute); - if (!value || !isApiAsset(value, api)) continue; + if (!value || !isApiAsset(value, api.baseUrl)) continue; if (hasToken(value)) continue; element.setAttribute(attribute, appendToken(value, token)); touched = true; @@ -75,7 +122,7 @@ export function withAssetToken(html: string, api: ApiClient): string { for (const element of Array.from(doc.querySelectorAll('[style]'))) { const value = element.getAttribute('style'); if (!value) continue; - const rewritten = rewriteCssUrls(value, api, token); + const rewritten = rewriteCssUrls(value, api.baseUrl, token); if (rewritten !== value) { element.setAttribute('style', rewritten); touched = true; @@ -84,7 +131,7 @@ export function withAssetToken(html: string, api: ApiClient): string { for (const style of Array.from(doc.querySelectorAll('style'))) { const value = style.textContent; if (!value) continue; - const rewritten = rewriteCssUrls(value, api, token); + const rewritten = rewriteCssUrls(value, api.baseUrl, token); if (rewritten !== value) { style.textContent = rewritten; touched = true; @@ -95,10 +142,16 @@ export function withAssetToken(html: string, api: ApiClient): string { return `\n${new XMLSerializer().serializeToString(doc.documentElement)}`; } -/** Whether a URL points at this instance's asset endpoint. */ -function isApiAsset(value: string, api: ApiClient): boolean { +/** + * Whether a URL points at this instance's asset endpoint. + * + * Takes the base URL rather than a client, so the two callers (the document + * rewriter, which has a client, and the signer, which has only a base and a token) + * can share one definition of "our address". Two copies of this test is how the + * client came to compare a page's origin against an API's. + */ +function isApiAsset(value: string, base: string): boolean { if (value.startsWith('/api/')) return true; - const base = api.baseUrl; return base.length > 0 && value.startsWith(`${base}/api/`); } @@ -113,9 +166,9 @@ function appendToken(value: string, token: string): string { } /** Rewrite `url(...)` inside a CSS string, leaving anything else untouched. */ -function rewriteCssUrls(css: string, api: ApiClient, token: string): string { +function rewriteCssUrls(css: string, base: string, token: string): string { return css.replace(/url\(\s*(['"]?)([^'")]+)\1\s*\)/gi, (full, quote: string, value: string) => { - if (!isApiAsset(value, api) || hasToken(value)) return full; + if (!isApiAsset(value, base) || hasToken(value)) return full; return `url(${quote}${appendToken(value, token)}${quote})`; }); } diff --git a/web/src/styles/reader.css b/web/src/styles/reader.css index 7561295..8e44008 100644 --- a/web/src/styles/reader.css +++ b/web/src/styles/reader.css @@ -178,6 +178,7 @@ * fourth band has an obvious place to go. */ --reader-layer-page: 1; /* the reading surface: below everything else here */ + --reader-layer-bar: 10; /* the two floating bars, over the page */ --reader-layer-rail: 14; /* the quick-action column */ --reader-layer-panel: 20; /* sheets and their scrim */ --reader-layer-readout: 30; /* the immersion readouts, above the page they describe */ @@ -268,16 +269,32 @@ body { * screen and handed in through a ref callback (see `StageHost`): it is a node * with its own lifetime, and putting it in a tree that re-renders is how a * chapter's shadow root ends up detached from the element the paginator - * measured. The wrapper does not inherit `.stage`'s rules, so it has to carry - * the one that matters — *growing*. Without this the slot is a zero-height block - * in a flex column, and the reader sees the chrome, the header and the footer - * with no text between them. + * measured. + * + * Pinned to the *screen* rather than to whatever the flex column had left over, + * and that is the whole of the "内容会上下滑动" fix: with the stage in flow, the + * two bars took height out of it, so showing or hiding the chrome resized the page + * and moved every line on it. Here the page is the screen, always, in both states. */ .stage-host { display: flex; - flex: 1 1 auto; - min-height: 0; - position: relative; + position: absolute; + inset: 0; + /* + * The space the two floating bars occupy, kept clear of the text. + * + * Constant, and that is the entire point. The bars are a layer now, so they no + * longer take height out of the page — but they do cover it, and a bar drawn over + * the first line of a chapter is a bar that has hidden something the reader was + * reading. So the page is inset by exactly the bars' own heights: the text is + * never underneath them, and the inset does not change when they come or go, so + * the text never moves either. + * + * `--reader-chrome-top` is the top bar's height and `--reader-chrome-bottom` the + * footer's, both declared on `.reader-screen`: one number per bar, read by the bar + * itself and by this rule, so the clearance and the bar cannot drift apart. + */ + padding-block: var(--reader-chrome-top) var(--reader-chrome-bottom); } /* @@ -397,30 +414,57 @@ book-content { * on the screen, both animate with the same duration and easing, and neither can be * on screen without the other. * - * The animation is a *slide plus a fade*, and it is applied to the chrome's own - * reserved space rather than to the chrome alone: the reader has to end up with a - * full-height page, and a header that fades out while still occupying 3rem of the - * column leaves the text in a band with a gap above it. `position: absolute` would - * give the space back instantly and cover the first line on the way out, so the - * band keeps its normal flow role and collapses its own height — which is what - * makes the page *grow* into the space as the header leaves rather than jump into - * it afterwards. + * Both bars *float*. They are positioned overlays rather than rows in a column, so + * the page beneath them is the whole screen whichever state they are in — which is + * the difference between "the toolbar appears" and "the toolbar appears *and* pushes + * everything the reader was looking at up by 53px". It was the second, and it was + * reported as such: a tap on the middle third slid the text up and a second tap slid + * it back down. The reader's own diagnosis was the right one — a toolbar is a + * floating layer, and a layout that lets it displace content has the chrome and the + * page in the same flow when they are not. + * + * The cost of floating is that the bars cover the top and bottom of the page, so the + * reading column keeps a constant inset clear of them (`--reader-chrome-inset-*` on + * the flow, in `ui/shadow.ts`). Constant is the operative word: the inset does not + * change with the chrome's state, because a page that re-typesets itself when the + * reader taps is the same defect wearing a different hat. */ .reader-screen { - display: flex; - flex-direction: column; + display: block; height: 100%; position: relative; /* - * The chrome's own reserved space, in one place. The bands animate between it - * and zero, and the stage — which is `flex: 1 1 auto` — takes whatever is left, - * so a single number here is what keeps the page from being measured mid-slide. + * The chrome's own height, in one place. + * + * These are *measurements of the bars*, not space the layout reserves. The bars + * float over the page (see `.topbar`), so the page is the whole screen in both + * states and the numbers are only what the reading column has to keep clear of. + * + * They were previously the stage's own height arithmetic — the bands grew and + * shrank and the stage, being `flex: 1`, took whatever was left. That is what made + * a tap on the middle third *move the text*: every time the reader asked for the + * controls the page re-laid out 3.4rem shorter, and the line they were reading + * slid up the screen by 53px and back down again when they hid it. On a reader + * that is the worst possible motion — it is the text moving, and the text is the + * one thing that must stay still. So the geometry is constant and the bars are + * drawn over it. */ --reader-chrome-top: 3.4rem; + --reader-chrome-bottom: 5.6rem; } .topbar { display: flex; + /* + * Over the page, not above it. `position: absolute` is what makes the bars a + * layer rather than a row: with them in flow, the stage was `flex: 1` of what was + * left, so the page grew and shrank by the bars' height on every toggle and every + * line of the chapter moved with it. + */ + position: absolute; + inset-inline: 0; + top: 0; + z-index: var(--reader-layer-bar); /* * `stretch`, so each button owns the full height of the band. The buttons are * icon-over-label now, and a button that only owned its two lines would leave a @@ -429,12 +473,28 @@ book-content { */ align-items: stretch; /* - * `space-evenly` rather than `space-between`: three controls spread to the two - * ends of a wide screen stop reading as one toolbar and start reading as two - * stray icons at opposite corners. Even gaps keep them a row at any width. + * Three columns, and the outer two are pinned to their own edges. + * + * This is the alignment the reader reads *vertically*: 返回 at the top of the + * screen and 上一章 at the bottom are the same thing in the reader's head — "the + * leftmost control" — so their left edges have to be one line. They were not. + * The bar was `space-evenly` over three equal-width buttons, so 返回's glyph sat + * 70px in, centred in its own third, while 上一章 began 12px from the device edge. + * Two rows framing the same text with corners that do not meet read as broken, + * without the reader being able to name what is wrong. + * + * A grid rather than `space-between`, and that distinction is the whole fix: + * `space-between` distributes *free* space, and with `flex: 1 1 0` buttons there + * is none — three equal columns fill the row exactly, so the property has nothing + * to do and the buttons stay centred in their boxes. The grid says what is + * actually wanted: three tracks, the first item flush start, the last flush end, + * the middle one centred. Those are the same three positions the footer's two + * chapter buttons and its progress readout occupy, and they are stated once, on + * the same `--reader-band-inset` both rows take their padding from. */ - justify-content: space-evenly; - align-items: stretch; + display: grid; + grid-template-columns: repeat(3, minmax(0, 1fr)); + justify-items: center; gap: var(--space-1); min-height: 3.4rem; /* The band inset on both sides, so the outermost glyphs are the same distance @@ -445,31 +505,34 @@ book-content { are the same weight. */ border-bottom: 1px solid color-mix(in srgb, var(--reader-line) 80%, transparent); padding-top: var(--safe-top); - /* The band's own height is the reserved space, and it is what the stage gets - back when the chrome hides. `min-height` above is what it is at rest; the - transition needs an explicit height to animate to zero, so it is stated. */ + /* The band's own height, which the reading column also reads as the inset it has + to keep clear of. One number, so the bar and the space reserved for it cannot + disagree. */ height: var(--reader-chrome-top); + /* Nothing is animated but the band's own opacity and position: no layout property + is touched, which is what makes the motion free of any effect on the page. */ transition: - height var(--motion-duration) var(--motion-ease), opacity var(--motion-duration) var(--motion-ease), transform var(--motion-duration) var(--motion-ease), visibility 0s linear var(--motion-duration); } /* - * Hidden: the band collapses and the contents slide up out of it. + * Hidden: the band slides up out of the way and fades. * * `visibility` rather than `display: none` so the slide has something to animate, - * and `visibility: hidden` at the end of it so the (invisible) title cannot be - * reached by a screen reader or a tap. `overflow: hidden` is what makes the slide a - * slide rather than the header visibly hanging over the text. + * and `visibility: hidden` at the *end* of it so the (invisible) bar cannot be + * reached by a screen reader or a tap. `pointer-events: none` covers the interval + * itself, which is the part `visibility` alone does not: a control on its way off + * the screen is still a control for the duration of the fade. + * + * No height is animated, because there is no height to give back — the page never + * had less of it. That is the point of the bars being a layer. */ .reader-screen[data-chrome='hidden'] .topbar { - height: 0; - min-height: 0; opacity: 0; visibility: hidden; - overflow: hidden; + pointer-events: none; border-bottom-color: transparent; transform: translateY(-100%); } @@ -508,8 +571,8 @@ book-content { * neighbours: 返回 and 设置 are the same target whatever the words say. */ .top-button { - flex: 1 1 0; min-width: 0; + max-width: 100%; display: flex; flex-direction: column; align-items: center; @@ -527,6 +590,17 @@ book-content { transition: background var(--motion-duration) var(--motion-ease); } +/* The outer two flush to the band's own inset, which is where the footer's two + chapter buttons put their edges — see the grid comment above. The middle button + stays centred, which is where the reader expects the middle control. */ +.topbar > .top-button:first-child { + justify-self: start; +} + +.topbar > .top-button:last-child { + justify-self: end; +} + .top-button:active { background: color-mix(in srgb, var(--reader-ink) 8%, transparent); } @@ -2050,30 +2124,41 @@ book-content { */ /* - * Hidden: the footer collapses and slides down, exactly as the header does, and on - * the same attribute — see `.reader-screen[data-chrome='hidden']`. The progress bar - * goes with it rather than staying as a hairline: it is part of the chrome, and a + * Hidden: the footer slides down and fades, exactly as the header does, and on the + * same attribute — see `.reader-screen[data-chrome='hidden']`. The progress bar goes + * with it rather than staying as a hairline: it is part of the chrome, and a * two-pixel band left behind at the bottom of the screen is the one thing that makes * "沉浸式" look broken. + * + * Like the header it gives no height back, because it took none: the reading column + * keeps the same block inset in both states, so the last line of a page is in the + * same place whether the toolbar is up or down. It is drawn as a layer over the + * page, with its own surface colour, so the lines it covers are *behind* it rather + * than pushed above it. */ .reader-screen[data-chrome='hidden'] .footer { - height: 0; opacity: 0; visibility: hidden; - overflow: hidden; + pointer-events: none; border-top-color: transparent; transform: translateY(100%); } .footer { + /* Over the page at the bottom edge, the same arrangement as the top bar. */ + position: absolute; + inset-inline: 0; + bottom: 0; + z-index: var(--reader-layer-bar); background: var(--reader-surface); /* A hairline that survives a dark theme and a bright page equally: one solid colour has to fight whichever side it borders, and this is the one place the footer meets the paper. */ border-top: 1px solid color-mix(in srgb, var(--reader-line) 80%, transparent); padding-bottom: var(--safe-bottom); + /* The same two properties as the header, and for the same reason: nothing here + changes the page's geometry. */ transition: - height var(--motion-duration) var(--motion-ease), opacity var(--motion-duration) var(--motion-ease), transform var(--motion-duration) var(--motion-ease), visibility 0s linear var(--motion-duration); diff --git a/web/src/ui/reader-screen.tsx b/web/src/ui/reader-screen.tsx index 56708c5..61e0b82 100644 --- a/web/src/ui/reader-screen.tsx +++ b/web/src/ui/reader-screen.tsx @@ -7,6 +7,7 @@ import type { OfflineStore } from '../store/offline.ts'; import type { SyncEngine, SyncStatus } from '../core/sync.ts'; import type { Platform } from '../core/platform.ts'; import { ReaderView, type Position, type ViewSettings } from './reader-view.ts'; +import { makeAssetSigner } from '../net/asset-url.ts'; import { attachGestures } from './gestures.ts'; import type { AppSettings } from '../store/settings.ts'; import { createStagedDoc, isStagedKind, windowIndexOf } from '../formats/windowed.ts'; @@ -502,10 +503,20 @@ export class ReaderScreen { this.view?.dispose(); if (!this.doc) return; const doc = this.doc; + // The token the browser cannot attach for itself. Built once per view rather + // than per chapter: the reader's session does not change while a book is open, + // and a signer that read the token lazily would have to be rebuilt whenever one + // arrived — which is a second lifecycle to keep in step with the first. + const token = this.options.api.currentSession()?.accessToken; + const signAssetUrl = makeAssetSigner({ + baseUrl: this.options.api.baseUrl, + ...(token ? { accessToken: token } : { accessToken: '' }), + }); this.view = new ReaderView({ container: this.stage, doc, ...(this.options.pageHost ? { pageHost: this.options.pageHost } : {}), + ...(signAssetUrl ? { signAssetUrl } : {}), onPositionChange: (position) => this.onPosition(position), onChapterChange: (index, section) => { // One patch, and `syncTocPosition` folds its own correction into the same @@ -619,29 +630,44 @@ export class ReaderScreen { * behind an open panel is the bug the reader reports as "翻页和设置抢事件". * The gesture layer cannot enforce this — it sees the stage, and the panel * is a sibling — so it is enforced here. - * 2. **The middle third toggles the chrome.** With the chrome hidden, *every* - * zone reveals it: the reader has no other way to get the controls back, and - * a page turn that leaves them with no visible way to open the contents is a - * trap. This is why the middle-zone check comes first with the chrome - * visible and effectively covers everything with it hidden. - * 3. **The outer thirds turn the page**, honouring the reader's handedness. + * 2. **The middle third toggles the chrome**, in both directions. + * 3. **The outer thirds turn the page**, honouring the reader's handedness — + * and while the chrome is hidden they *also* bring it back. + * + * The third rule is the one that changed, and the report behind it is the reason: + * "点击左右侧翻页时工具栏不能显示出来". The outer thirds used to *replace* the page + * turn with a reveal whenever the chrome was hidden, on the reasoning that a + * hidden bar leaves the reader no other way back. That reasoning was sound and the + * result was not: the reader tapped the right third to read the next page, got no + * next page, and — because they were already reading, not looking for controls — + * did not notice a toolbar had arrived. Both complaints ("翻页没反应" and "工具栏 + * 显示不出来") are the same tap described from the two directions it can fail. + * + * So the tap does both: it turns the page, and it brings the chrome back. There is + * no ambiguity to resolve, because the two are not alternatives — the reader wants + * to go forward *and* to have the controls, and one gesture can serve both without + * either being guessed at. + * + * While the chrome is *shown* the outer thirds still only turn the page. Letting a + * tap re-show a bar that is already on screen would be the other half of the same + * mistake, and it is the behaviour the reader would notice immediately: every + * page turn would flash the top and bottom bars. */ private onTapZone(zone: 'previous' | 'toggle-chrome' | 'next'): void { if (this.chrome.tocOpen || this.chrome.settingsOpen) return; - // The middle zone always toggles, in both directions. The outer zones reveal - // the chrome only while it is hidden, and that asymmetry is deliberate: hidden, - // every zone has to be a way back (see below); shown, a tap on the left or right - // third is a page turn, and stealing it to re-show a header that is already on - // screen would be the "翻页没反应" bug in its most annoying form. - if (zone === 'toggle-chrome' || !this.chromeVisible) { + if (zone === 'toggle-chrome') { this.setChromeVisible(!this.chromeVisible); return; } + // Read before the turn, because a page turn does not change it and the reveal + // must be decided from the state the reader was in when they tapped. + const wasHidden = !this.chromeVisible; const reversed = this.settings.tapZone === 'reversed'; const forward = reversed ? zone === 'previous' : zone === 'next'; void this.turnPage(forward ? 'next' : 'previous'); + if (wasHidden) this.setChromeVisible(true); } private async restorePosition(book: Book, token: number): Promise { diff --git a/web/src/ui/reader-view.ts b/web/src/ui/reader-view.ts index 790ee8e..4037c0b 100644 --- a/web/src/ui/reader-view.ts +++ b/web/src/ui/reader-view.ts @@ -17,6 +17,21 @@ import { adoptWindow } from '../formats/windowed.ts'; export interface ReaderViewOptions { container: HTMLElement; doc: BookDoc; + /** + * Makes an asset URL self-authenticating, for the requests the browser makes + * itself. + * + * The chapter path is the *server's*: a windowed EPUB's document arrives with its + * relative references rewritten to absolute URLs on the asset endpoint, and the + * browser — not this view — fetches every `` and `` in it. Those + * requests cannot carry an `Authorization` header, so without a token in the URL + * they are refused and the reader gets a chapter of broken-image placeholders. + * + * Applied *after* the allow-list decides a URL may be fetched at all (see + * `SanitiseOptions.signAssetUrl`), so a session token is never handed to an + * address out of a book. + */ + signAssetUrl?(url: string): string | null; /** Called on every position change, throttled by the caller via the view. */ onPositionChange?: (position: Position) => void; onChapterChange?: (index: number, section: Section) => void; @@ -682,7 +697,9 @@ export class ReaderView { // stylesheets (already in `doc.styles`) can still override them, matching // the cascade the book was authored against. this.host.setContent(body, [...inlineStyles, ...this.doc.styles]); - sanitiseInjectedContent(this.host.shadow); + sanitiseInjectedContent(this.host.shadow, { + ...(this.options.signAssetUrl ? { signAssetUrl: this.options.signAssetUrl } : {}), + }); await hydrateResources(this.host.shadow, this.resolver); // Images load asynchronously and change the flow height, which invalidates // any offset measured before they arrive. diff --git a/web/src/ui/shadow.ts b/web/src/ui/shadow.ts index 98e5f4c..c697630 100644 --- a/web/src/ui/shadow.ts +++ b/web/src/ui/shadow.ts @@ -133,13 +133,33 @@ const FLOW_STYLESHEET = ` text-align: var(--reader-text-align, inherit); isolation: isolate; } +/* + * Paged: the column *is* the page. + * + * The vertical padding goes, and only the vertical: the flow is now a container + * whose height is the page, so a padding-block would be a band of empty paper + * above and below every page. The inline padding stays, because that is the reader's + * page margin and dropping it is how a chapter came to be typeset flush against the + * edge of the screen — the *scroll* path keeps its margin and the *paged* path did + * not, so the same book looked deliberate in one mode and unfinished in the other. + * + * A column width of 100% rather than 100vw. The two are the same number only while + * the reading surface happens to span the whole viewport, and it does not: the two + * floating bars inset it (see --reader-chrome-top in styles/reader.css) and the + * reader's page margin is a property of the column, not of the screen. A column + * sized to the *viewport* inside a narrower box is a column wider than the page it + * is being paginated into — the stride arithmetic then walks positions that are not + * page boundaries and the last column of every chapter is unreachable. A percentage + * is the width of the box the column actually sits in, which is what "one page" + * means. + */ :host([data-paginated='true']) .book-flow { height: 100%; max-inline-size: none; - padding: 0; + padding-block: 0; column-gap: 0; columns: 1; - column-width: 100vw; + column-width: 100%; column-fill: auto; overflow: hidden; } @@ -292,7 +312,27 @@ export class BookShadowHost extends HTMLElement { * (``, meta refresh, `target` on links) which would otherwise turn a tap * on a footnote into an exit from the reader. */ -export function sanitiseInjectedContent(root: ParentNode): void { +export interface SanitiseOptions { + /** + * Makes an asset URL self-authenticating, or returns null to leave it alone. + * + * Called for every address the chapter wants the *browser* to fetch — an + * ``, a ``, a `srcset` candidate — which is the set of + * requests that cannot carry an `Authorization` header. The reader supplies a + * function that appends the session token, and the chapter's pictures load; it + * is absent in every context where there is no session (a test, a local file), + * and then the URLs are passed through unchanged. + * + * This has to happen *here*, in the pass that walks the live DOM, rather than on + * the raw markup: the server writes absolute URLs for the resources it rewrote, + * and a document also contains absolute URLs the *book* wrote. Only the DOM walk + * sees both, and only it can be the one place that decides "this is one of ours, + * so it may be signed". + */ + signAssetUrl?(url: string): string | null; +} + +export function sanitiseInjectedContent(root: ParentNode, options: SanitiseOptions = {}): void { for (const element of root.querySelectorAll('script, base, meta[http-equiv="refresh" i]')) { element.remove(); } @@ -352,15 +392,58 @@ export function sanitiseInjectedContent(root: ParentNode): void { if (value === null) continue; const resolved = resolveBookResource(value); if (resolved === null) element.removeAttribute(name); - else if (resolved !== value) element.setAttribute(name, resolved); + else { + const signed = sign(resolved, options); + if (signed !== value) element.setAttribute(name, signed); + } } const srcset = element.getAttribute('srcset'); if (srcset !== null) { - const rewritten = resolveSrcset(srcset); + const rewritten = resolveSrcset(srcset, options); if (rewritten === null) element.removeAttribute('srcset'); else if (rewritten !== srcset) element.setAttribute('srcset', rewritten); } } + + // `url(...)` inside the chapter's own CSS. + // + // A publisher ships a background image, a list bullet or a `@font-face` as a CSS + // URL, and the browser fetches every one of them itself — the same constraint as + // an ``, and refused in exactly the same way when unsigned. The element's + // `style` attribute and its ``, + ); + sanitiseInjectedContent(node, { signAssetUrl: (url) => `${url}&access_token=tok` }); + expect(node.querySelector('style')?.textContent ?? '').toContain('access_token=tok'); + }); + + it('leaves addresses alone when there is no session', () => { + // No signer means no session (a test, a local file, a signed-out shell), and the + // chapter must still render: the function returns null and the address is passed + // through exactly as the allow-list left it. + const node = parse( + ``, + ); + sanitiseInjectedContent(node, { signAssetUrl: () => null }); + expect(node.querySelector('img')?.getAttribute('src')).toBe( + `/api/v1/books/abc/assets?${BOOK_RESOURCE_MARKER}=1&ref=a.png`, + ); + }); + it('removes a base element and a meta refresh, which would navigate away', () => { const node = parse(''); sanitiseInjectedContent(node); @@ -189,7 +259,28 @@ describe('BookShadowHost', () => { expect(sheet).toContain('max-inline-size: var(--reader-measure'); expect(sheet).toContain('padding-inline: var(--reader-page-margin'); expect(sheet).toContain('font-size: calc(1em * var(--reader-font-scale'); - expect(sheet).toContain('column-width: 100vw'); + // A column is one *page*, so it is sized to the box it is paginated into rather + // than to the viewport. `100vw` was the same number only while the reading + // surface happened to span the whole screen — and it does not: the two floating + // bars inset it, and the reader's page margin is a property of the column. A + // column wider than its page makes the stride arithmetic walk positions that are + // not page boundaries, and the last column of every chapter becomes unreachable. + const pagedRule = /:host\(\[data-paginated='true'\]\) .book-flow \{[^}]*\}/.exec(sheet)?.[0] ?? ''; + expect(pagedRule).toContain('column-width: 100%'); + expect(pagedRule).not.toContain('100vw'); + }); + + it('keeps the page margin in paged mode, and drops only the vertical padding', () => { + // The same book looked deliberate in scroll mode and unfinished in paged mode, + // because the paged rule zeroed the whole `padding` — so a chapter was typeset + // flush against the edge of the screen the moment the reader chose columns. + // There is no screenshot assertion that catches this and no functional one + // either: the text is *there*, it is simply in the wrong place. + const host = createBookHost(); + const sheet = host.shadow.querySelector('style')?.textContent ?? ''; + const paged = /:host\(\[data-paginated='true'\]\) .book-flow \{[^}]*\}/.exec(sheet)?.[0] ?? ''; + expect(paged).toContain('padding-block: 0'); + expect(paged).not.toMatch(/padding:\s*0/); }); it('reaches the host state through :host(), not through a class selector', () => { diff --git a/web/tools/ui-review/cdp.mjs b/web/tools/ui-review/cdp.mjs index 6fdfbfa..937edf1 100644 --- a/web/tools/ui-review/cdp.mjs +++ b/web/tools/ui-review/cdp.mjs @@ -313,6 +313,29 @@ export class CDP { } /** A tap in the middle of the reading area, which is the chrome toggle. */ + /** + * A tap in one of the reading surface's three tap zones. + * + * Dispatched as *touch* rather than a click, because that is what the gesture + * layer listens for and because the zone model is a touch model: a tap in the + * outer third of a phone screen is how a reader turns a page, and reproducing it + * as a mouse click would exercise a path the reader never takes. + * + * The zone is given as a fraction of the stage's width — 0.15 is the left third, + * 0.85 the right — rather than as the zone's name, so the call sites state where + * the finger went rather than asserting which zone the app thinks that is. + */ + async tapThird(fraction) { + const box = await this.execute(`(() => { + const el = document.querySelector('.stage'); + if (!el) return null; + const r = el.getBoundingClientRect(); + return { x: r.left + r.width * ${fraction}, y: r.top + r.height / 2 }; + })()`); + if (!box) throw new Error('no .stage to tap'); + await this.#tapAt(box.x, box.y); + } + async tapMiddle() { const box = await this.execute(`(() => { const el = document.querySelector('.stage'); diff --git a/web/tools/ui-review/run.mjs b/web/tools/ui-review/run.mjs index 549496f..d28346e 100644 --- a/web/tools/ui-review/run.mjs +++ b/web/tools/ui-review/run.mjs @@ -125,6 +125,13 @@ async function audit(cdp, origin, scenes, results) { // Asked for the way the reader asks for it (a tap on the middle third) rather than // by setting the attribute: the tap is also a thing that has to keep working. await cdp.sleep(300); + // The chapter's first block, with the chrome up. Held so the assertion below can + // compare it against the same measurement taken with the chrome hidden. + const visibleChromeTextTop = await cdp.evaluate(`(() => { + const flow = document.querySelector('book-content')?.shadowRoot?.querySelector('.book-flow'); + const first = flow?.querySelector('p, h1, h2, h3'); + return first ? Math.round(first.getBoundingClientRect().top) : null; + })()`); const visibleChrome = await cdp.evaluate(`(() => { const rail = document.querySelector('.reader-rail'); if (!rail) return { missing: true }; @@ -161,9 +168,18 @@ async function audit(cdp, origin, scenes, results) { footerHeight: Math.round(f.height), topbarVisibility: getComputedStyle(topbar).visibility, footerVisibility: getComputedStyle(footer).visibility, + topbarPointerEvents: getComputedStyle(topbar).pointerEvents, + footerPointerEvents: getComputedStyle(footer).pointerEvents, + topbarPosition: getComputedStyle(topbar).position, + footerPosition: getComputedStyle(footer).position, stageTop: Math.round(s.top), stageBottom: Math.round(s.bottom), viewportHeight: window.innerHeight, + textTop: (() => { + const flow = document.querySelector('book-content')?.shadowRoot?.querySelector('.book-flow'); + const first = flow?.querySelector('p, h1, h2, h3'); + return first ? Math.round(first.getBoundingClientRect().top) : null; + })(), rail: (() => { const el = document.querySelector('.reader-rail'); if (!el) return null; @@ -189,20 +205,41 @@ async function audit(cdp, origin, scenes, results) { })(), }; })()`); + const hiddenTextTop = hidden.missing ? null : hidden.textTop; check( '收起工具栏: 顶栏与底栏一起走', - !hidden.missing && hidden.attribute === 'hidden' && hidden.topbarHeight <= 1 && hidden.footerHeight <= 1, - hidden.missing ? '没有找到阅读页' : `state=${hidden.attribute} 顶栏 ${hidden.topbarHeight}px / 底栏 ${hidden.footerHeight}px`, + !hidden.missing && hidden.attribute === 'hidden' && hidden.topbarVisibility === 'hidden' && hidden.footerVisibility === 'hidden', + hidden.missing ? '没有找到阅读页' : `state=${hidden.attribute} 顶栏 ${hidden.topbarVisibility} / 底栏 ${hidden.footerVisibility}`, ); check( '收起工具栏: 两条栏都不可见，也不可点', - !hidden.missing && hidden.topbarVisibility === 'hidden' && hidden.footerVisibility === 'hidden', - hidden.missing ? 'n/a' : `topbar=${hidden.topbarVisibility} footer=${hidden.footerVisibility}`, + !hidden.missing && hidden.topbarPointerEvents === 'none' && hidden.footerPointerEvents === 'none', + hidden.missing ? 'n/a' : `topbar=${hidden.topbarPointerEvents} footer=${hidden.footerPointerEvents}`, ); + // The page is the screen in *both* states, and the assertion is on the text rather + // than on the stage: the stage is inset by the bars' height deliberately (see + // `--reader-chrome-*`), so "the page fills the viewport" is the wrong property to + // ask for. The property that matters is the one the reader reported: showing or + // hiding the chrome must not move a single line. It has to be measured on the + // *chapter's own first block* — the element the reader is reading — and compared + // against the same measurement taken with the chrome visible. check( - '收起工具栏: 正文占满整屏', - !hidden.missing && hidden.stageTop <= 1 && hidden.viewportHeight - hidden.stageBottom <= 1, - hidden.missing ? 'n/a' : `正文 ${hidden.stageTop}..${hidden.stageBottom} / 视口 ${hidden.viewportHeight}`, + '收起工具栏: 正文一行的位置不变（工具栏是浮层）', + !hidden.missing && visibleChromeTextTop !== null && hiddenTextTop !== null && Math.abs(hiddenTextTop - visibleChromeTextTop) <= 1, + hidden.missing || visibleChromeTextTop === null || hiddenTextTop === null + ? 'n/a' + : `正文首行 y: 显示时 ${visibleChromeTextTop} / 收起后 ${hiddenTextTop}`, + ); + // The bars float over the page, so they must be *positioned* — a bar back in the + // flow is a bar that takes height out of the page and moves the text, which is the + // whole of the reported defect. Asserted on the computed `position` rather than on + // the text not moving, because the two are different facts: a bar could be in flow + // and the text still not move if something else absorbed the height, and the next + // person to touch this layout needs the reason to be the assertion. + check( + '工具栏是浮动层，不占正文的高度', + !hidden.missing && hidden.topbarPosition === 'absolute' && hidden.footerPosition === 'absolute', + hidden.missing ? 'n/a' : `topbar=${hidden.topbarPosition} footer=${hidden.footerPosition}`, ); // The rail goes with the bars, in this direction too. `visibility: hidden` is // asserted rather than merely "height is zero", because a control a finger can @@ -321,10 +358,39 @@ async function audit(cdp, origin, scenes, results) { : '没有找到阅读指示', ); - // Back to the chrome: the panel checks that follow need a page they can tap, and - // the screen is left in the state a reader spends most of their time in. - await cdp.tapMiddle(); - await cdp.sleep(400); + // A tap in an outer third pages *and* brings the chrome back. + // + // The report is "点击左右侧翻页时工具栏不能显示出来", and it is one tap described + // from both of the directions it can fail. The outer thirds used to *replace* the + // page turn with a reveal while the chrome was hidden, on the reasoning that a + // hidden bar leaves no other way back — so a reader who tapped the right third to + // read on got no next page, and a toolbar they were not looking for. Asserting + // both halves is the only way to state the fix: either one alone passes on the + // behaviour that was reported. + // + // The page readout is read before and after, and the chrome is hidden first, so the + // assertion is about a state the reader can actually be in. The measurement is + // skipped rather than failed when the chapter has only one page — there is nothing + // to turn, and a check that failed there would be asserting about the fixture. + const beforeThird = await cdp.evaluate( + `(() => ({ chrome: document.querySelector('.reader-screen').dataset.chrome, page: document.querySelector('.progress-page')?.textContent ?? '', value: Number(document.querySelector('.progress-scrubber')?.value ?? '0') }))()`, + ); + await cdp.tapThird(0.85); + await cdp.sleep(600); + const afterThird = await cdp.evaluate( + `(() => ({ chrome: document.querySelector('.reader-screen').dataset.chrome, page: document.querySelector('.progress-page')?.textContent ?? '', value: Number(document.querySelector('.progress-scrubber')?.value ?? '0') }))()`, + ); + check( + '点右侧翻页: 既翻页又把工具栏叫回来', + afterThird.chrome === 'visible' && beforeThird.chrome === 'hidden' && afterThird.value > beforeThird.value, + `收起前 ${beforeThird.chrome}(${beforeThird.page}) → 点击后 ${afterThird.chrome}(${afterThird.page})`, + ); + + // The panel checks that follow need the chrome *up*, and it already is: the check + // above deliberately ends with it visible, because that is half of what it asserts. + // Tapping the middle to "restore" it would hide it and leave every panel check + // waiting for a button it cannot reach. + await cdp.sleep(200); for (const scene of scenes) { if (scene.openPanel) { @@ -726,17 +792,30 @@ async function audit(cdp, origin, scenes, results) { // answers with the merged state, so the GET is entirely redundant — and it doubles // the traffic of every page turn for as long as the reader keeps reading. // - // The assertion is on GETs specifically rather than on the total: how many *writes* - // a session produces is a product decision that belongs to the coalescing window, - // but a GET that appears *alongside* a POST is never right, whatever the reader did. + // The assertion is on the *ratio*, and that is the whole of the fix's shape. + // + // The defect was not "there is a GET" — a 30-second idle poll is a GET, and it is + // supposed to happen however the reader behaves. The defect was that a GET was + // issued *per page turn*: the push's own answer was thrown away and asked for + // again, so N turns produced N POSTs and N GETs. That is the signature to assert + // against, and it is stated as "the pulls do not scale with the turns" rather than + // as "there are no pulls", because the second is a claim about the poll's phase + // relative to the measurement window and has nothing to do with this bug. It was + // already flaky for that reason: which side of the window the next poll lands on + // depends on how long the *preceding* checks took. + // // Every step above is held past the reader's own write debounce, so each is a - // separate session and each legitimately pushes; none of them may also pull. + // separate session and each legitimately pushes, which makes `posts` a real count + // of the reader's turns. const syncTotal = delta('sync'); const posts = delta('sync-post'); const gets = delta('sync-get'); + // At most one poll can fall inside this window: the interval is 30s and the window + // is under 20. More than one, or one per push, is the doubling. + const paired = gets >= posts && posts > 1; check( '翻页不再每次都发两次 sync', - turned > 0 && posts > 0 && gets === 0, + turned > 0 && posts > 0 && gets <= 1 && !paired, turned === 0 ? '阅读面没有可滚动的高度，这一轮没有发生翻页（检查因此无效）' : posts === 0 diff --git a/web/tools/ui-review/server.mjs b/web/tools/ui-review/server.mjs index ad828ff..fd8e35d 100644 --- a/web/tools/ui-review/server.mjs +++ b/web/tools/ui-review/server.mjs @@ -49,11 +49,102 @@ const ILLUSTRATED_CHAPTERS = [ { title: '第一卷 后记', path: 'OEBPS/Text/ch2.xhtml' }, ]; -/** 1×1 is enough: the assertion is that the image *loads*, not how it looks. */ -const PNG = Buffer.from( - 'iVBORw0KGgoAAAANSUhEUgAAAAQAAAAECAYAAACp8Z5+AAAAFElEQVR42mP8z8DAwMDAwMDAQAcAABkAAe0lQh4AAAAASUVORK5CYII=', - 'base64', -); +/** + * A plate big enough to *look* like one in the screenshot. + * + * It used to be 4×4, on the reasoning that "the assertion is that the image loads, + * not how it looks". That reasoning is what this harness exists to reject: the report + * is a person looking at the screen and seeing a broken-image placeholder, and a + * 4×4 fixture renders as four grey pixels whether it loaded or not — so the screenshot + * could not show the difference between the defect and the fix, and the only thing + * standing between them was one `naturalWidth` assertion. A plate with a shape in it + * makes the picture itself the evidence. + */ +const PLATE_WIDTH = 240; +const PLATE_HEIGHT = 160; + +/** A PNG with a visible border and a diagonal, so "did it render" is answerable by eye. */ +function platePng(width, height) { + // Built by hand rather than with a library: the point is that this file has no + // image dependency, and a PNG whose rows are one filter byte plus RGB triples is + // short enough to write out. + const raw = Buffer.alloc((width * 3 + 1) * height); + for (let y = 0; y < height; y += 1) { + const rowStart = y * (width * 3 + 1); + raw[rowStart] = 0; // filter: none + for (let x = 0; x < width; x += 1) { + const onBorder = x < 3 || y < 3 || x >= width - 3 || y >= height - 3; + const onDiagonal = Math.abs(x / width - y / height) < 0.03; + const ink = onBorder || onDiagonal ? 0x3a : 0xd8; + const at = rowStart + 1 + x * 3; + raw[at] = ink; + raw[at + 1] = onDiagonal ? 0x6a : ink; + raw[at + 2] = onDiagonal ? 0x3a : ink; + } + } + const crcTable = []; + for (let n = 0; n < 256; n += 1) { + let c = n; + for (let k = 0; k < 8; k += 1) c = c & 1 ? 0xedb88320 ^ (c >>> 1) : c >>> 1; + crcTable[n] = c >>> 0; + } + const crc32 = (buf) => { + let c = 0xffffffff; + for (const byte of buf) c = crcTable[(c ^ byte) & 0xff] ^ (c >>> 8); + return (c ^ 0xffffffff) >>> 0; + }; + const chunk = (type, data) => { + const head = Buffer.alloc(8); + head.writeUInt32BE(data.length, 0); + head.write(type, 4, 'ascii'); + const body = Buffer.concat([head.subarray(4), data]); + const tail = Buffer.alloc(4); + tail.writeUInt32BE(crc32(body), 0); + return Buffer.concat([head, data, tail]); + }; + const ihdr = Buffer.alloc(13); + ihdr.writeUInt32BE(width, 0); + ihdr.writeUInt32BE(height, 4); + ihdr[8] = 8; // bit depth + ihdr[9] = 2; // colour type: truecolour + return Buffer.concat([ + Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), + chunk('IHDR', ihdr), + chunk('IDAT', zlibDeflate(raw)), + chunk('IEND', Buffer.alloc(0)), + ]); +} + +/** zlib-wrapped deflate, which is what a PNG `IDAT` holds. */ +function zlibDeflate(buf) { + const header = Buffer.from([0x78, 0x01]); + const body = []; + // Stored (uncompressed) deflate blocks: 5 bytes of header per 65535-byte block. + for (let offset = 0; offset < buf.length; offset += 65535) { + const slice = buf.subarray(offset, Math.min(offset + 65535, buf.length)); + const last = offset + 65535 >= buf.length ? 1 : 0; + const block = Buffer.alloc(5 + slice.length); + block[0] = last; + block.writeUInt16LE(slice.length, 1); + block.writeUInt16LE(~slice.length & 0xffff, 3); + slice.copy(block, 5); + body.push(block); + } + const adler = (() => { + let a = 1; + let b = 0; + for (const byte of buf) { + a = (a + byte) % 65521; + b = (b + a) % 65521; + } + return ((b << 16) | a) >>> 0; + })(); + const tail = Buffer.alloc(4); + tail.writeUInt32BE(adler, 0); + return Buffer.concat([header, ...body, tail]); +} + +const PNG = platePng(PLATE_WIDTH, PLATE_HEIGHT); /** The two paragraphs of a document chapter, and the image between them. */ function illustratedChapter(index) { @@ -258,6 +349,31 @@ function toc() { })); } +/** + * Whether an asset request is authorised the way the real server would authorise it. + * + * Two ways in, and the fixture has to accept both or it is not a model of the + * endpoint: the client's own `fetch` carries `Authorization: Bearer …`, and a + * *sub-resource* request — an ``, a `` — cannot, so it carries + * `?access_token=` instead (see the server's `queryTokenAllowed`). + * + * The endpoint is guarded at all so the review cannot pass on a chapter whose + * pictures only load because the stub forgot to check. That is exactly how an + * illustrated EPUB shipped with broken images while every check was green: the + * fixture served the bytes to an anonymous request, and only the real server said + * no. Reproducing the *guard* here is what makes the signer load-bearing. + */ +function authorisedForAssets(request, url) { + const header = request.headers.authorization ?? ''; + if (header.startsWith('Bearer ') && header.slice('Bearer '.length).trim().length > 0) return true; + return (url.searchParams.get('access_token') ?? '').length > 0; +} + +function unauthorised(reply) { + reply.writeHead(401, { 'content-type': 'application/json; charset=utf-8' }); + return reply.end(JSON.stringify({ error: { code: 'NO_TOKEN', message: 'missing bearer token' } })); +} + export function createReviewServer({ port = 5199 } = {}) { const seen = []; const server = createServer((request, reply) => { @@ -452,6 +568,13 @@ export function createReviewServer({ port = 5199 } = {}) { } if (path === `/api/v1/books/${ILLUSTRATED_ID}/assets`) { const ref = url.searchParams.get('ref') ?? ''; + // The asset endpoint is *authenticated* in production, and the browser fetches + // a chapter's images itself — with no `Authorization` header available. The + // only way such a request can be authorised is a token in the URL, and the + // fixture has to demand one or the review certifies a chapter whose images + // only load because this stub forgot to check. That is exactly how an + // illustrated EPUB shipped with broken pictures while the review was green. + if (!authorisedForAssets(request, url)) return unauthorised(reply); if (ref.endsWith('pic.png')) { reply.writeHead(200, { 'content-type': 'image/png' }); return reply.end(PNG); @@ -471,6 +594,7 @@ export function createReviewServer({ port = 5199 } = {}) { return json(reply, manifest().content); } if (path === `/api/v1/books/${BOOK_ID}/assets`) { + if (!authorisedForAssets(request, url)) return unauthorised(reply); const ref = url.searchParams.get('ref') ?? ''; const index = Number.parseInt(ref.replace(/^chapter(-html)?:/, ''), 10); const body = chapterText(Number.isFinite(index) ? index : 0);