Repository navigation
fix(webui): Mermaid 已画图形随主题切换重画(#224 评审跟进) - #231
Open
stevenjj33 wants to merge 1 commit into
Open
stevenjj33 wants to merge 1 commit into
stevenjj33 wants to merge 1 commit into
Conversation
Review follow-up on #224 (5 comments), plus one adjacent finding. 1. A diagram already on screen did not follow a theme switch: the render effect read the theme from the DOM port inside the effect, whose dependencies were only [dom, text]. The app has no reactive theme store — every theme transition ends as the appearance effect flipping the dark class on the document element — so the port grows subscribeDocumentTheme (a class-attribute MutationObserver) and the theme joins the render dependencies. A redraw keeps the previous SVG on screen until the new palette is ready, so the fence never flashes its source text. Browser case switches theme through the real settings UI with a no-reload witness. 2. The theme option's doc said 'defaults to the app's current theme' while the code defaults to light; the comment now says what the code does and why (pure function, no document). 3. Documented why the retained <style> element is safe, with the probe evidence against the pinned library under securityLevel strict: classDef/style/linkStyle reject url() and stray braces at parse time, init directive colours go through a colour parser, and the directive cannot flip securityLevel/htmlLabels. Added @import / off-page url() stripping as the backstop for a future raw-CSS surface. 4. Adjacent finding: an @{img: 'https://...'} node makes the library fetch the image over the network mid-render (verified: the request leaves the page) — a render must not turn model output into a request to an arbitrary host. Such bodies now reject before the library is imported; data: URIs draw offline and still pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#224 的评审跟进。评审 APPROVED 附 5 条意见,其中两条进代码(主题重画、注释与代码对不上),两条是确认项(见下),一条(
<style>保留)以实证 + 兜底过滤回应;另带一条超出评审点的新发现(渲染期外链 fetch)。1. 切主题时已画好的图不重画(评审点 1,进代码)
App 没有响应式的主题源——主题是 SettingsModal 的局部 state,每次变化最终都是 appearance effect 翻转
<html>上的darkclass(设置点击、跟随系统的 OS 切换都走这一条)。所以:WebuiBrowserDom端口新增subscribeDocumentTheme:平台层用 MutationObserver 观察 documentElement 的 class 属性,谁翻 class 都能听见;无 DOM / 无 MutationObserver 时降级为 no-op。WebuiMermaid订阅它,把主题作为渲染 effect 的依赖。重画期间旧 SVG 留在屏上,直到新调色板就绪——读者切主题不会看到围栏闪回源码文本。window见证标记证明没有刷新;原 reload 型用例保留(覆盖 boot 应用路径)。2.
theme选项注释与代码对不上(评审点 2,进代码)注释改为如实描述:默认
light,因为纯函数拿不到文档;组件侧显式传当前主题,线上路径不依赖默认值。3. 回落块无行号槽(评审点 3,确认不改)
维持原取舍:一条 Mermaid 围栏在任何状态下都不出行号,渲染成功与渲染失败两种形态一致;失败时突然多出行号槽反而会造成「失败了」的视觉跳变。
4.
splitting: true是全仓产物布局改动(评审点 4,确认为有意)重新确认过:动态 import 若不切分,mermaid(和 slash palette)会内联进 entry chunk,每次页面加载都要拉;
resolveClientAsset本就伺服/assets/前缀。三平台 + 打包门禁(#228–#230 落地后)均绿。5.
<style>保留的安全性(评审点 5,实证 + 兜底)对锁定版本(mermaid 12.1.0)在
securityLevel: "strict"+htmlLabels: false下用 chromium 实测:%%{init}%%themeVariables CSS 逃逸(#f00; } * { …)、@import拼接classDef/style/linkStyle带url(https://…)、{}逃逸%%{init}%%翻securityLevel/htmlLabelsclick … call/callbackclick A href "https://…"<a xlink:href="https://…">,点击才触发——与 transcript 里本来就渲染的 markdown 外链等价,不剥结论写进了净化函数的文档注释。另加两条兜底正则(
@import与外部url()→none,保留url(#fragment)与data:image):今天的库到不了这里,成本为零,防的是未来某个图型长出裸 CSS 面。6. 新发现:
@{img: 'https://…'}是渲染期网络信标(超出评审点,可要求拆掉)实测:flowchart 节点写
A@{img: 'https://evil.example/logo.png'}时,渲染期间页面真的发出对该 URL 的请求(fetch 发生在mermaid.render返回之前;DNS 解析成功时图片会被拉取)。图身是模型输出——一次渲染不该把它变成对任意主机的请求。守卫在动态 import 之前拒绝这类围栏(走既有的普通围栏回落),data:URI 自带字节、离线可画,放行(有单测钉住)。这条不在评审意见里;若认为超出本 PR 范围,可以直接 revert 掉守卫与对应两条单测,其余不受影响。
测试与证伪
markdown-mermaid.test.tsx22 → 25 条(远程图片守卫 ×1、data: URI 放行 ×1、CSS 兜底 ×1)。证伪:src 退回 63b9739 后新增用例 2 failed(守卫、兜底)/ 23 passed;恢复后 25 passed。transcript.spec.mjsmermaid+theme 5 条含新增无刷新重画用例。证伪:旧构建下新用例 红(调色板 20s 不变超时);新构建 5/5 绿。pnpm verify --profile full21 gates PASS(darwin 按配置跳过 browser 套件,已单独本地跑过);无新增文件,vitest-suites / public-source / path-metadata oracle 均无需变更。