feat(web-shell): add browser tab favicon - #6091
Conversation
|
Thanks for the PR, @wenshao! Template looks good ✓ On direction: this is a small, well-scoped polish fix — the Web Shell tab shows a generic browser glyph and fires a useless On approach: the scope is exactly right — one Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 方向:这是一个小而精准的打磨修复——Web Shell 标签页目前显示浏览器通用图标,还会触发一个无用的 方案:范围恰好——一个 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: add a The PR matches this exactly. One Minor observation: the HTML comment block is quite long (~10 lines) for a single TestingFavicon renders in browser chrome (tab bar), not the page viewport — can't be captured by terminal/headless screenshots. Verified via build artifact inspection and unit tests instead: All checks green. The favicon data URI decodes byte-for-byte to the shipping Qwen mark, Vite preserves it in the built output, and the CSP already permits 中文说明代码审查独立方案:在 PR 与方案完全一致。一个 小观察:HTML 注释块对一个 测试Favicon 渲染在浏览器外框(标签栏),不在页面视口内——无法通过终端/无头截图捕获。改为通过构建产物检查和单元测试验证:favicon data URI 逐字节解码为现有的 Qwen logo,Vite 在构建输出中保留了它,CSP 已在 — Qwen Code · qwen3.7-max |
|
This is about as clean as it gets. One file, 16 lines added, solves a real cosmetic gap — the Web Shell tab showed a generic browser glyph and fired a useless The build and verification confirm everything: SVG round-trips byte-for-byte, Vite preserves it in The HTML comment block is a touch long for a single LGTM. ✅ 中文说明这个 PR 非常干净。一个文件,加了 16 行,解决了一个实际的细节缺口——Web Shell 标签页显示浏览器通用图标,还会触发一个无用的 构建和验证确认一切正常:SVG 逐字节还原,Vite 在 HTML 注释块对一个 LGTM. ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
The Web Shell HTML shell shipped without a <link rel="icon">, so the browser tab fell back to the generic page glyph and every load fired a 404 for /favicon.ico (the daemon static server only exposes /assets/* and /, so a dist-root favicon file is unreachable). Inline the Qwen mark as a data: URI in index.html instead of adding a file + a new served route. The encoding mirrors packages/web-templates/src/export-html (encodeURIComponent(svg)) and the data: URI is already permitted by the shell CSP (img-src 'self' data:). The purple #6D44E8 brand fill stays legible on both light and dark browser tab bars.
6329a55 to
371d160
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
Noted on the force-push — my mistake; I amended to keep a single commit, but since integration auto-squashes that was unnecessary. I'll append commits from here on rather than rewrite history. For reassurance: nothing was lost by the amend — all three review threads re-anchored to the current commit 中文force-push 这点收到了 —— 是我的疏忽;我 amend 是想保持单个提交,但既然集成会自动 squash,这一步没必要。之后我会追加提交、不再改写历史。 补充说明:这次 amend 没有丢失任何东西 —— 三条评审 thread 都重新锚定到当前提交 |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
Adds a browser-tab favicon to the Web Shell. The HTML shell (
packages/web-shell/client/index.html) previously had a<title>but no<link rel="icon">, so the daemon-served Web Shell tab fell back to the browser's generic page glyph. This inlines the Qwen mark as adata:URI so the tab now shows the brand icon.The icon is inlined rather than shipped as a
/favicon.svgfile on purpose: the daemon's static server (packages/cli/src/serve/web-shell-static.ts) only exposesGET /assets/*andGET /, so a file at the dist root would never be reachable without also adding a new served route. The encoding (encodeURIComponent(svg)) and artwork match the existingpackages/web-templates/src/export-htmlfavicon, and thedata:URI is already permitted by the shell's CSP (img-src 'self' data:). The#6D44E8purple brand fill stays legible on both light and dark browser tab bars.Why it's needed
A missing favicon makes the Web Shell tab look unfinished and hard to pick out among many open tabs, and because there is no icon link the browser fires a default
GET /favicon.icoprobe that the daemon can only answer with a 404 (that path matches neither/assets/*nor the exact/route). The repo already carries this exact Qwen mark as the favicon for the desktop webui and the HTML export, so this just brings the Web Shell in line with the rest of the product at near-zero cost.Reviewer Test Plan
How to verify
cd packages/web-shell && npx vite build— the build succeeds and the inlined<link rel="icon">is copied verbatim intodist/index.html(Vite leavesdata:URIs untouched).hrefand confirm it is the brand mark byte-for-byte:npx vitest run→ 540/540 pass (incl.index.test.tsx).qwen serve --web, open the printed URL, and confirm the browser tab shows the purple Qwen mark instead of the generic page glyph.Evidence (Before & After)
A browser-tab favicon lives in browser chrome, not the page viewport, so it cannot be captured by a headless/terminal screenshot. The verifiable artifact evidence:
Before — no icon link in
index.html:Browser shows the generic page glyph; the default
GET /favicon.icoprobe has no daemon route and 404s.After — inline
data:URI favicon in the builtdist/index.html:Decodes byte-for-byte to the shipping Qwen mark (
packages/desktop/apps/webui/src/public/favicon.svg); no extra request, CSP-permitted, no more/favicon.icoprobe.Tested on
Change is a platform-independent static HTML string served by the same Express handler on every OS, so the macOS verification generalizes; Windows/Linux not run locally.
Environment (optional)
npx vite build+npx vitest runinpackages/web-shellon macOS (Node from the repo toolchain).Risk & Scope
<link>to the HTML shell. The ~1.2 KB inlinedata:URI is the same trade-off the HTML export already makes.data:URI already fitsimg-src 'self' data:).Linked Issues
None.
中文说明
这个 PR 做了什么
给 Web Shell 加上浏览器标签页图标(favicon)。HTML 外壳(
packages/web-shell/client/index.html)此前只有<title>、没有<link rel="icon">,所以 daemon 提供的 Web Shell 标签页只能显示浏览器默认的通用页面图标。这里把 Qwen logo 以data:URI 内联进去,标签页现在会显示品牌图标。之所以内联而不是放一个
/favicon.svg文件:daemon 的静态服务器(packages/cli/src/serve/web-shell-static.ts)只暴露GET /assets/*和GET /,放在 dist 根目录的文件不额外加路由根本访问不到。编码方式(encodeURIComponent(svg))和图案与现有的packages/web-templates/src/export-htmlfavicon 一致,而data:URI 已被外壳的 CSP(img-src 'self' data:)放行。紫色#6D44E8品牌填充色在浅色和深色标签栏上都清晰可辨。为什么需要
缺少 favicon 会让 Web Shell 标签页显得没做完,多标签时也不好辨认;而且因为没有 icon link,浏览器会自动发一个默认的
GET /favicon.ico探测请求,daemon 只能回 404(该路径既不匹配/assets/*也不匹配精确的/)。仓库里 desktop webui 和 HTML 导出已经在用这枚 Qwen logo 作 favicon,这个改动只是让 Web Shell 与产品其余部分对齐,成本几乎为零。评审验证计划
如何验证
cd packages/web-shell && npx vite build—— 构建成功,内联的<link rel="icon">会被原样拷进dist/index.html(Vite 不会改写data:URI)。href,确认与品牌图标逐字节一致(命令见上方英文)→ 输出round-trips to brand SVG: true。npx vitest run→ 540/540 通过(含index.test.tsx)。qwen serve --web,打开打印出的 URL,确认浏览器标签显示紫色 Qwen logo 而非通用图标。证据(前后对比)
标签页 favicon 显示在浏览器**外框(chrome)**里、不在页面视口内,无法用无头/终端截图捕获。可核验的产物证据:
Before:
index.html无 icon link,浏览器显示通用图标,默认GET /favicon.ico探测无路由 → 404。After:构建后的
dist/index.html含内联data:URI favicon,解码后与仓库现有 Qwen logo(packages/desktop/apps/webui/src/public/favicon.svg)逐字节一致;不产生额外请求,CSP 已放行,也不再有/favicon.ico探测。测试平台
仅在 macOS 本地跑了
vite build+vitest。改动是各平台一致、由同一个 Express handler 以字符串形式返回的静态 HTML,因此 macOS 的验证可推广;Windows/Linux 未本地运行。风险与范围
<link>。约 1.2 KB 的内联data:URI 与 HTML 导出已有的取舍相同。data:URI 已符合img-src 'self' data:)。关联 Issue
无。