fix(web-shell): stabilize mobile composer after resume - #8263
Merged
Conversation
Collaborator
Author
E2E Test ReportEnvironment
Scenarios
Result
8/8 passed. |
Collaborator
|
|
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: this is a real, observed failure class, not theoretical hardening. Mobile Chrome discards WebGL resources while a tab is backgrounded, and on resume the lost decorative surface can render as a white failure texture over the composer. WebGL context loss is a well-documented, spec-defined event, and the PR ships a deterministic reproduction (the new e2e test drives a real Direction: aligned. The fix degrades gracefully — the purely decorative overlay is dropped while the CSS focus treatment and all input controls stay usable, which is exactly the right tradeoff for an Size: not applicable — no core paths are touched. The change is ~15 production lines against ~69 test lines (unit + Playwright), which is a healthy ratio. Approach: the scope feels right and the diff is minimal — every edit serves the stated goal, no drive-by churn. The two changes (context-loss teardown + placeholder gating) are distinct but both squarely under "stabilize the mobile composer after resume," so keeping them together reads fine. I don't see a materially simpler path. Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个真实观测到的故障类别，而非理论性加固。移动端 Chrome 在标签页位于后台时会回收 WebGL 资源，恢复后丢失上下文的装饰层可能在输入框上方渲染为白色故障纹理。WebGL 上下文丢失是有明确文档、由规范定义的事件，而且本 PR 提供了确定性复现（新增的 e2e 测试触发真实的 方向：对齐。该修复优雅降级——丢弃纯装饰覆盖层，同时保留 CSS 聚焦效果和所有可用输入控件，对于一个 规模：不适用——未触及核心路径。改动约 15 行生产代码对约 69 行测试代码（单元 + Playwright），比例健康。 方案：范围合理，diff 极简——每一处改动都服务于既定目标，没有顺手夹带的改动。两处改动（上下文丢失清理 + 占位动画门控）虽各自独立，但都属于"稳定移动端输入框恢复后表现"这一主题，放在一起合理。没有看到更简路径。 风险：无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewIndependent proposal first: for a lost WebGL context on a purely decorative overlay, I'd stop the animation loop, drop the canvas, and guard against restarting it from later pointer/focus events — input already passes through, so there's nothing to recover. For the placeholder, I'd gate the typewriter on the empty-state flag with a default that preserves today's behavior. The PR does essentially this, and does it cleanly. The The No correctness, security, or convention concerns. The new tests are genuinely load-bearing, not green-in-name-only: the unit test asserts the canvas is removed, One non-blocking observation: Test evidenceThis is an unattended CI run, so I did not build or execute the PR's code; the signal below is the PR's own CI on the reviewed commit, fetched via the API. At review time the suite had not settled — the Linux unit job and the web-shell visual capture are still in flight, with no failures recorded so far. The Windows/macOS test legs and the integration/verify/tmux jobs are skipped on this trigger. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle the behavioural claim independently: 中文说明代码审查先给出独立方案：对于一个纯装饰覆盖层丢失 WebGL 上下文的情况，我会停止动画循环、丢弃 canvas，并防止后续指针/聚焦事件重启它——输入本来就能穿透，因此无需恢复。对于占位文本，我会用空状态标志门控打字机，并给一个保留现有行为的默认值。本 PR 基本就是这么做的，而且很干净。
无正确性、安全性或规范问题。新增测试确实是承重的，而非名义上变绿：单元测试断言 canvas 被移除、 一个非阻塞观察： 测试证据这是无人值守的 CI 运行，因此我未构建或执行 PR 代码；以下信号来自通过 API 获取的、该 PR 在被审 commit 上的自身 CI。审查时测试套件尚未结束——Linux 单元任务与 web-shell 视觉截取仍在运行，目前无失败记录。Windows/macOS 测试分支以及集成/verify/tmux 任务在此触发下被跳过。 沙箱验证可独立坐实该行为性声明： — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 4/5 — a small, correct, well-tested fix; the only thing standing between it and a clean approve is CI still being in flight. Stepping back: this solves something users actually hit — a white failed-WebGL surface over the composer after returning to a backgrounded mobile tab — and it does so with the minimum viable change. The implementation matches my independent proposal almost exactly: tear down the decorative loop on context loss, guard the restart path, and let input keep flowing through to the textarea underneath. Nothing in the diff is trying too hard; there's no abstraction, no speculative recovery path, and the placeholder gating is a one-prop change with a behavior-preserving default. The tests are what push this to a confident approve: they pin the actual behaviour (canvas removed, loop not restarted, textarea still editable after a real If I had to maintain this in six months I'd thank the author — it's the kind of graceful-degradation fix that quietly stops a mobile papercut from looking like a broken input. Approval deferred until CI lands green on 中文说明置信度：4/5 —— 一个小巧、正确、测试充分的修复；唯一阻碍它干净通过的是 CI 仍在运行。 退一步看：这解决了用户真实会遇到的问题——从后台切回移动端标签页后，输入框上方出现白色 WebGL 故障层——并且以最小可行改动实现。实现几乎完全符合我的独立方案：上下文丢失时清理装饰循环、守卫重启路径、让输入继续穿透到底层 textarea。diff 中没有过度设计，没有抽象，没有投机性的恢复路径，占位动画门控也只是单个属性改动且默认保留现有行为。 测试是让我有信心通过的原因：它们钉住了真实行为（canvas 被移除、循环不重启、真实 如果六个月后由我来维护，我会感谢作者——这是一种优雅降级的修复，悄无声息地让移动端的小毛病不至于看起来像输入框坏掉。 批准已推迟，等待 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
ytahdn
added
the
autofix/takeover
Collaborator
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Aug 1, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+306
to
+312
| const onContextLost = (event: Event) => { | ||
| event.preventDefault(); | ||
| running = false; | ||
| window.cancelAnimationFrame(frameId); | ||
| resizeObserver.disconnect(); | ||
| canvas.remove(); | ||
| }; |
Collaborator
There was a problem hiding this comment.
[Suggestion] onContextLost tears down the canvas, the rAF loop and the ResizeObserver, but leaves the four DOM listeners registered just below (pointermove on window, pointerleave on document.documentElement, focusin/focusout on target) active until the component unmounts — the effect cleanup removes them, but it only runs on unmount or a theme/reducedMotion change, not on context loss. — Concrete cost: after a webglcontextlost event (the exact mobile-Chrome-resume case this PR targets), the composer stays mounted for the rest of the session, so every subsequent pointermove anywhere in the window still runs onPointerMove, which calls target.getBoundingClientRect() (a forced layout read) plus trig that is never consumed (startLoop is correctly guarded by canvas.isConnected, so nothing restarts). It is a single layout read per pointer event rather than layout thrash, but it is pure waste for the remaining component lifetime and the fix is mechanical.
Suggested change
| const onContextLost = (event: Event) => { | |
| event.preventDefault(); | |
| running = false; | |
| window.cancelAnimationFrame(frameId); | |
| resizeObserver.disconnect(); | |
| canvas.remove(); | |
| }; | |
| const onContextLost = (event: Event) => { | |
| event.preventDefault(); | |
| running = false; | |
| window.cancelAnimationFrame(frameId); | |
| resizeObserver.disconnect(); | |
| window.removeEventListener('pointermove', onPointerMove); | |
| document.documentElement.removeEventListener('pointerleave', onPointerLeave); | |
| target.removeEventListener('focusin', onFocusIn); | |
| target.removeEventListener('focusout', onFocusOut); | |
| canvas.remove(); | |
| }; |
中文说明
onContextLost 会移除 canvas、取消 rAF 循环并断开 ResizeObserver，但紧接其下注册的四个 DOM 监听器（window 上的 pointermove、document.documentElement 上的 pointerleave、target 上的 focusin/focusout）会一直保留到组件卸载——effect 的清理函数会移除它们，但它只在卸载或 theme/reducedMotion 变化时运行，而不会在上下文丢失时运行。— 具体代价：在 webglcontextlost 事件（正是本 PR 针对的移动 Chrome 恢复场景）之后，composer 会在整个会话期间保持挂载，因此之后窗口中任意位置的 pointermove 仍会执行 onPointerMove，调用 target.getBoundingClientRect()（一次强制布局读取）以及永远不会被使用的三角函数计算（startLoop 已被 canvas.isConnected 正确守卫，因此不会重启）。这是每个指针事件一次布局读取，而非布局抖动，但在组件剩余生命周期内是纯粹的浪费，且修复方式是机械性的。
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
10150
to
+10152
| composerInputVersion={composerInputVersion} | ||
| placeholderText={composerPlaceholderText} | ||
| animatePlaceholder={isChatEmptyState} |
Collaborator
There was a problem hiding this comment.
[Suggestion] The positive branch of this wiring — animated placeholder ON in the welcome/empty state — has no test at the App integration level; only the negative branch (off in-session) is pinned, by the new mobile e2e assertion. — Failure scenario: the mutation animatePlaceholder={isChatEmptyState} → animatePlaceholder={false} survives every test in this diff: the ChatEditor unit tests render the component directly and never exercise App's prop wiring, and the mobile e2e visits /session/<id> where isChatEmptyState is already false, so toHaveCount(0) passes either way. The welcome page could silently lose its animated placeholder and CI would ship green. Suggested fix: add an e2e (or App-level render test) that loads the pre-session welcome state and asserts the typewriter is present, pairing the mobile spec's in-session count-0 assertion so both sides of the wiring are pinned.
// e.g. in a welcome-state spec
await page.goto('/');
await expect(
page.locator('[data-web-shell-composer-typewriter]'),
).toHaveCount(1);中文说明
这段接线的正向分支——在欢迎/空状态下开启动画占位文本——在 App 集成层面没有测试；只有负向分支（会话内关闭）被新的移动 e2e 断言固定。— 失败场景：将 animatePlaceholder={isChatEmptyState} 突变为 animatePlaceholder={false} 后，本 diff 中的所有测试仍能通过：ChatEditor 单元测试直接渲染组件，从不验证 App 的 prop 接线；而移动 e2e 访问的是 /session/<id>，此时 isChatEmptyState 已为 false，因此 toHaveCount(0) 无论如何都会通过。欢迎页可能会悄无声息地失去动画占位文本，而 CI 仍会绿灯通过。建议修复：增加一个 e2e（或 App 层渲染测试），加载会话前的欢迎状态并断言打字机存在，与移动 spec 中会话内 count-0 的断言配对，从而固定接线的两侧。
— qwen3.8-max-preview via Qwen Code /review
Collaborator
There was a problem hiding this comment.
Declined — not worth the diff-growth/flakiness tradeoff. The component-level default (animatePlaceholder=true) is already pinned: the pre-existing ChatEditor typewriter test renders the component with the prop omitted and asserts the typewriter mounts and plays. The remaining gap you correctly identify is the App-level wiring's positive branch (welcome state → typewriter present). Pinning that needs a welcome-state (/) e2e or an App-render test; every existing web-shell e2e visits /session/<id> against the mock daemon, so a welcome-state spec would add new scaffolding and flakiness risk disproportionate to the risk of a single-line, review-visible prop pass-through mutating to false. Happy to add a welcome-state e2e if a maintainer wants both sides of the wiring pinned.
中文说明
已拒绝——不值得以 diff 膨胀/不稳定性为代价。组件层默认值（animatePlaceholder=true）已被固定：已有的 ChatEditor 打字机测试在不传该 prop 的情况下渲染组件，并断言打字机挂载并播放。你正确指出的剩余缺口是 App 层接线的正向分支（欢迎状态 → 打字机存在）。要固定它需要一个欢迎状态（/）e2e 或 App 渲染测试；而现有所有 web-shell e2e 都在 mock daemon 下访问 /session/<id>，因此一个欢迎状态的用例会引入新的脚手架，且其不稳定性风险与“单行、审查可见的 prop 透传被改为 false”这一低风险不相称。如果维护者希望固定接线的两侧，我很乐意补充一个欢迎状态的 e2e。
Collaborator
Review —
|
Collaborator
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedThanks for the thorough review. The two items flagged as required before merge are done, plus the low-risk hardening and test nits. Two judgment-call items are declined with reasons below. Automated reviewer inline suggestions
Maintainer review (@wenshao)
Conflict notes
Verification
中文说明已处理的审查反馈感谢详尽的审查。合并前必须解决的两项已完成，低风险的加固与测试细节也已处理。另有两项属于取舍判断的问题，下面给出了拒绝理由。 自动审查器的行内建议
维护者审查（@wenshao）
冲突说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
pushed a commit
that referenced
this pull request
Aug 1, 2026
qwen-code-ci-bot
approved these changes
Aug 1, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max-preview via Qwen Code /review
wenshao
approved these changes
Aug 1, 2026
Collaborator
|
Released in v0.21.3. |
water-in-stone
pushed a commit
to water-in-stone/qwen-code
that referenced
this pull request
Aug 18, 2026
… opacity (QwenLM#9349) * Revert "fix(web-shell): stabilize mobile composer after resume (QwenLM#8263)" This reverts commit d213c85. # Conflicts: # packages/web-shell/client/e2e/web-shell.composer.mobile.spec.ts * Revert "feat(web-shell): enhance composer and empty-session animations (QwenLM#8098)" This reverts commit e379ea4. * fix(web-shell): drop QwenLM#8601 mobile-composer overrides superseded by the animation revert The QwenLM#8098 animation revert removes the position: relative / z-index: 1 on .appChatEmpty .chatViewWrap, so QwenLM#8601's position: static override (a fix for the containing-block shift that QwenLM#8098 introduced) is a no-op and its comment describes a dot field that no longer exists. Sync the e2e layout assertions to the restored pre-QwenLM#8098 values (z-index auto, static wrap at 761px) while keeping the footer-anchoring contract they verify. * style(web-shell): cap the dac composer glow at 50% opacity Design feedback: the restored composer glow reads too bright. Halve the opacity ceiling for the aura grid, the halo and the pulse glow while keeping the dac-aura-cols dance animation and the focus/pulse dynamics intact. * fix(web-shell): keep the custom-footer wrap positioned for the bottom panels Dropping the QwenLM#8601 override removed the positioned containing block for the bottom panels (status panel) in the custom-footer mobile welcome variant: that variant renders the composer footer display: contents, so its wrap is the only positioned ancestor the absolutely-positioned bottom panels can anchor to. Restore position: relative on just that variant instead of the full-positioned wrap the deleted rule had excluded, keeping the plain variants static so the footer keeps anchoring to the chat pane. * fix(web-shell): address review feedback on the animation revert - Restore disabled?: boolean on the ChatEditor test render props: two surviving call sites still pass disabled: true, and dropping the member leaves object-literal excess-property violations (latent only while test files stay out of tsc). - Update the stale chatViewWithCustomFooter marker comment in App.tsx: the :not(...) exclusion it described was removed, and the class now completes the compound selector that keeps the custom-footer wrap positioned for the bottom panels. - Document the once-only glow effect invariant and the legacy data-dac-glow marker, both restored verbatim from the pre-QwenLM#8098 implementation. --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
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
4 participants
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.
What this PR does
Gracefully removes the composer's decorative WebGL overlay when its rendering context is lost, leaving the existing CSS focus treatment and all input controls usable. It also limits the animated placeholder to the new-session welcome state so existing and resumed sessions use a quiet static placeholder.
Why it's needed
Android Chrome may discard WebGL resources while a tab remains in the background. After returning to the tab, the lost decorative surface could render as a large white failure texture above the composer. The overlay ignored pointer events, so typing still worked underneath even though the input and toolbar appeared broken. Replaying the placeholder animation in active conversations also added unnecessary visual distraction.
Reviewer Test Plan
How to verify
WEBGL_lose_contexton the composer's decorative canvas and confirm the canvas is removed while the textarea remains visible, accepts input, and leaves the send button enabled.Evidence (Before & After)
WEBGL_lose_contextevent removes only the decorative surface; the textarea and send control remain visible and usable. Existing sessions have no typewriter node, while the empty new-session state retains the animation.Tested on
Environment (optional)
Node.js 22, Vitest with jsdom, Playwright Pixel 7 mobile Chromium emulation, and a production Vite build.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
当输入框装饰层的 WebGL 渲染上下文丢失时，安全移除该纯装饰覆盖层，保留现有 CSS 聚焦效果以及所有可用的输入控件。同时将输入框占位文本的打字机动画限制在新建会话欢迎页，已有会话和恢复会话改用安静的静态占位文本。
为什么需要
Android Chrome 在标签页长时间位于后台时可能回收 WebGL 资源。返回标签页后，已经丢失上下文的装饰层可能在输入框上方显示为大块白色故障纹理。该覆盖层会忽略指针事件，因此底层输入框仍能接收输入，但输入区和操作栏看起来已经损坏。在活跃对话中反复播放占位文本动画也会造成不必要的视觉干扰。
Reviewer 测试计划
如何验证
WEBGL_lose_context，确认 canvas 被移除，而 textarea 仍然可见、可以输入，发送按钮保持启用。证据（修复前后）
WEBGL_lose_context事件只会移除装饰层；textarea 和发送控件保持可见且可用。已有会话不再包含打字机节点，空的新建会话仍保留动画。已测试平台
环境（可选）
Node.js 22、Vitest + jsdom、Playwright Pixel 7 移动 Chromium 仿真，以及 Vite 生产构建。
风险与范围
关联 Issue
无