fix(web): keep chat transcript pinned in WebKit (#3135)
This commit is contained in:
@@ -25,7 +25,8 @@ import { createDebugLogger, isDebug } from "@/lib/debug/log";
|
||||
|
||||
const paginationDebug = createDebugLogger("messages:pagination");
|
||||
|
||||
const NATIVE_BOTTOM_SCROLL_TOP = Number.MAX_SAFE_INTEGER;
|
||||
// INT32_MAX: WebKit resolves Number.MAX_SAFE_INTEGER to 0 (not bottom).
|
||||
const NATIVE_BOTTOM_SCROLL_TOP = 2_147_483_647;
|
||||
|
||||
/** Writes a clamped maximum so the browser resolves the native bottom without
|
||||
* forcing a synchronous scrollHeight layout read. */
|
||||
|
||||
@@ -407,7 +407,7 @@ describe("useScrollToDividerOrBottom — anchored-bar offset", () => {
|
||||
expect(scrollContainer.scrollTop).toBe(123);
|
||||
});
|
||||
|
||||
it("pins an appended message without reading content size", () => {
|
||||
it("uses a WebKit-safe maximum offset for pinned appends", () => {
|
||||
const { rerender } = render(
|
||||
<AutoScrollHarness isWorking={false} hasUnreadDivider={false} messages={TEST_MESSAGES} />,
|
||||
);
|
||||
@@ -442,7 +442,7 @@ describe("useScrollToDividerOrBottom — anchored-bar offset", () => {
|
||||
);
|
||||
}).not.toThrow();
|
||||
expect(writes).toBe(1);
|
||||
expect(scrollTop).toBe(Number.MAX_SAFE_INTEGER);
|
||||
expect(scrollTop).toBe(2_147_483_647);
|
||||
});
|
||||
|
||||
it("restores the disabled offset after a transient layout clamp", () => {
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
---
|
||||
created: 2026-08-29
|
||||
status: done
|
||||
requirements:
|
||||
- REQ-UI-TRANSCRIPT-AUTO-SCROLL-001
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/transcript-auto-scroll.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Keep WebKit Transcripts at the Bottom
|
||||
|
||||
## Overview
|
||||
|
||||
Replace the unsafe bottom-scroll offset with the largest signed 32-bit integer.
|
||||
Keep the write-only message path and all current scroll-owner guards.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Keep enabled transcripts at the bottom in Safari, Orion, Firefox, and
|
||||
Chromium browsers.
|
||||
- Preserve the write-only scroll command for live message commits.
|
||||
- Add a regression test for the WebKit-safe offset.
|
||||
- Keep the existing desktop and mobile scroll behavior.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Transcript controls, copy, layout, pagination, or navigation.
|
||||
- Disabled auto-scroll behavior and saved scroll positions.
|
||||
- A new Playwright browser project or a change to the CI browser matrix.
|
||||
- Browser-specific user-agent branches.
|
||||
|
||||
## Confirmed root cause
|
||||
|
||||
[PR #3093](https://github.com/kdlbs/kandev/pull/3093) changed the native bottom
|
||||
write from `scrollTop = scrollHeight` to `scrollTop = Number.MAX_SAFE_INTEGER`.
|
||||
The change removed a synchronous layout read.
|
||||
|
||||
A Playwright WebKit 26.5 probe used a container with a 900 px scroll range.
|
||||
The old `scrollHeight` write produced 900. The new `Number.MAX_SAFE_INTEGER`
|
||||
write produced 0. A `2_147_483_647` write produced 900.
|
||||
|
||||
The live-message effect calls this helper after each streamed update. WebKit
|
||||
therefore moves the transcript to the top after each update. Chromium and
|
||||
Firefox clamp the same unsafe value to the bottom.
|
||||
|
||||
## Technical approach
|
||||
|
||||
### Bottom-scroll helper
|
||||
|
||||
- Change `NATIVE_BOTTOM_SCROLL_TOP` in
|
||||
`apps/web/components/task/chat/message-list-native-scroll.ts` to
|
||||
`2_147_483_647`.
|
||||
- Keep `scrollNativeToBottom` as one write-only operation.
|
||||
- Keep the work-start, live-append, and catch-up call sites unchanged.
|
||||
|
||||
### Regression test
|
||||
|
||||
- Update `apps/web/components/task/chat/message-list-native.test.tsx`.
|
||||
- Name the failing regression `uses a WebKit-safe maximum offset for pinned appends`.
|
||||
- Make the `scrollHeight` getter throw to protect the write-only requirement.
|
||||
- Record the setter value and require `2_147_483_647`.
|
||||
|
||||
## Tests
|
||||
|
||||
| Acceptance criterion | Test evidence |
|
||||
| --- | --- |
|
||||
| `AC-UI-TRANSCRIPT-AUTO-SCROLL-001.5` | The native-scroll regression requires a WebKit-safe bottom offset after an append. |
|
||||
| `AC-UI-TRANSCRIPT-AUTO-SCROLL-001.7` | The same regression fails on any synchronous `scrollHeight` read. |
|
||||
|
||||
## E2E tests
|
||||
|
||||
- Desktop: run the enabled live-message scenario in
|
||||
`apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts` with the `chromium` project.
|
||||
- Mobile: run the matching scenario in
|
||||
`apps/web/e2e/tests/chat/mobile-auto-scroll-toggle.spec.ts` with the
|
||||
`mobile-chrome` project.
|
||||
- WebKit: the direct browser probe proves the browser-specific offset behavior.
|
||||
The unit regression applies that safe value to the product helper.
|
||||
|
||||
## Mobile design contract
|
||||
|
||||
- Desktop outcome: live messages keep the transcript at the bottom.
|
||||
- Mobile entry point: the existing Chat tab in the full-height task layout.
|
||||
- Nearest exemplar: `apps/web/components/task/task-layout.tsx` and the current
|
||||
mobile auto-scroll test.
|
||||
- Hierarchy and surface: the transcript remains the only vertical scroll owner.
|
||||
- Shared logic: desktop and mobile use the same bottom-scroll helper.
|
||||
- Mobile proof: the current mobile live-message scenario covers the shared path.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Use a WebKit-safe bottom offset](task-01-use-webkit-safe-bottom-offset.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
- RED: the regression received `9_007_199_254_740_991` and expected
|
||||
`2_147_483_647`. The other 26 focused tests passed.
|
||||
- GREEN: all 27 native-scroll tests passed after the one-line correction.
|
||||
- TypeScript typecheck and targeted ESLint passed.
|
||||
- All specification files passed lint. The specification linter's 20 tests
|
||||
passed.
|
||||
- Desktop Chromium E2E passed one live-message scenario with retries disabled.
|
||||
- Mobile Chrome E2E passed the matching Pixel 5 scenario with retries disabled.
|
||||
- Direct probes showed that `2_147_483_647` reaches the bottom in Chromium 151,
|
||||
Firefox 152, and WebKit 26.5.
|
||||
|
||||
## Risks
|
||||
|
||||
- A smaller sentinel can fail if a transcript exceeds that scroll range.
|
||||
The signed 32-bit maximum is larger than a practical browser scroll range.
|
||||
- A direct WebKit E2E project expands the CI browser matrix.
|
||||
This fix keeps that expansion out of scope.
|
||||
@@ -0,0 +1,102 @@
|
||||
---
|
||||
id: "01-use-webkit-safe-bottom-offset"
|
||||
title: "Use a WebKit-safe bottom offset"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-UI-TRANSCRIPT-AUTO-SCROLL-001
|
||||
acceptance_criteria:
|
||||
- AC-UI-TRANSCRIPT-AUTO-SCROLL-001.5
|
||||
- AC-UI-TRANSCRIPT-AUTO-SCROLL-001.7
|
||||
system_design:
|
||||
- ../../specs/ui/system-design/transcript-auto-scroll.md
|
||||
---
|
||||
|
||||
# Task 01: Use a WebKit-safe Bottom Offset
|
||||
|
||||
## Summary
|
||||
|
||||
Use the largest signed 32-bit integer for write-only bottom placement. Add a
|
||||
regression that protects WebKit behavior and the no-layout-read contract.
|
||||
|
||||
## Failing regression first
|
||||
|
||||
Add a test named `uses a WebKit-safe maximum offset for pinned appends`.
|
||||
Require the setter to receive `2_147_483_647`. The current test receives
|
||||
`Number.MAX_SAFE_INTEGER`, so the regression fails before the correction.
|
||||
|
||||
## In scope
|
||||
|
||||
- Change the native bottom offset constant.
|
||||
- Keep `scrollNativeToBottom` write-only.
|
||||
- Update the focused unit regression.
|
||||
- Run the existing desktop and mobile live-message scenarios.
|
||||
- Update the transcript auto-scroll design with the WebKit range constraint.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Other transcript scroll paths or controls.
|
||||
- Pagination, prepend restoration, and explicit message navigation.
|
||||
- New user-facing copy or responsive composition.
|
||||
- Playwright configuration and CI browser installation.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Live append writes `2_147_483_647` without reading content size.
|
||||
- Safari-compatible WebKit clamps the request to the transcript bottom.
|
||||
- Existing desktop and mobile live-message scenarios remain green.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && pnpm --filter @kandev/web test -- components/task/chat/message-list-native.test.tsx
|
||||
cd apps && pnpm --filter @kandev/web run typecheck
|
||||
cd apps/web && pnpm exec eslint components/task/chat/message-list-native-scroll.ts components/task/chat/message-list-native.test.tsx
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check -- docs/specs docs/plans apps/web/components/task/chat
|
||||
cd apps/web && pnpm e2e:run --host --project chromium tests/chat/auto-scroll-toggle.spec.ts -- --grep "enabled auto-scroll stays at the bottom for live messages" --retries=0
|
||||
cd apps/web && pnpm e2e:run --host --no-build --project mobile-chrome tests/chat/mobile-auto-scroll-toggle.spec.ts -- --grep "enabled auto-scroll stays at the bottom for live messages on mobile" --retries=0
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/components/task/chat/message-list-native-scroll.ts`
|
||||
- `apps/web/components/task/chat/message-list-native.test.tsx`
|
||||
- `docs/specs/ui/system-design/transcript-auto-scroll.md`
|
||||
- `docs/plans/webkit-transcript-auto-scroll/plan.md`
|
||||
- `docs/plans/webkit-transcript-auto-scroll/task-01-use-webkit-safe-bottom-offset.md`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
- The sentinel must remain positive after WebKit converts the scroll offset.
|
||||
- A regression that only checks Chromium cannot detect this browser difference.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- `REQ-UI-TRANSCRIPT-AUTO-SCROLL-001` acceptance criteria 5 and 7.
|
||||
- The bottom-placement section in the transcript auto-scroll design.
|
||||
- The existing native-scroll unit harness and live-message browser scenarios.
|
||||
|
||||
## Results
|
||||
|
||||
- Changed the native bottom sentinel from `Number.MAX_SAFE_INTEGER` to
|
||||
`2_147_483_647`.
|
||||
- Added a regression that keeps the append path write-only and requires the
|
||||
WebKit-safe offset.
|
||||
- The regression failed on the old sentinel and passed after the correction.
|
||||
- All 27 focused unit tests passed.
|
||||
- Typecheck, targeted ESLint, specification lint, and diff checks passed.
|
||||
- The desktop Chromium and mobile Chrome live-message scenarios passed with
|
||||
retries disabled.
|
||||
- Direct Chromium, Firefox, and WebKit probes accepted the new offset and
|
||||
placed the 900 px test container at its bottom.
|
||||
@@ -4,6 +4,7 @@ system: ui
|
||||
requirements:
|
||||
- REQ-UI-TRANSCRIPT-AUTO-SCROLL-001
|
||||
created: 2026-08-27
|
||||
updated: 2026-08-29
|
||||
owners:
|
||||
- kandev
|
||||
---
|
||||
@@ -31,12 +32,16 @@ common message-update and work-start paths call this helper. They do not read
|
||||
`scrollHeight`, `clientHeight`, a bounding rectangle, or computed style before
|
||||
the write.
|
||||
|
||||
The browser clamps the requested offset to the current maximum. The existing
|
||||
near-bottom reference remains the decision input. The helper does not add a
|
||||
second animation frame, smooth scrolling, or a resize observer.
|
||||
The helper writes `2_147_483_647`, which is the largest signed 32-bit integer.
|
||||
Chromium, Gecko, and WebKit clamp this positive offset to the current maximum.
|
||||
The helper must not write `Number.MAX_SAFE_INTEGER`. WebKit resolves that value
|
||||
to the top of the scroll container.
|
||||
|
||||
The existing near-bottom reference remains the decision input. The helper does
|
||||
not add a second animation frame, smooth scrolling, or a resize observer.
|
||||
|
||||
Tests instrument the common append path. A `scrollHeight` getter fails the test
|
||||
if that path reads it. The scroll setter records the maximum-offset request.
|
||||
if that path reads it. The scroll setter records the WebKit-safe offset.
|
||||
|
||||
## Interaction boundaries
|
||||
|
||||
@@ -73,8 +78,10 @@ change.
|
||||
## Failure modes
|
||||
|
||||
- If the container is not mounted, the helper performs no work.
|
||||
- If a browser clamps a large requested offset, the result is the native
|
||||
- Chromium, Gecko, and WebKit clamp the signed 32-bit maximum to the native
|
||||
maximum scroll position.
|
||||
- An offset outside WebKit's safe range can resolve to zero and move the
|
||||
transcript to the top.
|
||||
- If a layout restore or explicit navigation owns the scroll, the existing
|
||||
guards suppress bottom placement until that owner releases control.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user