diff --git a/.changeset/fix-mobile-send-jump-freeze-followups.md b/.changeset/fix-mobile-send-jump-freeze-followups.md new file mode 100644 index 000000000..53e2fbc22 --- /dev/null +++ b/.changeset/fix-mobile-send-jump-freeze-followups.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Fix mobile message sending, preserve jump-to-message context during timeline refreshes, and add PWA freeze diagnostics for service worker recovery. diff --git a/docs/MANUAL_QA_ISSUES_103_109_111.md b/docs/MANUAL_QA_ISSUES_103_109_111.md new file mode 100644 index 000000000..7ad72c1f5 --- /dev/null +++ b/docs/MANUAL_QA_ISSUES_103_109_111.md @@ -0,0 +1,85 @@ +# Manual QA Checklist + +Manual checks for the current Charm follow-up work on `#111`, `#103`, and `#109`. + +## Issue #111: Mobile send button UX + +Environment: + +- iPhone-sized Safari layout +- Safari standalone PWA if available +- one room with delayed events enabled +- one room or account state where delayed events are unavailable + +Steps: + +1. Open a room and focus the composer so the keyboard is visible. +2. Type a short message and tap the main send button. +3. Confirm the message sends immediately while the keyboard is still open. +4. Type another message and tap the separate schedule button. +5. Close the schedule dialog without submitting. +6. Tap the main send button again. +7. Confirm the message sends immediately and the composer is not stuck. +8. Re-open the schedule dialog, choose a future time, and submit it. +9. Confirm the primary button reflects scheduled-send state and no immediate send occurs. +10. In a context where delayed events are unavailable, confirm there is no separate schedule button and normal sending still works. + +Expected results: + +- main send never depends on long-press +- schedule action is explicit and separate on mobile +- closing schedule UI does not break later sends + +## Issue #103: Jump buttons broken + +Environment: + +- a room with enough history that the target event is not already in the loaded viewport +- at least one reply link, bookmark, or permalink target + +Steps: + +1. Open a reply link or permalink to an older message. +2. Confirm the target message appears before any snap back to the latest timeline. +3. Scroll upward from the jumped position. +4. Scroll downward from the jumped position. +5. Repeat using a bookmark jump if available. +6. Background the app briefly, return to foreground, and confirm the same jumped context remains stable. + +Expected results: + +- jump lands on the requested target +- timeline does not immediately snap to bottom +- surrounding scroll does not jitter or reset unexpectedly +- foreground return does not discard the event-targeted context too early + +## Issue #109: PWA freeze instrumentation + +Environment: + +- Safari desktop PWA preferred +- Sentry and browser console available if possible + +Steps: + +1. Launch the PWA and leave it idle for an extended period. +2. Return and attempt basic interaction: + - switch rooms + - open a DM + - click the composer + - try a normal browser reload if the UI still responds +3. If the app freezes, note: + - whether the window still repaints + - whether clicks are ignored everywhere + - whether reload is possible +4. Capture any visible console output and the latest Sentry breadcrumbs/metrics around: + - app visibility changes + - pageshow restore + - service worker controller changes + - service worker claim requests + - background client startup or failure + - forced reload requests + +Expected results: + +- enough telemetry exists to classify the freeze as controller churn, restore failure, background-client deadlock, or sync/network stall diff --git a/src/app/components/editor/autocomplete/AutocompleteMenu.test.tsx b/src/app/components/editor/autocomplete/AutocompleteMenu.test.tsx new file mode 100644 index 000000000..cda6f8b1a --- /dev/null +++ b/src/app/components/editor/autocomplete/AutocompleteMenu.test.tsx @@ -0,0 +1,54 @@ +import type { ReactNode } from 'react'; +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import type { Editor } from 'slate'; +import { vi } from 'vitest'; + +import { AutocompleteMenu } from './AutocompleteMenu'; + +vi.mock('focus-trap-react', () => ({ + default: ({ children }: { children: ReactNode }) => children, +})); + +vi.mock('slate-react', () => ({ + ReactEditor: { + focus: vi.fn<() => void>(), + }, +})); + +describe('AutocompleteMenu', () => { + const editor = {} as Editor; + + it('marks the first item selected by default', () => { + render( + void>()} editor={editor}> + + + + ); + + expect(screen.getByRole('button', { name: 'First' })).toHaveAttribute('data-selected', 'true'); + expect(screen.getByRole('button', { name: 'Second' })).toHaveAttribute( + 'data-selected', + 'false' + ); + }); + + it('updates the selected item when autocomplete-navigate is dispatched', () => { + const { container } = render( + void>()} editor={editor}> + + + + + ); + + const menu = container.querySelector('[data-autocomplete-menu]'); + expect(menu).not.toBeNull(); + + menu!.dispatchEvent(new CustomEvent('autocomplete-navigate', { detail: { direction: 1 } })); + + expect(screen.getByRole('button', { name: 'Second' })).toHaveAttribute('data-selected', 'true'); + expect(screen.getByRole('button', { name: 'First' })).toHaveAttribute('data-selected', 'false'); + }); +}); diff --git a/src/app/components/editor/autocomplete/AutocompleteMenu.tsx b/src/app/components/editor/autocomplete/AutocompleteMenu.tsx index 72decd462..ef59bc14a 100644 --- a/src/app/components/editor/autocomplete/AutocompleteMenu.tsx +++ b/src/app/components/editor/autocomplete/AutocompleteMenu.tsx @@ -1,5 +1,5 @@ import type { ReactNode } from 'react'; -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import FocusTrap from 'focus-trap-react'; import { isKeyHotkey } from 'is-hotkey'; import { Header, Menu, Scroll, config } from 'folds'; @@ -25,6 +25,7 @@ export function AutocompleteMenu({ }: AutocompleteMenuProps) { const alive = useAlive(); const itemsRef = useRef(null); + const selectedIndexRef = useRef(0); const handleDeactivate = () => { if (alive()) { @@ -34,6 +35,73 @@ export function AutocompleteMenu({ }; const [isActive, setIsActive] = useState(true); useEffect(() => ReactEditor.focus(editor), [editor, isActive]); + + const applySelectedIndex = useCallback((nextIndex: number, focus = false) => { + const buttons = Array.from( + itemsRef.current?.querySelectorAll('button') ?? [] + ); + if (buttons.length === 0) { + selectedIndexRef.current = 0; + return; + } + + const clampedIndex = Math.max(0, Math.min(nextIndex, buttons.length - 1)); + selectedIndexRef.current = clampedIndex; + + buttons.forEach((button, index) => { + button.dataset.selected = String(index === clampedIndex); + }); + + if (focus) { + const selectedButton = buttons[clampedIndex]; + selectedButton?.focus({ preventScroll: true }); + selectedButton?.scrollIntoView?.({ block: 'nearest' }); + } + }, []); + + useEffect(() => { + applySelectedIndex(0); + }, [children, applySelectedIndex]); + + useEffect(() => { + const items = itemsRef.current; + const menuRoot = items?.closest('[data-autocomplete-menu]'); + if (!items || !menuRoot) return undefined; + + const handleNavigate = (event: Event) => { + const direction = (event as CustomEvent<{ direction?: number }>).detail?.direction ?? 0; + if (!Number.isFinite(direction) || direction === 0) return; + applySelectedIndex(selectedIndexRef.current + direction, true); + }; + + const handleFocusIn = (event: FocusEvent) => { + const target = event.target; + if (!(target instanceof HTMLButtonElement)) return; + const buttons = Array.from(items.querySelectorAll('button')); + const index = buttons.indexOf(target); + if (index >= 0) applySelectedIndex(index); + }; + + const handlePointerMove = (event: PointerEvent) => { + const target = event.target; + if (!(target instanceof Element)) return; + const button = target.closest('button'); + if (!button) return; + const buttons = Array.from(items.querySelectorAll('button')); + const index = buttons.indexOf(button); + if (index >= 0) applySelectedIndex(index); + }; + + menuRoot.addEventListener('autocomplete-navigate', handleNavigate as EventListener); + items.addEventListener('focusin', handleFocusIn); + items.addEventListener('pointermove', handlePointerMove); + return () => { + menuRoot.removeEventListener('autocomplete-navigate', handleNavigate as EventListener); + items.removeEventListener('focusin', handleFocusIn); + items.removeEventListener('pointermove', handlePointerMove); + }; + }, [applySelectedIndex]); + function handleInput(evt: KeyboardEvent) { if (!evt) return; if ( @@ -71,7 +139,11 @@ export function AutocompleteMenu({ {headerContent} -
+
{children}
diff --git a/src/app/features/room/RoomInput.tsx b/src/app/features/room/RoomInput.tsx index 7a863fa31..e13bb2bb9 100644 --- a/src/app/features/room/RoomInput.tsx +++ b/src/app/features/room/RoomInput.tsx @@ -373,7 +373,9 @@ export const RoomInput = forwardRef( ); const uploadBoardHandlers = useRef(); const longPressTimer = useRef | null>(null); - const isLongPress = useRef(false); + const suppressNextSendClick = useRef(false); + const longPressPointerId = useRef(null); + const longPressStartPoint = useRef<{ x: number; y: number } | null>(null); const imagePackRooms: Room[] = useImagePackRooms(roomId, roomToParents); @@ -471,12 +473,36 @@ export const RoomInput = forwardRef( const isEncrypted = room.hasEncryptionStateEvent(); const { triggerPreLift } = useKeyboardHeight(); + const isMobileLayout = mobileOrTablet(); // Always active on mobile: iOS can apply window.scrollY even with overflow:hidden // on body (scroll-prediction bug). The lock snaps scrollY back to 0 immediately // on any scroll event, preventing the "header scrolls up then snaps" jank. // useKeyboardHeight now manages --sable-visible-height synchronously in its own // event handler, so no useEffect here is needed for CSS variable management. - useScrollLock(mobileOrTablet()); + useScrollLock(isMobileLayout); + + const closeSchedulePicker = useCallback(() => { + suppressNextSendClick.current = false; + setShowSchedulePicker(false); + setScheduleMenuAnchor(undefined); + }, []); + + const clearLongPressTimer = useCallback(() => { + if (longPressTimer.current !== null) { + clearTimeout(longPressTimer.current); + longPressTimer.current = null; + } + longPressPointerId.current = null; + longPressStartPoint.current = null; + }, []); + + const openSchedulePicker = useCallback(() => { + setSendError(undefined); + setScheduleMenuAnchor(undefined); + setShowSchedulePicker(true); + }, []); + + useEffect(() => clearLongPressTimer, [clearLongPressTimer]); useElementSizeObserver( useCallback(() => fileDropContainerRef.current, [fileDropContainerRef]), @@ -1520,7 +1546,7 @@ export const RoomInput = forwardRef( return ( // eslint-disable-next-line jsx-a11y/no-static-element-interactions -
+
{selectedFiles.length > 0 && ( ( size="300" radii="300" onClick={() => { - setScheduleMenuAnchor(undefined); - setShowSchedulePicker(true); + openSchedulePicker(); }} before={menuIcon(Clock)} > @@ -2050,42 +2075,57 @@ export const RoomInput = forwardRef( title="Send Message" aria-label="Send your composed Message" onClick={() => { - if (isLongPress.current) { - isLongPress.current = false; + if (suppressNextSendClick.current) { + suppressNextSendClick.current = false; return; } submit(); }} onMouseDown={(e: MouseEvent) => e.preventDefault()} - onPointerDown={() => { - isLongPress.current = false; - if (mobileOrTablet() && delayedEventsSupported) { - longPressTimer.current = setTimeout(() => { - isLongPress.current = true; - setShowSchedulePicker(true); - }, 1000); + onPointerDown={(evt) => { + clearLongPressTimer(); + if (!isMobileLayout || !delayedEventsSupported || evt.pointerType === 'mouse') { + return; } + + longPressPointerId.current = evt.pointerId; + longPressStartPoint.current = { x: evt.clientX, y: evt.clientY }; + longPressTimer.current = setTimeout(() => { + if (longPressPointerId.current !== evt.pointerId) return; + suppressNextSendClick.current = true; + clearLongPressTimer(); + openSchedulePicker(); + }, 550); }} - onPointerUp={() => { - if (longPressTimer.current !== null) { - clearTimeout(longPressTimer.current); - longPressTimer.current = null; + onPointerMove={(evt) => { + if (longPressPointerId.current !== evt.pointerId) return; + const start = longPressStartPoint.current; + if (!start) return; + const movedX = Math.abs(evt.clientX - start.x); + const movedY = Math.abs(evt.clientY - start.y); + if (movedX > 12 || movedY > 12) { + clearLongPressTimer(); } }} + onPointerUp={() => { + clearLongPressTimer(); + }} onPointerCancel={() => { - if (longPressTimer.current !== null) { - clearTimeout(longPressTimer.current); - longPressTimer.current = null; - } + clearLongPressTimer(); + }} + onPointerLeave={() => { + clearLongPressTimer(); }} variant={scheduledTime ? 'Primary' : 'SurfaceVariant'} size="300" radii="0" - className={delayedEventsSupported ? css.SplitSendButton : undefined} + className={ + delayedEventsSupported && !isMobileLayout ? css.SplitSendButton : undefined + } > {scheduledTime ? composerIcon(Clock) : composerIcon(PaperPlaneTilt)} - {delayedEventsSupported && !mobileOrTablet() && ( + {delayedEventsSupported && !isMobileLayout && ( ) => { setScheduleMenuAnchor(evt.currentTarget.getBoundingClientRect()); @@ -2109,10 +2149,10 @@ export const RoomInput = forwardRef( setShowSchedulePicker(false)} + onCancel={closeSchedulePicker} onSubmit={(date) => { setScheduledTime(date); - setShowSchedulePicker(false); + closeSchedulePicker(); setSendError(undefined); }} /> diff --git a/src/app/hooks/timeline/useTimelineSync.test.tsx b/src/app/hooks/timeline/useTimelineSync.test.tsx index 0cf0c1057..b5d1a057c 100644 --- a/src/app/hooks/timeline/useTimelineSync.test.tsx +++ b/src/app/hooks/timeline/useTimelineSync.test.tsx @@ -3,6 +3,7 @@ import { act, renderHook } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; import type { Room } from '$types/matrix-sdk'; import { ClientEvent, RoomEvent } from '$types/matrix-sdk'; +import { appEvents } from '$utils/appEvents'; import { useTimelineSync } from './useTimelineSync'; vi.mock('@sentry/react', () => ({ @@ -47,6 +48,7 @@ function createMx() { const mxEmitter = new EventEmitter(); return { getUserId: () => '@alice:test', + paginateEventTimeline: vi.fn<() => Promise>().mockResolvedValue(false), on: mxEmitter.on.bind(mxEmitter), off: mxEmitter.off.bind(mxEmitter), emit: mxEmitter.emit.bind(mxEmitter), @@ -241,6 +243,177 @@ describe('useTimelineSync', () => { expect(scrollToBottom).not.toHaveBeenCalled(); }); + it('preserves a loaded event context when ClientEvent.Room fires', async () => { + const { room } = createRoom('!room:test', [{ getId: () => '$live:event' }]); + const targetEvent = { getId: () => '$target:event' }; + const contextTimeline = createTimeline([targetEvent]); + const mx = { + ...createMx(), + getEventTimeline: vi.fn<() => Promise>().mockResolvedValue(contextTimeline), + }; + + const { result } = renderHook(() => + useTimelineSync({ + room: room as Room, + mx: mx as never, + eventId: '$target:event', + isAtBottom: false, + isAtBottomRef: { current: false }, + scrollToBottom: vi.fn<() => void>(), + unreadInfo: undefined, + setUnreadInfo: vi.fn<() => void>(), + hideReadsRef: { current: false }, + readUptoEventIdRef: { current: undefined }, + }) + ); + + await act(async () => { + await result.current.loadEventTimeline('$target:event'); + }); + + await act(async () => { + mx.emit(ClientEvent.Room, room); + await Promise.resolve(); + }); + + expect(result.current.timeline.linkedTimelines).toEqual([contextTimeline]); + }); + + it('does not replace an in-flight event context with the live timeline on ClientEvent.Room', async () => { + const { room } = createRoom('!room:test', [{ getId: () => '$live:event' }]); + let resolveTimeline: ((timeline: FakeTimeline) => void) | undefined; + const pendingTimeline = new Promise((resolve) => { + resolveTimeline = resolve; + }); + const targetEvent = { getId: () => '$target:event' }; + const contextTimeline = createTimeline([targetEvent]); + const mx = { + ...createMx(), + getEventTimeline: vi.fn<() => Promise>().mockReturnValue(pendingTimeline), + }; + + const { result } = renderHook(() => + useTimelineSync({ + room: room as Room, + mx: mx as never, + eventId: '$target:event', + isAtBottom: false, + isAtBottomRef: { current: false }, + scrollToBottom: vi.fn<() => void>(), + unreadInfo: undefined, + setUnreadInfo: vi.fn<() => void>(), + hideReadsRef: { current: false }, + readUptoEventIdRef: { current: undefined }, + }) + ); + + expect(result.current.timeline.linkedTimelines).toEqual([]); + + const loadPromise = result.current.loadEventTimeline('$target:event'); + + await act(async () => { + mx.emit(ClientEvent.Room, room); + await Promise.resolve(); + }); + + expect(result.current.timeline.linkedTimelines).toEqual([]); + + await act(async () => { + resolveTimeline?.(contextTimeline); + await loadPromise; + }); + + expect(result.current.timeline.linkedTimelines).toEqual([contextTimeline]); + }); + + it('marks jump context pending immediately when eventId changes', async () => { + const { room } = createRoom('!room:test', [{ getId: () => '$live:event' }]); + let resolveTimeline: ((timeline: FakeTimeline) => void) | undefined; + const pendingTimeline = new Promise((resolve) => { + resolveTimeline = resolve; + }); + const targetEvent = { getId: () => '$target:event' }; + const contextTimeline = createTimeline([targetEvent]); + const mx = { + ...createMx(), + getEventTimeline: vi.fn<() => Promise>().mockReturnValue(pendingTimeline), + }; + + const { result, rerender } = renderHook( + ({ eventId }: { eventId: string | undefined }) => + useTimelineSync({ + room: room as Room, + mx: mx as never, + eventId, + isAtBottom: false, + isAtBottomRef: { current: false }, + scrollToBottom: vi.fn<() => void>(), + unreadInfo: undefined, + setUnreadInfo: vi.fn<() => void>(), + hideReadsRef: { current: false }, + readUptoEventIdRef: { current: undefined }, + }), + { initialProps: { eventId: undefined as string | undefined } } + ); + + expect(result.current.timeline.linkedTimelines).toHaveLength(1); + + rerender({ eventId: '$target:event' }); + const loadPromise = result.current.loadEventTimeline('$target:event'); + + await act(async () => { + mx.emit(ClientEvent.Room, room); + appEvents.emitVisibilityChange(true); + await Promise.resolve(); + }); + + expect(result.current.timeline.linkedTimelines).toHaveLength(1); + expect(result.current.timeline.linkedTimelines[0]).not.toBe(contextTimeline); + + await act(async () => { + resolveTimeline?.(contextTimeline); + await loadPromise; + }); + + expect(result.current.timeline.linkedTimelines).toEqual([contextTimeline]); + }); + + it('preserves a loaded event context when the app returns to foreground', async () => { + const { room } = createRoom('!room:test', [{ getId: () => '$live:event' }]); + const targetEvent = { getId: () => '$target:event' }; + const contextTimeline = createTimeline([targetEvent]); + const mx = { + ...createMx(), + getEventTimeline: vi.fn<() => Promise>().mockResolvedValue(contextTimeline), + }; + + const { result } = renderHook(() => + useTimelineSync({ + room: room as Room, + mx: mx as never, + eventId: '$target:event', + isAtBottom: false, + isAtBottomRef: { current: false }, + scrollToBottom: vi.fn<() => void>(), + unreadInfo: undefined, + setUnreadInfo: vi.fn<() => void>(), + hideReadsRef: { current: false }, + readUptoEventIdRef: { current: undefined }, + }) + ); + + await act(async () => { + await result.current.loadEventTimeline('$target:event'); + }); + + await act(async () => { + appEvents.emitVisibilityChange(true); + await Promise.resolve(); + }); + + expect(result.current.timeline.linkedTimelines).toEqual([contextTimeline]); + }); + it('does not snap a non-bottom user to latest after TimelineReset', async () => { const { room, timelineSet, events } = createRoom(); const scrollToBottom = vi.fn<() => void>(); diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index d1d5b7f65..1516b7033 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -433,6 +433,9 @@ export function useTimelineSync({ readUptoEventIdRef, }: UseTimelineSyncOptions) { const alive = useAlive(); + const eventContextActiveRef = useRef(false); + const eventContextPendingRef = useRef(Boolean(eventId)); + const eventContextEventIdRef = useRef(eventId); const [timeline, setTimeline] = useState(() => eventId ? getEmptyTimeline() : { linkedTimelines: getInitialTimeline(room).linkedTimelines } @@ -452,6 +455,11 @@ export function useTimelineSync({ const eventsLength = getTimelinesEventsCount(timeline.linkedTimelines); const liveTimelineLinked = timeline.linkedTimelines.at(-1) === getLiveTimeline(room); + const eventTargetChangedAtRender = eventContextEventIdRef.current !== eventId; + const preservingEventContext = + Boolean(eventId) && !eventTargetChangedAtRender && eventContextActiveRef.current; + const waitingForEventContext = + Boolean(eventId) && (eventTargetChangedAtRender || eventContextPendingRef.current); const canPaginateBack = typeof timeline.linkedTimelines[0]?.getPaginationToken(Direction.Backward) === 'string'; @@ -514,6 +522,37 @@ export function useTimelineSync({ const timelineRef = useRef(timeline); timelineRef.current = timeline; + useEffect(() => { + const eventTargetChanged = eventContextEventIdRef.current !== eventId; + eventContextEventIdRef.current = eventId; + + if (!eventId) { + eventContextActiveRef.current = false; + eventContextPendingRef.current = false; + return; + } + + if (eventTargetChanged) { + eventContextActiveRef.current = false; + eventContextPendingRef.current = true; + return; + } + + if (timeline.linkedTimelines.length === 0) { + eventContextPendingRef.current = true; + return; + } + + if (!liveTimelineLinked) { + eventContextActiveRef.current = true; + eventContextPendingRef.current = false; + return; + } + + eventContextActiveRef.current = false; + eventContextPendingRef.current = false; + }, [eventId, timeline.linkedTimelines.length, liveTimelineLinked]); + const loadEventTimeline = useEventTimelineLoader( mx, room, @@ -521,6 +560,8 @@ export function useTimelineSync({ (evtId, lTimelines, evtAbsIndex) => { if (!alive()) return; + eventContextPendingRef.current = false; + eventContextActiveRef.current = true; setTimeline({ linkedTimelines: lTimelines }); setFocusItem({ @@ -534,6 +575,8 @@ export function useTimelineSync({ ), useCallback(() => { if (!alive()) return; + eventContextPendingRef.current = false; + eventContextActiveRef.current = false; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); scrollToBottom('instant'); }, [alive, room, scrollToBottom]), @@ -654,6 +697,8 @@ export function useTimelineSync({ // chain still references the old detached timeline. When auto-scroll recovery // is pending for a bottom-pinned user, the guard is meaningless lag. if ( + preservingEventContext || + waitingForEventContext || !(isAtBottom || resetAutoScrollPending) || (!liveTimelineLinked && !resetAutoScrollPending) || eventsLength === 0 @@ -664,7 +709,14 @@ export function useTimelineSync({ lastScrolledAtEventsLengthRef.current = eventsLength; scrollToBottom('instant'); - }, [isAtBottom, liveTimelineLinked, eventsLength, scrollToBottom]); + }, [ + preservingEventContext, + waitingForEventContext, + isAtBottom, + liveTimelineLinked, + eventsLength, + scrollToBottom, + ]); useEffect(() => { if (eventId) return; @@ -693,7 +745,9 @@ export function useTimelineSync({ if (eventRoom.roomId !== room.roomId) return; // Don't update to live timeline when waiting for eventId context to load. // The eventId-specific loading path will handle setting the correct timeline. - if (eventId) return; + if (eventId) { + if (waitingForEventContext || preservingEventContext) return; + } // Only update if the live timeline actually has events now — prevents // spurious updates that would reset scroll position during normal sync. const liveEvents = getLiveTimeline(room).getEvents(); @@ -724,7 +778,15 @@ export function useTimelineSync({ return () => { mx.off(ClientEvent.Room, handleRoomInitialized); }; - }, [mx, room, eventId, timeline.linkedTimelines, eventsLengthRef]); + }, [ + mx, + room, + eventId, + waitingForEventContext, + preservingEventContext, + timeline.linkedTimelines, + eventsLengthRef, + ]); const prevRoomIdRef = useRef(room.roomId); const eventIdRef = useRef(eventId); @@ -748,6 +810,9 @@ export function useTimelineSync({ useEffect(() => { const handleVisibilityChange = (isVisible: boolean) => { if (!isVisible) return; // Only act on foreground events + if (eventId) { + if (waitingForEventContext || preservingEventContext) return; + } // Check if SDK has events but React timeline state is empty or stale const liveTimeline = getLiveTimeline(room); @@ -781,7 +846,14 @@ export function useTimelineSync({ const unsubscribe = appEvents.onVisibilityChange(handleVisibilityChange); return unsubscribe; - }, [room, timeline.linkedTimelines, eventsLengthRef]); + }, [ + room, + eventId, + waitingForEventContext, + preservingEventContext, + timeline.linkedTimelines, + eventsLengthRef, + ]); return { timeline, diff --git a/src/app/hooks/useAppVisibility.ts b/src/app/hooks/useAppVisibility.ts index b144b0e80..ebabb2531 100644 --- a/src/app/hooks/useAppVisibility.ts +++ b/src/app/hooks/useAppVisibility.ts @@ -19,6 +19,46 @@ const DEFAULT_HEARTBEAT_INTERVAL_MS = 10 * 60 * 1000; type SessionSyncReason = 'heartbeat'; +const requestServiceWorkerClaim = (reason: 'visible_foreground' | 'pageshow_restore') => { + if (!('serviceWorker' in navigator)) return; + if (navigator.serviceWorker.controller) return; + + Sentry.addBreadcrumb({ + category: 'service_worker.claim', + message: 'Requested service worker client claim', + level: 'warning', + data: { + reason, + visibilityState: document.visibilityState, + online: navigator.onLine, + }, + }); + Sentry.metrics.count('sable.sw.claim_requested', 1, { + attributes: { + reason, + visibility_state: document.visibilityState, + online: navigator.onLine, + }, + }); + + navigator.serviceWorker.ready + .then((registration) => { + const activeWorker = registration.active; + if (!activeWorker) return; + if (activeWorker.state !== 'activated') return; + // oxlint-disable-next-line unicorn/require-post-message-target-origin + activeWorker.postMessage({ type: 'CLAIM_CLIENTS' }); + }) + .catch((error) => { + Sentry.addBreadcrumb({ + category: 'service_worker.claim', + message: 'Service worker claim request failed', + level: 'warning', + data: { reason, error: error instanceof Error ? error.message : String(error) }, + }); + }); +}; + export function useAppVisibility(mx: MatrixClient | undefined, activeSession?: Session) { const clientConfig = useClientConfig(); const [usePushNotifications] = useSetting(settingsAtom, 'usePushNotifications'); @@ -140,6 +180,9 @@ export function useAppVisibility(mx: MatrixClient | undefined, activeSession?: S } ); appEvents.emitVisibilityChange(isVisible); + if (isVisible) { + requestServiceWorkerClaim('visible_foreground'); + } if (!isVisible) { appEvents.emitVisibilityHidden(); } @@ -227,7 +270,27 @@ export function useAppVisibility(mx: MatrixClient | undefined, activeSession?: S }; const handlePageShow = (ev: PageTransitionEvent) => { - if (ev.persisted) emitVisible(); + if (ev.persisted) { + Sentry.addBreadcrumb({ + category: 'app.visibility', + message: 'App restored from pageshow', + level: 'info', + data: { + persisted: ev.persisted, + visibilityState: document.visibilityState, + online: navigator.onLine, + }, + }); + Sentry.metrics.count('sable.app.pageshow', 1, { + attributes: { + persisted: ev.persisted, + visibility_state: document.visibilityState, + online: navigator.onLine, + }, + }); + requestServiceWorkerClaim('pageshow_restore'); + emitVisible(); + } }; const timeoutId = window.setTimeout(emitVisible, 100); diff --git a/src/app/utils/keyboard.test.ts b/src/app/utils/keyboard.test.ts new file mode 100644 index 000000000..cdb34a374 --- /dev/null +++ b/src/app/utils/keyboard.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from 'vitest'; +import { onTabPress } from './keyboard'; + +describe('onTabPress', () => { + it('does not trigger when the tab event was already handled', () => { + const callback = vi.fn<() => void>(); + const preventDefault = vi.fn<() => void>(); + + onTabPress( + { + key: 'Tab', + which: 9, + altKey: false, + ctrlKey: false, + defaultPrevented: true, + metaKey: false, + shiftKey: false, + preventDefault, + }, + callback + ); + + expect(callback).not.toHaveBeenCalled(); + expect(preventDefault).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/utils/keyboard.ts b/src/app/utils/keyboard.ts index 37ab9a3f8..3b9137b74 100644 --- a/src/app/utils/keyboard.ts +++ b/src/app/utils/keyboard.ts @@ -6,12 +6,14 @@ export interface KeyboardEventLike { which: number; altKey: boolean; ctrlKey: boolean; + defaultPrevented?: boolean; metaKey: boolean; shiftKey: boolean; preventDefault(): void; } export const onTabPress = (evt: KeyboardEventLike, callback: () => void) => { + if (evt.defaultPrevented) return; if (isKeyHotkey('tab', evt)) { evt.preventDefault(); callback(); diff --git a/src/serviceWorkerBootstrap.ts b/src/serviceWorkerBootstrap.ts index 017d73f34..26996adf3 100644 --- a/src/serviceWorkerBootstrap.ts +++ b/src/serviceWorkerBootstrap.ts @@ -10,6 +10,23 @@ import { pushSessionToSW } from './sw-session'; const log = createLogger('service-worker-bootstrap'); const DONT_SHOW_PROMPT_KEY = 'cinny_dont_show_sw_update_prompt'; +const recordForcedReload = (reason: string, data?: Record) => { + Sentry.addBreadcrumb({ + category: 'app.reload', + message: 'Forced reload requested', + level: 'warning', + data: { + reason, + visibilityState: typeof document !== 'undefined' ? document.visibilityState : 'unknown', + online: typeof navigator !== 'undefined' ? navigator.onLine : undefined, + ...data, + }, + }); + Sentry.metrics.count('sable.app.reload_requested', 1, { + attributes: { reason }, + }); +}; + const showUpdateAvailablePrompt = (registration: ServiceWorkerRegistration) => { const userPreference = localStorage.getItem(DONT_SHOW_PROMPT_KEY); @@ -20,8 +37,11 @@ const showUpdateAvailablePrompt = (registration: ServiceWorkerRegistration) => { // eslint-disable-next-line no-alert if (window.confirm('A new version of the app is available. Refresh to update?')) { if (registration.waiting) { + recordForcedReload('sw_update_prompt_waiting', { hasWaitingWorker: true }); // oxlint-disable-next-line unicorn/require-post-message-target-origin registration.waiting.postMessage({ type: 'SKIP_WAITING_AND_CLAIM' }); + } else { + recordForcedReload('sw_update_prompt_reload', { hasWaitingWorker: false }); } window.location.reload(); } @@ -137,7 +157,18 @@ export function registerAppServiceWorker() { category: 'service_worker', message: 'Service worker controller changed', level: 'warning', - data: { visibilityState: document.visibilityState }, + data: { + visibilityState: document.visibilityState, + online: navigator.onLine, + hasController: !!navigator.serviceWorker.controller, + }, + }); + Sentry.metrics.count('sable.sw.controller_change', 1, { + attributes: { + visibility_state: document.visibilityState, + online: navigator.onLine, + has_controller: !!navigator.serviceWorker.controller, + }, }); }); diff --git a/src/sw.ts b/src/sw.ts index 545ca0ac8..ebcd50e27 100644 --- a/src/sw.ts +++ b/src/sw.ts @@ -51,6 +51,7 @@ type PushTelemetryEvent = | 'received' | 'confirmed_visible' | 'suppressed_visible' + | 'claim_clients' | 'stale_focus_ignored' | 'shown_os' | 'decrypt_timeout' @@ -1178,6 +1179,10 @@ self.addEventListener('message', (event: ExtendableMessageEvent) => { // sessions Map. Fire-and-forget: responses come via setSession messages. const claimedClients = await self.clients.matchAll({ type: 'window' }); claimedClients.forEach((c) => c.postMessage({ type: 'requestSession' })); + await recordPushTelemetry('claim_clients', { + client_count: claimedClients.length, + duration_ms: Math.round(performance.now() - claimStartedAt), + }); await postSentryBreadcrumb('service_worker', 'Service worker claimed clients', 'warning', { claimedClientCount: claimedClients.length, durationMs: Math.round(performance.now() - claimStartedAt),