fix(page-mode): harden scrolling and editable focus
- commit
- 5b1cfa8075d1e9febaa21ab51030eb01ab4f78b1
- author
- brice · · 2026-07-28 07:50 UTC
- parent
- 0c4b1f1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
21 files · +913 -199
M README.md +7 -7
| @@ -169,13 +169,13 @@ type = "webkit" | |||
| 169 | 169 | ||
| 170 | 170 | ### Page scroll execution | |
| 171 | 171 | ||
| 172 | - | Page Mode scroll commands are routed through `port.PageScrollable`, and each | |
| 173 | - | engine owns its execution strategy: | |
| 174 | - | - **CEF** sends synthetic native key events (arrow keys, Page Up/Down) through | |
| 175 | - | the Chromium browser host for the best compatibility with SPA scroll containers. | |
| 176 | - | - **WebKit** falls back to JavaScript scroll delta injection using the shared | |
| 177 | - | `BuildScrollByJS` helper, which coalesces rapid held-key repeats via | |
| 178 | - | `requestAnimationFrame` and resolves the nearest scrollable ancestor. | |
| 172 | + | Page Mode scroll commands are routed through `port.PageScrollable`. CEF and | |
| 173 | + | WebKit use the shared `BuildScrollByJS` target resolver when the page is ready: | |
| 174 | + | it starts under the viewport center, walks through ancestors that can move in | |
| 175 | + | the requested direction, and hands scrolling to the document when a nested | |
| 176 | + | container reaches its boundary. The application repeater owns held-key cadence; | |
| 177 | + | each engine executes one immediate scroll step per tick. Cross-origin frame | |
| 178 | + | contents remain best-effort. | |
| 179 | 179 | ||
| 180 | 180 | ### Rendering notes | |
| 181 | 181 | ||
| @@ -169,13 +169,13 @@ type = "webkit" | |||||
| 169 | 169 | ||||
| 170 | ### Page scroll execution | 170 | ### Page scroll execution | ||
| 171 | 171 | ||||
| 172 | - | Page Mode scroll commands are routed through `port.PageScrollable`, and each | 172 | + | Page Mode scroll commands are routed through `port.PageScrollable`. CEF and |
| 173 | - | engine owns its execution strategy: | 173 | + | WebKit use the shared `BuildScrollByJS` target resolver when the page is ready: |
| 174 | - | - **CEF** sends synthetic native key events (arrow keys, Page Up/Down) through | 174 | + | it starts under the viewport center, walks through ancestors that can move in |
| 175 | - | the Chromium browser host for the best compatibility with SPA scroll containers. | 175 | + | the requested direction, and hands scrolling to the document when a nested |
| 176 | - | - **WebKit** falls back to JavaScript scroll delta injection using the shared | 176 | + | container reaches its boundary. The application repeater owns held-key cadence; |
| 177 | - | `BuildScrollByJS` helper, which coalesces rapid held-key repeats via | 177 | + | each engine executes one immediate scroll step per tick. Cross-origin frame |
| 178 | - | `requestAnimationFrame` and resolves the nearest scrollable ancestor. | 178 | + | contents remain best-effort. |
| 179 | 179 | ||||
| 180 | ### Rendering notes | 180 | ### Rendering notes | ||
| 181 | 181 | ||||
M docs/reference/keybindings.md +1 -3
| @@ -57,9 +57,7 @@ Keybinding tables use uppercase letters as visual labels for unshifted letter ke | |||
| 57 | 57 | ||
| 58 | 58 | Page Mode is an explicit page-scrolling mode for the active pane only. It shows a local `PAGE` indicator on the owning pane, uses `workspace.styling.pane_mode_color` for the local accent, and exits automatically when focus moves into the omnibox, find bar, overlays, or an editable element inside the page. The default `timeout_ms` is `0`, so Page Mode does not auto-time out unless you configure one. Arrow keys continue to flow through the browser engine's native page-navigation path while Page Mode is active, while other app-level shortcuts stay suspended until you leave the mode. | |
| 59 | 59 | ||
| 60 | - | Scroll execution depends on the active browser engine: | |
| 61 | - | - **CEF**: Page Mode scroll commands (`h/j/k/l`, `Shift+J/K`) are translated to native Chromium key events (arrow keys, Page Up/Down) for reliable behaviour with SPA scroll containers. | |
| 62 | - | - **WebKit**: Falls back to JavaScript-driven scroll delta injection handled by `BuildScrollByJS`, which resolves the nearest scrollable ancestor and coalesces held-key repeats via `requestAnimationFrame`. | |
| 60 | + | CEF and WebKit execute Page Mode scroll commands (`h/j/k/l`, `Shift+J/K`) with the shared `BuildScrollByJS` resolver. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. Cross-origin frame contents remain best-effort. | |
| 63 | 61 | ||
| 64 | 62 | | Action | Keys | | |
| 65 | 63 | |--------|------| | |
| @@ -57,9 +57,7 @@ Keybinding tables use uppercase letters as visual labels for unshifted letter ke | |||||
| 57 | 57 | ||||
| 58 | Page Mode is an explicit page-scrolling mode for the active pane only. It shows a local `PAGE` indicator on the owning pane, uses `workspace.styling.pane_mode_color` for the local accent, and exits automatically when focus moves into the omnibox, find bar, overlays, or an editable element inside the page. The default `timeout_ms` is `0`, so Page Mode does not auto-time out unless you configure one. Arrow keys continue to flow through the browser engine's native page-navigation path while Page Mode is active, while other app-level shortcuts stay suspended until you leave the mode. | 58 | Page Mode is an explicit page-scrolling mode for the active pane only. It shows a local `PAGE` indicator on the owning pane, uses `workspace.styling.pane_mode_color` for the local accent, and exits automatically when focus moves into the omnibox, find bar, overlays, or an editable element inside the page. The default `timeout_ms` is `0`, so Page Mode does not auto-time out unless you configure one. Arrow keys continue to flow through the browser engine's native page-navigation path while Page Mode is active, while other app-level shortcuts stay suspended until you leave the mode. | ||
| 59 | 59 | ||||
| 60 | - | Scroll execution depends on the active browser engine: | 60 | + | CEF and WebKit execute Page Mode scroll commands (`h/j/k/l`, `Shift+J/K`) with the shared `BuildScrollByJS` resolver. Each step starts under the viewport center, walks up through ancestors that can move in the requested direction, and hands scrolling to the document when a nested container reaches its boundary. Cross-origin frame contents remain best-effort. |
| 61 | - | - **CEF**: Page Mode scroll commands (`h/j/k/l`, `Shift+J/K`) are translated to native Chromium key events (arrow keys, Page Up/Down) for reliable behaviour with SPA scroll containers. | |||
| 62 | - | - **WebKit**: Falls back to JavaScript-driven scroll delta injection handled by `BuildScrollByJS`, which resolves the nearest scrollable ancestor and coalesces held-key repeats via `requestAnimationFrame`. | |||
| 63 | 61 | ||||
| 64 | | Action | Keys | | 62 | | Action | Keys | | ||
| 65 | |--------|------| | 63 | |--------|------| | ||
M internal/application/port/webview.go +10 -0
| @@ -415,6 +415,9 @@ type PageScrollRequest struct { | |||
| 415 | 415 | Command PageScrollCommand | |
| 416 | 416 | FallbackDX int | |
| 417 | 417 | FallbackDY int | |
| 418 | + | // Continuous distinguishes autonomous held-key steps from the guaranteed | |
| 419 | + | // tap step so adapters can cancel queued held work without dropping taps. | |
| 420 | + | Continuous bool | |
| 418 | 421 | } | |
| 419 | 422 | ||
| 420 | 423 | // PageScrollable is an optional capability for WebViews that support semantic | |
| @@ -427,3 +430,10 @@ type PageScrollRequest struct { | |||
| 427 | 430 | type PageScrollable interface { | |
| 428 | 431 | ScrollPage(ctx context.Context, request PageScrollRequest) error | |
| 429 | 432 | } | |
| 433 | + | ||
| 434 | + | // PageScrollCanceler is an optional capability for adapters that queue held | |
| 435 | + | // page-scroll work. Implementations discard unconsumed continuous deltas while | |
| 436 | + | // preserving any guaranteed tap already accepted. | |
| 437 | + | type PageScrollCanceler interface { | |
| 438 | + | CancelPageScroll(ctx context.Context) | |
| 439 | + | } | |
| @@ -415,6 +415,9 @@ type PageScrollRequest struct { | |||||
| 415 | Command PageScrollCommand | 415 | Command PageScrollCommand | ||
| 416 | FallbackDX int | 416 | FallbackDX int | ||
| 417 | FallbackDY int | 417 | FallbackDY int | ||
| 418 | + | // Continuous distinguishes autonomous held-key steps from the guaranteed | |||
| 419 | + | // tap step so adapters can cancel queued held work without dropping taps. | |||
| 420 | + | Continuous bool | |||
| 418 | } | 421 | } | ||
| 419 | 422 | ||||
| 420 | // PageScrollable is an optional capability for WebViews that support semantic | 423 | // PageScrollable is an optional capability for WebViews that support semantic | ||
| @@ -427,3 +430,10 @@ type PageScrollRequest struct { | |||||
| 427 | type PageScrollable interface { | 430 | type PageScrollable interface { | ||
| 428 | ScrollPage(ctx context.Context, request PageScrollRequest) error | 431 | ScrollPage(ctx context.Context, request PageScrollRequest) error | ||
| 429 | } | 432 | } | ||
| 433 | + | ||||
| 434 | + | // PageScrollCanceler is an optional capability for adapters that queue held | |||
| 435 | + | // page-scroll work. Implementations discard unconsumed continuous deltas while | |||
| 436 | + | // preserving any guaranteed tap already accepted. | |||
| 437 | + | type PageScrollCanceler interface { | |||
| 438 | + | CancelPageScroll(ctx context.Context) | |||
| 439 | + | } | |||
M internal/application/usecase/page_scroll.go +25 -1
| @@ -44,7 +44,30 @@ func NewPageScrollUseCase() *PageScrollUseCase { | |||
| 44 | 44 | // Scroll applies a semantic scroll command to a WebView. | |
| 45 | 45 | // It returns an error if the WebView does not support semantic page scrolling | |
| 46 | 46 | // (i.e., does not implement port.PageScrollable). | |
| 47 | - | func (*PageScrollUseCase) Scroll(ctx context.Context, wv port.WebView, cmd PageScrollCommand) error { | |
| 47 | + | func (uc *PageScrollUseCase) Scroll(ctx context.Context, wv port.WebView, cmd PageScrollCommand) error { | |
| 48 | + | return uc.scroll(ctx, wv, cmd, false) | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | // ScrollContinuous applies one autonomous held-key step. The pixel policy is | |
| 52 | + | // identical to Scroll; only lifecycle metadata differs for adapter coalescing. | |
| 53 | + | func (uc *PageScrollUseCase) ScrollContinuous(ctx context.Context, wv port.WebView, cmd PageScrollCommand) error { | |
| 54 | + | return uc.scroll(ctx, wv, cmd, true) | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | // Stop cancels queued continuous work when the adapter supports cancellation. | |
| 58 | + | // Adapters such as WebKit that execute each step immediately need no special | |
| 59 | + | // capability and therefore treat this as a no-op. | |
| 60 | + | func (*PageScrollUseCase) Stop(ctx context.Context, wv port.WebView) error { | |
| 61 | + | if wv == nil { | |
| 62 | + | return errors.New("page scroll: nil webview") | |
| 63 | + | } | |
| 64 | + | if canceler, ok := wv.(port.PageScrollCanceler); ok { | |
| 65 | + | canceler.CancelPageScroll(ctx) | |
| 66 | + | } | |
| 67 | + | return nil | |
| 68 | + | } | |
| 69 | + | ||
| 70 | + | func (*PageScrollUseCase) scroll(ctx context.Context, wv port.WebView, cmd PageScrollCommand, continuous bool) error { | |
| 48 | 71 | if wv == nil { | |
| 49 | 72 | return errors.New("page scroll: nil webview") | |
| 50 | 73 | } | |
| @@ -59,6 +82,7 @@ func (*PageScrollUseCase) Scroll(ctx context.Context, wv port.WebView, cmd PageS | |||
| 59 | 82 | Command: toPortPageScrollCommand(cmd), | |
| 60 | 83 | FallbackDX: dx, | |
| 61 | 84 | FallbackDY: dy, | |
| 85 | + | Continuous: continuous, | |
| 62 | 86 | } | |
| 63 | 87 | if err := scroller.ScrollPage(ctx, req); err != nil { | |
| 64 | 88 | return fmt.Errorf("page scroll: %w", err) | |
| @@ -44,7 +44,30 @@ func NewPageScrollUseCase() *PageScrollUseCase { | |||||
| 44 | // Scroll applies a semantic scroll command to a WebView. | 44 | // Scroll applies a semantic scroll command to a WebView. | ||
| 45 | // It returns an error if the WebView does not support semantic page scrolling | 45 | // It returns an error if the WebView does not support semantic page scrolling | ||
| 46 | // (i.e., does not implement port.PageScrollable). | 46 | // (i.e., does not implement port.PageScrollable). | ||
| 47 | - | func (*PageScrollUseCase) Scroll(ctx context.Context, wv port.WebView, cmd PageScrollCommand) error { | 47 | + | func (uc *PageScrollUseCase) Scroll(ctx context.Context, wv port.WebView, cmd PageScrollCommand) error { |
| 48 | + | return uc.scroll(ctx, wv, cmd, false) | |||
| 49 | + | } | |||
| 50 | + | ||||
| 51 | + | // ScrollContinuous applies one autonomous held-key step. The pixel policy is | |||
| 52 | + | // identical to Scroll; only lifecycle metadata differs for adapter coalescing. | |||
| 53 | + | func (uc *PageScrollUseCase) ScrollContinuous(ctx context.Context, wv port.WebView, cmd PageScrollCommand) error { | |||
| 54 | + | return uc.scroll(ctx, wv, cmd, true) | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | // Stop cancels queued continuous work when the adapter supports cancellation. | |||
| 58 | + | // Adapters such as WebKit that execute each step immediately need no special | |||
| 59 | + | // capability and therefore treat this as a no-op. | |||
| 60 | + | func (*PageScrollUseCase) Stop(ctx context.Context, wv port.WebView) error { | |||
| 61 | + | if wv == nil { | |||
| 62 | + | return errors.New("page scroll: nil webview") | |||
| 63 | + | } | |||
| 64 | + | if canceler, ok := wv.(port.PageScrollCanceler); ok { | |||
| 65 | + | canceler.CancelPageScroll(ctx) | |||
| 66 | + | } | |||
| 67 | + | return nil | |||
| 68 | + | } | |||
| 69 | + | ||||
| 70 | + | func (*PageScrollUseCase) scroll(ctx context.Context, wv port.WebView, cmd PageScrollCommand, continuous bool) error { | |||
| 48 | if wv == nil { | 71 | if wv == nil { | ||
| 49 | return errors.New("page scroll: nil webview") | 72 | return errors.New("page scroll: nil webview") | ||
| 50 | } | 73 | } | ||
| @@ -59,6 +82,7 @@ func (*PageScrollUseCase) Scroll(ctx context.Context, wv port.WebView, cmd PageS | |||||
| 59 | Command: toPortPageScrollCommand(cmd), | 82 | Command: toPortPageScrollCommand(cmd), | ||
| 60 | FallbackDX: dx, | 83 | FallbackDX: dx, | ||
| 61 | FallbackDY: dy, | 84 | FallbackDY: dy, | ||
| 85 | + | Continuous: continuous, | |||
| 62 | } | 86 | } | ||
| 63 | if err := scroller.ScrollPage(ctx, req); err != nil { | 87 | if err := scroller.ScrollPage(ctx, req); err != nil { | ||
| 64 | return fmt.Errorf("page scroll: %w", err) | 88 | return fmt.Errorf("page scroll: %w", err) | ||
M internal/application/usecase/page_scroll_test.go +62 -0
| @@ -138,6 +138,59 @@ func TestPageScroll_PropagatesScrollError(t *testing.T) { | |||
| 138 | 138 | } | |
| 139 | 139 | } | |
| 140 | 140 | ||
| 141 | + | func TestPageScroll_TapDistancesRemainFixed(t *testing.T) { | |
| 142 | + | tests := []struct { | |
| 143 | + | name string | |
| 144 | + | cmd PageScrollCommand | |
| 145 | + | dx int | |
| 146 | + | dy int | |
| 147 | + | }{ | |
| 148 | + | {"left 80 px", PageScrollLeft, -80, 0}, | |
| 149 | + | {"right 80 px", PageScrollRight, 80, 0}, | |
| 150 | + | {"up 80 px", PageScrollUp, 0, -80}, | |
| 151 | + | {"down 80 px", PageScrollDown, 0, 80}, | |
| 152 | + | {"up fast 320 px", PageScrollUpFast, 0, -320}, | |
| 153 | + | {"down fast 320 px", PageScrollDownFast, 0, 320}, | |
| 154 | + | } | |
| 155 | + | for _, tt := range tests { | |
| 156 | + | t.Run(tt.name, func(t *testing.T) { | |
| 157 | + | wv := newScrollableStub(nil) | |
| 158 | + | if err := NewPageScrollUseCase().Scroll(context.Background(), wv, tt.cmd); err != nil { | |
| 159 | + | t.Fatal(err) | |
| 160 | + | } | |
| 161 | + | if wv.lastRequest.FallbackDX != tt.dx || wv.lastRequest.FallbackDY != tt.dy { | |
| 162 | + | t.Fatalf("tap delta=(%d,%d), want (%d,%d)", wv.lastRequest.FallbackDX, wv.lastRequest.FallbackDY, tt.dx, tt.dy) | |
| 163 | + | } | |
| 164 | + | if wv.lastRequest.Continuous { | |
| 165 | + | t.Fatal("tap request must not be marked continuous") | |
| 166 | + | } | |
| 167 | + | }) | |
| 168 | + | } | |
| 169 | + | } | |
| 170 | + | ||
| 171 | + | func TestPageScroll_ScrollContinuousMarksRequest(t *testing.T) { | |
| 172 | + | wv := newScrollableStub(nil) | |
| 173 | + | if err := NewPageScrollUseCase().ScrollContinuous(context.Background(), wv, PageScrollDown); err != nil { | |
| 174 | + | t.Fatal(err) | |
| 175 | + | } | |
| 176 | + | if !wv.lastRequest.Continuous || wv.lastRequest.FallbackDY != 80 { | |
| 177 | + | t.Fatalf("continuous request = %+v, want continuous 80 px", wv.lastRequest) | |
| 178 | + | } | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | func TestPageScroll_StopCancelsOptionalCapability(t *testing.T) { | |
| 182 | + | wv := &cancelableScrollableStub{scrollableStub: newScrollableStub(nil)} | |
| 183 | + | if err := NewPageScrollUseCase().Stop(context.Background(), wv); err != nil { | |
| 184 | + | t.Fatal(err) | |
| 185 | + | } | |
| 186 | + | if wv.cancelCalls != 1 { | |
| 187 | + | t.Fatalf("cancel calls=%d, want 1", wv.cancelCalls) | |
| 188 | + | } | |
| 189 | + | if err := NewPageScrollUseCase().Stop(context.Background(), newScrollableStub(nil)); err != nil { | |
| 190 | + | t.Fatalf("non-cancelable WebKit-style adapter should be a no-op: %v", err) | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | ||
| 141 | 194 | func TestPageScroll_DeltaMethod(t *testing.T) { | |
| 142 | 195 | tests := []struct { | |
| 143 | 196 | cmd PageScrollCommand | |
| @@ -212,6 +265,15 @@ type scrollableStub struct { | |||
| 212 | 265 | scrollErr error | |
| 213 | 266 | } | |
| 214 | 267 | ||
| 268 | + | type cancelableScrollableStub struct { | |
| 269 | + | *scrollableStub | |
| 270 | + | cancelCalls int | |
| 271 | + | } | |
| 272 | + | ||
| 273 | + | func (s *cancelableScrollableStub) CancelPageScroll(context.Context) { | |
| 274 | + | s.cancelCalls++ | |
| 275 | + | } | |
| 276 | + | ||
| 215 | 277 | func newScrollableStub(scrollErr error) *scrollableStub { | |
| 216 | 278 | return &scrollableStub{ | |
| 217 | 279 | nonScrollableStub: &nonScrollableStub{}, | |
| @@ -138,6 +138,59 @@ func TestPageScroll_PropagatesScrollError(t *testing.T) { | |||||
| 138 | } | 138 | } | ||
| 139 | } | 139 | } | ||
| 140 | 140 | ||||
| 141 | + | func TestPageScroll_TapDistancesRemainFixed(t *testing.T) { | |||
| 142 | + | tests := []struct { | |||
| 143 | + | name string | |||
| 144 | + | cmd PageScrollCommand | |||
| 145 | + | dx int | |||
| 146 | + | dy int | |||
| 147 | + | }{ | |||
| 148 | + | {"left 80 px", PageScrollLeft, -80, 0}, | |||
| 149 | + | {"right 80 px", PageScrollRight, 80, 0}, | |||
| 150 | + | {"up 80 px", PageScrollUp, 0, -80}, | |||
| 151 | + | {"down 80 px", PageScrollDown, 0, 80}, | |||
| 152 | + | {"up fast 320 px", PageScrollUpFast, 0, -320}, | |||
| 153 | + | {"down fast 320 px", PageScrollDownFast, 0, 320}, | |||
| 154 | + | } | |||
| 155 | + | for _, tt := range tests { | |||
| 156 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 157 | + | wv := newScrollableStub(nil) | |||
| 158 | + | if err := NewPageScrollUseCase().Scroll(context.Background(), wv, tt.cmd); err != nil { | |||
| 159 | + | t.Fatal(err) | |||
| 160 | + | } | |||
| 161 | + | if wv.lastRequest.FallbackDX != tt.dx || wv.lastRequest.FallbackDY != tt.dy { | |||
| 162 | + | t.Fatalf("tap delta=(%d,%d), want (%d,%d)", wv.lastRequest.FallbackDX, wv.lastRequest.FallbackDY, tt.dx, tt.dy) | |||
| 163 | + | } | |||
| 164 | + | if wv.lastRequest.Continuous { | |||
| 165 | + | t.Fatal("tap request must not be marked continuous") | |||
| 166 | + | } | |||
| 167 | + | }) | |||
| 168 | + | } | |||
| 169 | + | } | |||
| 170 | + | ||||
| 171 | + | func TestPageScroll_ScrollContinuousMarksRequest(t *testing.T) { | |||
| 172 | + | wv := newScrollableStub(nil) | |||
| 173 | + | if err := NewPageScrollUseCase().ScrollContinuous(context.Background(), wv, PageScrollDown); err != nil { | |||
| 174 | + | t.Fatal(err) | |||
| 175 | + | } | |||
| 176 | + | if !wv.lastRequest.Continuous || wv.lastRequest.FallbackDY != 80 { | |||
| 177 | + | t.Fatalf("continuous request = %+v, want continuous 80 px", wv.lastRequest) | |||
| 178 | + | } | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | func TestPageScroll_StopCancelsOptionalCapability(t *testing.T) { | |||
| 182 | + | wv := &cancelableScrollableStub{scrollableStub: newScrollableStub(nil)} | |||
| 183 | + | if err := NewPageScrollUseCase().Stop(context.Background(), wv); err != nil { | |||
| 184 | + | t.Fatal(err) | |||
| 185 | + | } | |||
| 186 | + | if wv.cancelCalls != 1 { | |||
| 187 | + | t.Fatalf("cancel calls=%d, want 1", wv.cancelCalls) | |||
| 188 | + | } | |||
| 189 | + | if err := NewPageScrollUseCase().Stop(context.Background(), newScrollableStub(nil)); err != nil { | |||
| 190 | + | t.Fatalf("non-cancelable WebKit-style adapter should be a no-op: %v", err) | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | ||||
| 141 | func TestPageScroll_DeltaMethod(t *testing.T) { | 194 | func TestPageScroll_DeltaMethod(t *testing.T) { | ||
| 142 | tests := []struct { | 195 | tests := []struct { | ||
| 143 | cmd PageScrollCommand | 196 | cmd PageScrollCommand | ||
| @@ -212,6 +265,15 @@ type scrollableStub struct { | |||||
| 212 | scrollErr error | 265 | scrollErr error | ||
| 213 | } | 266 | } | ||
| 214 | 267 | ||||
| 268 | + | type cancelableScrollableStub struct { | |||
| 269 | + | *scrollableStub | |||
| 270 | + | cancelCalls int | |||
| 271 | + | } | |||
| 272 | + | ||||
| 273 | + | func (s *cancelableScrollableStub) CancelPageScroll(context.Context) { | |||
| 274 | + | s.cancelCalls++ | |||
| 275 | + | } | |||
| 276 | + | ||||
| 215 | func newScrollableStub(scrollErr error) *scrollableStub { | 277 | func newScrollableStub(scrollErr error) *scrollableStub { | ||
| 216 | return &scrollableStub{ | 278 | return &scrollableStub{ | ||
| 217 | nonScrollableStub: &nonScrollableStub{}, | 279 | nonScrollableStub: &nonScrollableStub{}, | ||
M internal/bootstrap/runtime_config.go +1 -0
| @@ -140,6 +140,7 @@ func cloneRuntimeSearchShortcuts(in map[string]entity.RuntimeSearchShortcut) map | |||
| 140 | 140 | func cloneWorkspaceConfig(in entity.WorkspaceConfig) entity.WorkspaceConfig { | |
| 141 | 141 | in.PaneMode.Actions = cloneActionBindings(in.PaneMode.Actions) | |
| 142 | 142 | in.TabMode.Actions = cloneActionBindings(in.TabMode.Actions) | |
| 143 | + | in.PageMode.Actions = cloneActionBindings(in.PageMode.Actions) | |
| 143 | 144 | in.ResizeMode.Actions = cloneActionBindings(in.ResizeMode.Actions) | |
| 144 | 145 | in.Shortcuts.Actions = cloneActionBindings(in.Shortcuts.Actions) | |
| 145 | 146 | in.FloatingPane.Profiles = cloneFloatingPaneProfiles(in.FloatingPane.Profiles) | |
| @@ -140,6 +140,7 @@ func cloneRuntimeSearchShortcuts(in map[string]entity.RuntimeSearchShortcut) map | |||||
| 140 | func cloneWorkspaceConfig(in entity.WorkspaceConfig) entity.WorkspaceConfig { | 140 | func cloneWorkspaceConfig(in entity.WorkspaceConfig) entity.WorkspaceConfig { | ||
| 141 | in.PaneMode.Actions = cloneActionBindings(in.PaneMode.Actions) | 141 | in.PaneMode.Actions = cloneActionBindings(in.PaneMode.Actions) | ||
| 142 | in.TabMode.Actions = cloneActionBindings(in.TabMode.Actions) | 142 | in.TabMode.Actions = cloneActionBindings(in.TabMode.Actions) | ||
| 143 | + | in.PageMode.Actions = cloneActionBindings(in.PageMode.Actions) | |||
| 143 | in.ResizeMode.Actions = cloneActionBindings(in.ResizeMode.Actions) | 144 | in.ResizeMode.Actions = cloneActionBindings(in.ResizeMode.Actions) | ||
| 144 | in.Shortcuts.Actions = cloneActionBindings(in.Shortcuts.Actions) | 145 | in.Shortcuts.Actions = cloneActionBindings(in.Shortcuts.Actions) | ||
| 145 | in.FloatingPane.Profiles = cloneFloatingPaneProfiles(in.FloatingPane.Profiles) | 146 | in.FloatingPane.Profiles = cloneFloatingPaneProfiles(in.FloatingPane.Profiles) | ||
M internal/bootstrap/runtime_config_test.go +12 -0
| @@ -258,6 +258,9 @@ func runtimeConfigWithNestedMutableFields() *config.Config { | |||
| 258 | 258 | cfg.Workspace.TabMode.Actions = map[string]entity.ActionBinding{ | |
| 259 | 259 | "next-tab": {Keys: []string{"l"}, Desc: "Next tab"}, | |
| 260 | 260 | } | |
| 261 | + | cfg.Workspace.PageMode.Actions = map[string]entity.ActionBinding{ | |
| 262 | + | "page-scroll-down": {Keys: []string{"j"}, Desc: "Scroll down"}, | |
| 263 | + | } | |
| 261 | 264 | cfg.Workspace.ResizeMode.Actions = map[string]entity.ActionBinding{ | |
| 262 | 265 | "grow-right": {Keys: []string{"right"}, Desc: "Grow right"}, | |
| 263 | 266 | } | |
| @@ -280,6 +283,9 @@ func mutateNestedRuntimeConfigSnapshot(snapshot entity.RuntimeConfigSnapshot) { | |||
| 280 | 283 | mutateActionBinding(snapshot.UI.Workspace.TabMode.Actions, "next-tab", "mutated-tab") | |
| 281 | 284 | snapshot.UI.Workspace.TabMode.Actions["added-tab"] = entity.ActionBinding{Keys: []string{"added"}} | |
| 282 | 285 | ||
| 286 | + | mutateActionBinding(snapshot.UI.Workspace.PageMode.Actions, "page-scroll-down", "mutated-page") | |
| 287 | + | snapshot.UI.Workspace.PageMode.Actions["added-page"] = entity.ActionBinding{Keys: []string{"added"}} | |
| 288 | + | ||
| 283 | 289 | mutateActionBinding(snapshot.UI.Workspace.ResizeMode.Actions, "grow-right", "mutated-resize") | |
| 284 | 290 | snapshot.UI.Workspace.ResizeMode.Actions["added-resize"] = entity.ActionBinding{Keys: []string{"added"}} | |
| 285 | 291 | ||
| @@ -310,6 +316,9 @@ func assertNestedRuntimeConfigSnapshotUnchanged(t *testing.T, snapshot entity.Ru | |||
| 310 | 316 | assertActionBinding(t, snapshot.UI.Workspace.TabMode.Actions, "next-tab", "l") | |
| 311 | 317 | assertMapEntryAbsent(t, snapshot.UI.Workspace.TabMode.Actions, "added-tab") | |
| 312 | 318 | ||
| 319 | + | assertActionBinding(t, snapshot.UI.Workspace.PageMode.Actions, "page-scroll-down", "j") | |
| 320 | + | assertMapEntryAbsent(t, snapshot.UI.Workspace.PageMode.Actions, "added-page") | |
| 321 | + | ||
| 313 | 322 | assertActionBinding(t, snapshot.UI.Workspace.ResizeMode.Actions, "grow-right", "right") | |
| 314 | 323 | assertMapEntryAbsent(t, snapshot.UI.Workspace.ResizeMode.Actions, "added-resize") | |
| 315 | 324 | ||
| @@ -357,6 +366,9 @@ func assertNestedSourceConfigUnchanged(t *testing.T, cfg *config.Config) { | |||
| 357 | 366 | assertActionBinding(t, cfg.Workspace.TabMode.Actions, "next-tab", "l") | |
| 358 | 367 | assertMapEntryAbsent(t, cfg.Workspace.TabMode.Actions, "added-tab") | |
| 359 | 368 | ||
| 369 | + | assertActionBinding(t, cfg.Workspace.PageMode.Actions, "page-scroll-down", "j") | |
| 370 | + | assertMapEntryAbsent(t, cfg.Workspace.PageMode.Actions, "added-page") | |
| 371 | + | ||
| 360 | 372 | assertActionBinding(t, cfg.Workspace.ResizeMode.Actions, "grow-right", "right") | |
| 361 | 373 | assertMapEntryAbsent(t, cfg.Workspace.ResizeMode.Actions, "added-resize") | |
| 362 | 374 | ||
| @@ -258,6 +258,9 @@ func runtimeConfigWithNestedMutableFields() *config.Config { | |||||
| 258 | cfg.Workspace.TabMode.Actions = map[string]entity.ActionBinding{ | 258 | cfg.Workspace.TabMode.Actions = map[string]entity.ActionBinding{ | ||
| 259 | "next-tab": {Keys: []string{"l"}, Desc: "Next tab"}, | 259 | "next-tab": {Keys: []string{"l"}, Desc: "Next tab"}, | ||
| 260 | } | 260 | } | ||
| 261 | + | cfg.Workspace.PageMode.Actions = map[string]entity.ActionBinding{ | |||
| 262 | + | "page-scroll-down": {Keys: []string{"j"}, Desc: "Scroll down"}, | |||
| 263 | + | } | |||
| 261 | cfg.Workspace.ResizeMode.Actions = map[string]entity.ActionBinding{ | 264 | cfg.Workspace.ResizeMode.Actions = map[string]entity.ActionBinding{ | ||
| 262 | "grow-right": {Keys: []string{"right"}, Desc: "Grow right"}, | 265 | "grow-right": {Keys: []string{"right"}, Desc: "Grow right"}, | ||
| 263 | } | 266 | } | ||
| @@ -280,6 +283,9 @@ func mutateNestedRuntimeConfigSnapshot(snapshot entity.RuntimeConfigSnapshot) { | |||||
| 280 | mutateActionBinding(snapshot.UI.Workspace.TabMode.Actions, "next-tab", "mutated-tab") | 283 | mutateActionBinding(snapshot.UI.Workspace.TabMode.Actions, "next-tab", "mutated-tab") | ||
| 281 | snapshot.UI.Workspace.TabMode.Actions["added-tab"] = entity.ActionBinding{Keys: []string{"added"}} | 284 | snapshot.UI.Workspace.TabMode.Actions["added-tab"] = entity.ActionBinding{Keys: []string{"added"}} | ||
| 282 | 285 | ||||
| 286 | + | mutateActionBinding(snapshot.UI.Workspace.PageMode.Actions, "page-scroll-down", "mutated-page") | |||
| 287 | + | snapshot.UI.Workspace.PageMode.Actions["added-page"] = entity.ActionBinding{Keys: []string{"added"}} | |||
| 288 | + | ||||
| 283 | mutateActionBinding(snapshot.UI.Workspace.ResizeMode.Actions, "grow-right", "mutated-resize") | 289 | mutateActionBinding(snapshot.UI.Workspace.ResizeMode.Actions, "grow-right", "mutated-resize") | ||
| 284 | snapshot.UI.Workspace.ResizeMode.Actions["added-resize"] = entity.ActionBinding{Keys: []string{"added"}} | 290 | snapshot.UI.Workspace.ResizeMode.Actions["added-resize"] = entity.ActionBinding{Keys: []string{"added"}} | ||
| 285 | 291 | ||||
| @@ -310,6 +316,9 @@ func assertNestedRuntimeConfigSnapshotUnchanged(t *testing.T, snapshot entity.Ru | |||||
| 310 | assertActionBinding(t, snapshot.UI.Workspace.TabMode.Actions, "next-tab", "l") | 316 | assertActionBinding(t, snapshot.UI.Workspace.TabMode.Actions, "next-tab", "l") | ||
| 311 | assertMapEntryAbsent(t, snapshot.UI.Workspace.TabMode.Actions, "added-tab") | 317 | assertMapEntryAbsent(t, snapshot.UI.Workspace.TabMode.Actions, "added-tab") | ||
| 312 | 318 | ||||
| 319 | + | assertActionBinding(t, snapshot.UI.Workspace.PageMode.Actions, "page-scroll-down", "j") | |||
| 320 | + | assertMapEntryAbsent(t, snapshot.UI.Workspace.PageMode.Actions, "added-page") | |||
| 321 | + | ||||
| 313 | assertActionBinding(t, snapshot.UI.Workspace.ResizeMode.Actions, "grow-right", "right") | 322 | assertActionBinding(t, snapshot.UI.Workspace.ResizeMode.Actions, "grow-right", "right") | ||
| 314 | assertMapEntryAbsent(t, snapshot.UI.Workspace.ResizeMode.Actions, "added-resize") | 323 | assertMapEntryAbsent(t, snapshot.UI.Workspace.ResizeMode.Actions, "added-resize") | ||
| 315 | 324 | ||||
| @@ -357,6 +366,9 @@ func assertNestedSourceConfigUnchanged(t *testing.T, cfg *config.Config) { | |||||
| 357 | assertActionBinding(t, cfg.Workspace.TabMode.Actions, "next-tab", "l") | 366 | assertActionBinding(t, cfg.Workspace.TabMode.Actions, "next-tab", "l") | ||
| 358 | assertMapEntryAbsent(t, cfg.Workspace.TabMode.Actions, "added-tab") | 367 | assertMapEntryAbsent(t, cfg.Workspace.TabMode.Actions, "added-tab") | ||
| 359 | 368 | ||||
| 369 | + | assertActionBinding(t, cfg.Workspace.PageMode.Actions, "page-scroll-down", "j") | |||
| 370 | + | assertMapEntryAbsent(t, cfg.Workspace.PageMode.Actions, "added-page") | |||
| 371 | + | ||||
| 360 | assertActionBinding(t, cfg.Workspace.ResizeMode.Actions, "grow-right", "right") | 372 | assertActionBinding(t, cfg.Workspace.ResizeMode.Actions, "grow-right", "right") | ||
| 361 | assertMapEntryAbsent(t, cfg.Workspace.ResizeMode.Actions, "added-resize") | 373 | assertMapEntryAbsent(t, cfg.Workspace.ResizeMode.Actions, "added-resize") | ||
| 362 | 374 | ||||
M internal/infrastructure/cef/app.go +5 -5
| @@ -280,16 +280,16 @@ type dumberApp struct { | |||
| 280 | 280 | func newDumberApp(engine *Engine) purecef.App { | |
| 281 | 281 | app := &dumberApp{engine: engine} | |
| 282 | 282 | app.bph = &dumberBPH{engine: engine} | |
| 283 | - | app.rph = newPopupOpenerRenderProcessHandler() | |
| 283 | + | app.rph = newComposedRenderProcessHandler() | |
| 284 | 284 | return app | |
| 285 | 285 | } | |
| 286 | 286 | ||
| 287 | 287 | // NewSubprocessApp returns a lightweight raw App implementation for CEF | |
| 288 | - | // subprocesses re-executed from the main dumber binary. Keep the legacy | |
| 289 | - | // renderer bridge disabled, but allow the minimal popup-opener render handler | |
| 290 | - | // needed to install synthetic opener state before page scripts run. | |
| 288 | + | // subprocesses re-executed from the main dumber binary. Keep the legacy V8 | |
| 289 | + | // renderer bridge disabled while composing editable-focus reporting with the | |
| 290 | + | // popup-opener callbacks needed before page scripts run. | |
| 291 | 291 | func NewSubprocessApp() purecef.App { | |
| 292 | - | return &subprocessApp{rph: newPopupOpenerRenderProcessHandler()} | |
| 292 | + | return &subprocessApp{rph: newComposedRenderProcessHandler()} | |
| 293 | 293 | } | |
| 294 | 294 | ||
| 295 | 295 | // maxCmdLineLogLen limits logged command line length to avoid leaking sensitive paths. | |
| @@ -280,16 +280,16 @@ type dumberApp struct { | |||||
| 280 | func newDumberApp(engine *Engine) purecef.App { | 280 | func newDumberApp(engine *Engine) purecef.App { | ||
| 281 | app := &dumberApp{engine: engine} | 281 | app := &dumberApp{engine: engine} | ||
| 282 | app.bph = &dumberBPH{engine: engine} | 282 | app.bph = &dumberBPH{engine: engine} | ||
| 283 | - | app.rph = newPopupOpenerRenderProcessHandler() | 283 | + | app.rph = newComposedRenderProcessHandler() |
| 284 | return app | 284 | return app | ||
| 285 | } | 285 | } | ||
| 286 | 286 | ||||
| 287 | // NewSubprocessApp returns a lightweight raw App implementation for CEF | 287 | // NewSubprocessApp returns a lightweight raw App implementation for CEF | ||
| 288 | - | // subprocesses re-executed from the main dumber binary. Keep the legacy | 288 | + | // subprocesses re-executed from the main dumber binary. Keep the legacy V8 |
| 289 | - | // renderer bridge disabled, but allow the minimal popup-opener render handler | 289 | + | // renderer bridge disabled while composing editable-focus reporting with the |
| 290 | - | // needed to install synthetic opener state before page scripts run. | 290 | + | // popup-opener callbacks needed before page scripts run. |
| 291 | func NewSubprocessApp() purecef.App { | 291 | func NewSubprocessApp() purecef.App { | ||
| 292 | - | return &subprocessApp{rph: newPopupOpenerRenderProcessHandler()} | 292 | + | return &subprocessApp{rph: newComposedRenderProcessHandler()} |
| 293 | } | 293 | } | ||
| 294 | 294 | ||||
| 295 | // maxCmdLineLogLen limits logged command line length to avoid leaking sensitive paths. | 295 | // maxCmdLineLogLen limits logged command line length to avoid leaking sensitive paths. | ||
M internal/infrastructure/cef/popup_opener_renderer.go +25 -0
| @@ -50,6 +50,31 @@ func decodePopupOpenerRenderMetadata(extraInfo purecef.DictionaryValue) (popupOp | |||
| 50 | 50 | return metadata, true | |
| 51 | 51 | } | |
| 52 | 52 | ||
| 53 | + | // composedRenderProcessHandler keeps the popup-opener handler as the primary | |
| 54 | + | // CEF renderer handler and adds only editable-focus reporting from the legacy | |
| 55 | + | // renderer bridge. Embedding preserves every popup callback without enabling | |
| 56 | + | // the bridge's V8 extension in OnWebKitInitialized. | |
| 57 | + | type composedRenderProcessHandler struct { | |
| 58 | + | *popupOpenerRenderProcessHandler | |
| 59 | + | editableFocusHandler *rendererBridgeProcessHandler | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | func newComposedRenderProcessHandler() *composedRenderProcessHandler { | |
| 63 | + | return &composedRenderProcessHandler{ | |
| 64 | + | popupOpenerRenderProcessHandler: newPopupOpenerRenderProcessHandler(), | |
| 65 | + | editableFocusHandler: &rendererBridgeProcessHandler{}, | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | func (h *composedRenderProcessHandler) OnFocusedNodeChanged( | |
| 70 | + | browser purecef.Browser, | |
| 71 | + | frame purecef.Frame, | |
| 72 | + | node purecef.Domnode, | |
| 73 | + | ) { | |
| 74 | + | h.popupOpenerRenderProcessHandler.OnFocusedNodeChanged(browser, frame, node) | |
| 75 | + | h.editableFocusHandler.OnFocusedNodeChanged(browser, frame, node) | |
| 76 | + | } | |
| 77 | + | ||
| 53 | 78 | type popupOpenerRenderProcessHandler struct { | |
| 54 | 79 | mu sync.RWMutex | |
| 55 | 80 | byBrowserID map[int32]popupOpenerRenderMetadata | |
| @@ -50,6 +50,31 @@ func decodePopupOpenerRenderMetadata(extraInfo purecef.DictionaryValue) (popupOp | |||||
| 50 | return metadata, true | 50 | return metadata, true | ||
| 51 | } | 51 | } | ||
| 52 | 52 | ||||
| 53 | + | // composedRenderProcessHandler keeps the popup-opener handler as the primary | |||
| 54 | + | // CEF renderer handler and adds only editable-focus reporting from the legacy | |||
| 55 | + | // renderer bridge. Embedding preserves every popup callback without enabling | |||
| 56 | + | // the bridge's V8 extension in OnWebKitInitialized. | |||
| 57 | + | type composedRenderProcessHandler struct { | |||
| 58 | + | *popupOpenerRenderProcessHandler | |||
| 59 | + | editableFocusHandler *rendererBridgeProcessHandler | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | func newComposedRenderProcessHandler() *composedRenderProcessHandler { | |||
| 63 | + | return &composedRenderProcessHandler{ | |||
| 64 | + | popupOpenerRenderProcessHandler: newPopupOpenerRenderProcessHandler(), | |||
| 65 | + | editableFocusHandler: &rendererBridgeProcessHandler{}, | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | ||||
| 69 | + | func (h *composedRenderProcessHandler) OnFocusedNodeChanged( | |||
| 70 | + | browser purecef.Browser, | |||
| 71 | + | frame purecef.Frame, | |||
| 72 | + | node purecef.Domnode, | |||
| 73 | + | ) { | |||
| 74 | + | h.popupOpenerRenderProcessHandler.OnFocusedNodeChanged(browser, frame, node) | |||
| 75 | + | h.editableFocusHandler.OnFocusedNodeChanged(browser, frame, node) | |||
| 76 | + | } | |||
| 77 | + | ||||
| 53 | type popupOpenerRenderProcessHandler struct { | 78 | type popupOpenerRenderProcessHandler struct { | ||
| 54 | mu sync.RWMutex | 79 | mu sync.RWMutex | ||
| 55 | byBrowserID map[int32]popupOpenerRenderMetadata | 80 | byBrowserID map[int32]popupOpenerRenderMetadata | ||
M internal/infrastructure/cef/renderer_bridge_test.go +95 -0
| @@ -267,3 +267,98 @@ func TestRendererBridgeProcessHandler_OnFocusedNodeChanged_ReportsEditableState( | |||
| 267 | 267 | ||
| 268 | 268 | (&rendererBridgeProcessHandler{}).OnFocusedNodeChanged(nil, frame, stubEditableDomnode{editable: true}) | |
| 269 | 269 | } | |
| 270 | + | ||
| 271 | + | type runtimeEditableDomnode struct { | |
| 272 | + | stubEditableDomnode | |
| 273 | + | tagName string | |
| 274 | + | } | |
| 275 | + | ||
| 276 | + | func (n runtimeEditableDomnode) GetElementTagName() string { return n.tagName } | |
| 277 | + | ||
| 278 | + | func TestRuntimeRenderProcessHandlers_ReportEditableFocusedNodes(t *testing.T) { | |
| 279 | + | oldFactory := newRendererBridgeProcessMessage | |
| 280 | + | t.Cleanup(func() { newRendererBridgeProcessMessage = oldFactory }) | |
| 281 | + | newRendererBridgeProcessMessage = func(name string) purecef.ProcessMessage { | |
| 282 | + | return newTestBridgeProcessMessage(name, true) | |
| 283 | + | } | |
| 284 | + | ||
| 285 | + | apps := []struct { | |
| 286 | + | name string | |
| 287 | + | new func() purecef.App | |
| 288 | + | }{ | |
| 289 | + | {name: "main process", new: func() purecef.App { return newDumberApp(&Engine{}) }}, | |
| 290 | + | {name: "subprocess", new: NewSubprocessApp}, | |
| 291 | + | } | |
| 292 | + | nodes := []struct { | |
| 293 | + | name string | |
| 294 | + | node purecef.Domnode | |
| 295 | + | }{ | |
| 296 | + | {name: "input", node: runtimeEditableDomnode{stubEditableDomnode: stubEditableDomnode{editable: true}, tagName: "INPUT"}}, | |
| 297 | + | {name: "textarea", node: runtimeEditableDomnode{stubEditableDomnode: stubEditableDomnode{editable: true}, tagName: "TEXTAREA"}}, | |
| 298 | + | {name: "contenteditable", node: runtimeEditableDomnode{stubEditableDomnode: stubEditableDomnode{editable: true}, tagName: "DIV"}}, | |
| 299 | + | } | |
| 300 | + | ||
| 301 | + | for _, appCase := range apps { | |
| 302 | + | t.Run(appCase.name, func(t *testing.T) { | |
| 303 | + | for _, nodeCase := range nodes { | |
| 304 | + | t.Run(nodeCase.name, func(t *testing.T) { | |
| 305 | + | frame := cefmocks.NewMockFrame(t) | |
| 306 | + | frame.EXPECT().SendProcessMessage(purecef.ProcessIDPidBrowser, mock.Anything). | |
| 307 | + | Run(func(_ purecef.ProcessID, message purecef.ProcessMessage) { | |
| 308 | + | action, editable, ok := decodeRendererBridgeProcessMessage(message) | |
| 309 | + | require.True(t, ok) | |
| 310 | + | require.Equal(t, rendererBridgeActionEditableFocusChanged, action) | |
| 311 | + | require.Equal(t, "1", editable) | |
| 312 | + | }).Once() | |
| 313 | + | ||
| 314 | + | handler := appCase.new().GetRenderProcessHandler() | |
| 315 | + | require.NotNil(t, handler) | |
| 316 | + | handler.OnFocusedNodeChanged(nil, frame, nodeCase.node) | |
| 317 | + | }) | |
| 318 | + | } | |
| 319 | + | }) | |
| 320 | + | } | |
| 321 | + | } | |
| 322 | + | ||
| 323 | + | func TestRuntimeRenderProcessHandlers_PreservePopupOpenerCallbacks(t *testing.T) { | |
| 324 | + | apps := []struct { | |
| 325 | + | name string | |
| 326 | + | new func() purecef.App | |
| 327 | + | }{ | |
| 328 | + | {name: "main process", new: func() purecef.App { return newDumberApp(&Engine{}) }}, | |
| 329 | + | {name: "subprocess", new: NewSubprocessApp}, | |
| 330 | + | } | |
| 331 | + | ||
| 332 | + | for _, appCase := range apps { | |
| 333 | + | t.Run(appCase.name, func(t *testing.T) { | |
| 334 | + | browser := cefmocks.NewMockBrowser(t) | |
| 335 | + | browser.EXPECT().GetIdentifier().Return(int32(42)).Times(4) | |
| 336 | + | ctx := &testPopupOpenerV8Context{valid: true} | |
| 337 | + | handler := appCase.new().GetRenderProcessHandler() | |
| 338 | + | require.NotNil(t, handler) | |
| 339 | + | require.Nil(t, handler.GetLoadHandler()) | |
| 340 | + | ||
| 341 | + | handler.OnBrowserCreated(browser, testPopupOpenerDictionaryValue{ | |
| 342 | + | strings: map[string]string{ | |
| 343 | + | popupOpenerExtraInfoParentURIKey: "https://example.com/login", | |
| 344 | + | popupOpenerExtraInfoBridgeNonceKey: "bridge-nonce", | |
| 345 | + | }, | |
| 346 | + | bools: map[string]int32{popupOpenerExtraInfoEnabledKey: 1}, | |
| 347 | + | }) | |
| 348 | + | handler.OnContextCreated(browser, stubFrame{main: true, url: "https://example.com/popup"}, ctx) | |
| 349 | + | ||
| 350 | + | require.Equal(t, 1, ctx.enterCalls) | |
| 351 | + | require.Equal(t, 1, ctx.exitCalls) | |
| 352 | + | require.Contains(t, ctx.evalCode, "popup-opener-navigate") | |
| 353 | + | require.Contains(t, ctx.evalCode, "popup-opener-post-message") | |
| 354 | + | require.Equal(t, popupOpenerRenderScriptURL, ctx.evalURL) | |
| 355 | + | require.Zero(t, handler.OnProcessMessageReceived(nil, nil, purecef.ProcessIDPidBrowser, nil)) | |
| 356 | + | ||
| 357 | + | handler.OnBrowserDestroyed(browser) | |
| 358 | + | releasedCtx := &testPopupOpenerV8Context{valid: true} | |
| 359 | + | handler.OnContextCreated(browser, stubFrame{main: true, url: "https://example.com/popup"}, releasedCtx) | |
| 360 | + | require.Zero(t, releasedCtx.enterCalls) | |
| 361 | + | require.Empty(t, releasedCtx.evalCode) | |
| 362 | + | }) | |
| 363 | + | } | |
| 364 | + | } | |
| @@ -267,3 +267,98 @@ func TestRendererBridgeProcessHandler_OnFocusedNodeChanged_ReportsEditableState( | |||||
| 267 | 267 | ||||
| 268 | (&rendererBridgeProcessHandler{}).OnFocusedNodeChanged(nil, frame, stubEditableDomnode{editable: true}) | 268 | (&rendererBridgeProcessHandler{}).OnFocusedNodeChanged(nil, frame, stubEditableDomnode{editable: true}) | ||
| 269 | } | 269 | } | ||
| 270 | + | ||||
| 271 | + | type runtimeEditableDomnode struct { | |||
| 272 | + | stubEditableDomnode | |||
| 273 | + | tagName string | |||
| 274 | + | } | |||
| 275 | + | ||||
| 276 | + | func (n runtimeEditableDomnode) GetElementTagName() string { return n.tagName } | |||
| 277 | + | ||||
| 278 | + | func TestRuntimeRenderProcessHandlers_ReportEditableFocusedNodes(t *testing.T) { | |||
| 279 | + | oldFactory := newRendererBridgeProcessMessage | |||
| 280 | + | t.Cleanup(func() { newRendererBridgeProcessMessage = oldFactory }) | |||
| 281 | + | newRendererBridgeProcessMessage = func(name string) purecef.ProcessMessage { | |||
| 282 | + | return newTestBridgeProcessMessage(name, true) | |||
| 283 | + | } | |||
| 284 | + | ||||
| 285 | + | apps := []struct { | |||
| 286 | + | name string | |||
| 287 | + | new func() purecef.App | |||
| 288 | + | }{ | |||
| 289 | + | {name: "main process", new: func() purecef.App { return newDumberApp(&Engine{}) }}, | |||
| 290 | + | {name: "subprocess", new: NewSubprocessApp}, | |||
| 291 | + | } | |||
| 292 | + | nodes := []struct { | |||
| 293 | + | name string | |||
| 294 | + | node purecef.Domnode | |||
| 295 | + | }{ | |||
| 296 | + | {name: "input", node: runtimeEditableDomnode{stubEditableDomnode: stubEditableDomnode{editable: true}, tagName: "INPUT"}}, | |||
| 297 | + | {name: "textarea", node: runtimeEditableDomnode{stubEditableDomnode: stubEditableDomnode{editable: true}, tagName: "TEXTAREA"}}, | |||
| 298 | + | {name: "contenteditable", node: runtimeEditableDomnode{stubEditableDomnode: stubEditableDomnode{editable: true}, tagName: "DIV"}}, | |||
| 299 | + | } | |||
| 300 | + | ||||
| 301 | + | for _, appCase := range apps { | |||
| 302 | + | t.Run(appCase.name, func(t *testing.T) { | |||
| 303 | + | for _, nodeCase := range nodes { | |||
| 304 | + | t.Run(nodeCase.name, func(t *testing.T) { | |||
| 305 | + | frame := cefmocks.NewMockFrame(t) | |||
| 306 | + | frame.EXPECT().SendProcessMessage(purecef.ProcessIDPidBrowser, mock.Anything). | |||
| 307 | + | Run(func(_ purecef.ProcessID, message purecef.ProcessMessage) { | |||
| 308 | + | action, editable, ok := decodeRendererBridgeProcessMessage(message) | |||
| 309 | + | require.True(t, ok) | |||
| 310 | + | require.Equal(t, rendererBridgeActionEditableFocusChanged, action) | |||
| 311 | + | require.Equal(t, "1", editable) | |||
| 312 | + | }).Once() | |||
| 313 | + | ||||
| 314 | + | handler := appCase.new().GetRenderProcessHandler() | |||
| 315 | + | require.NotNil(t, handler) | |||
| 316 | + | handler.OnFocusedNodeChanged(nil, frame, nodeCase.node) | |||
| 317 | + | }) | |||
| 318 | + | } | |||
| 319 | + | }) | |||
| 320 | + | } | |||
| 321 | + | } | |||
| 322 | + | ||||
| 323 | + | func TestRuntimeRenderProcessHandlers_PreservePopupOpenerCallbacks(t *testing.T) { | |||
| 324 | + | apps := []struct { | |||
| 325 | + | name string | |||
| 326 | + | new func() purecef.App | |||
| 327 | + | }{ | |||
| 328 | + | {name: "main process", new: func() purecef.App { return newDumberApp(&Engine{}) }}, | |||
| 329 | + | {name: "subprocess", new: NewSubprocessApp}, | |||
| 330 | + | } | |||
| 331 | + | ||||
| 332 | + | for _, appCase := range apps { | |||
| 333 | + | t.Run(appCase.name, func(t *testing.T) { | |||
| 334 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 335 | + | browser.EXPECT().GetIdentifier().Return(int32(42)).Times(4) | |||
| 336 | + | ctx := &testPopupOpenerV8Context{valid: true} | |||
| 337 | + | handler := appCase.new().GetRenderProcessHandler() | |||
| 338 | + | require.NotNil(t, handler) | |||
| 339 | + | require.Nil(t, handler.GetLoadHandler()) | |||
| 340 | + | ||||
| 341 | + | handler.OnBrowserCreated(browser, testPopupOpenerDictionaryValue{ | |||
| 342 | + | strings: map[string]string{ | |||
| 343 | + | popupOpenerExtraInfoParentURIKey: "https://example.com/login", | |||
| 344 | + | popupOpenerExtraInfoBridgeNonceKey: "bridge-nonce", | |||
| 345 | + | }, | |||
| 346 | + | bools: map[string]int32{popupOpenerExtraInfoEnabledKey: 1}, | |||
| 347 | + | }) | |||
| 348 | + | handler.OnContextCreated(browser, stubFrame{main: true, url: "https://example.com/popup"}, ctx) | |||
| 349 | + | ||||
| 350 | + | require.Equal(t, 1, ctx.enterCalls) | |||
| 351 | + | require.Equal(t, 1, ctx.exitCalls) | |||
| 352 | + | require.Contains(t, ctx.evalCode, "popup-opener-navigate") | |||
| 353 | + | require.Contains(t, ctx.evalCode, "popup-opener-post-message") | |||
| 354 | + | require.Equal(t, popupOpenerRenderScriptURL, ctx.evalURL) | |||
| 355 | + | require.Zero(t, handler.OnProcessMessageReceived(nil, nil, purecef.ProcessIDPidBrowser, nil)) | |||
| 356 | + | ||||
| 357 | + | handler.OnBrowserDestroyed(browser) | |||
| 358 | + | releasedCtx := &testPopupOpenerV8Context{valid: true} | |||
| 359 | + | handler.OnContextCreated(browser, stubFrame{main: true, url: "https://example.com/popup"}, releasedCtx) | |||
| 360 | + | require.Zero(t, releasedCtx.enterCalls) | |||
| 361 | + | require.Empty(t, releasedCtx.evalCode) | |||
| 362 | + | }) | |||
| 363 | + | } | |||
| 364 | + | } | |||
M internal/infrastructure/cef/webview.go +127 -1
| @@ -36,6 +36,7 @@ var ( | |||
| 36 | 36 | _ port.ViewportSyncCapable = (*WebView)(nil) | |
| 37 | 37 | _ port.OAuthCallbackCapable = (*WebView)(nil) | |
| 38 | 38 | _ port.PageScrollable = (*WebView)(nil) | |
| 39 | + | _ port.PageScrollCanceler = (*WebView)(nil) | |
| 39 | 40 | ) | |
| 40 | 41 | ||
| 41 | 42 | // errDestroyed is returned when an operation is attempted on a destroyed WebView. | |
| @@ -71,8 +72,24 @@ var cefLoadWatchdogDelays = []time.Duration{ | |||
| 71 | 72 | const ( | |
| 72 | 73 | cefGTKSyncDispatchTimeout = 2 * time.Second | |
| 73 | 74 | cefGTKSyncDispatchSlowThreshold = 250 * time.Millisecond | |
| 75 | + | maxPendingPageScroll = 3200 | |
| 74 | 76 | ) | |
| 75 | 77 | ||
| 78 | + | type pageScrollQueue struct { | |
| 79 | + | mu sync.Mutex | |
| 80 | + | commitMu sync.Mutex | |
| 81 | + | tapDX int | |
| 82 | + | tapDY int | |
| 83 | + | heldDX int | |
| 84 | + | heldDY int | |
| 85 | + | flushPending bool | |
| 86 | + | generation uint64 | |
| 87 | + | ||
| 88 | + | // beforeExecute is a deterministic test seam for the drain-to-commit | |
| 89 | + | // boundary. Production leaves it nil. | |
| 90 | + | beforeExecute func() | |
| 91 | + | } | |
| 92 | + | ||
| 76 | 93 | var ( | |
| 77 | 94 | cefNewTask = purecef.NewTask | |
| 78 | 95 | cefNewStringVisitor = purecef.NewStringVisitor | |
| @@ -133,6 +150,8 @@ type WebView struct { | |||
| 133 | 150 | adaptiveFrameRatePollID uint | |
| 134 | 151 | lastAdaptiveFrameRate int32 | |
| 135 | 152 | ||
| 153 | + | pageScrollQueue pageScrollQueue | |
| 154 | + | ||
| 136 | 155 | // beginFrameTick drives CEF external BeginFrame requests while the GTK | |
| 137 | 156 | // widget is visible. Access is guarded by mu. | |
| 138 | 157 | beginFrameTick *gtk.TickCallback | |
| @@ -1203,6 +1222,7 @@ func (wv *WebView) Destroy() { | |||
| 1203 | 1222 | if !wv.destroyed.CompareAndSwap(false, true) { | |
| 1204 | 1223 | return | |
| 1205 | 1224 | } | |
| 1225 | + | wv.resetPageScrollQueue() | |
| 1206 | 1226 | wv.syntheticPopupMu.Lock() | |
| 1207 | 1227 | wv.syntheticPopups = nil | |
| 1208 | 1228 | wv.syntheticPopupMu.Unlock() | |
| @@ -2409,7 +2429,7 @@ func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollReques | |||
| 2409 | 2429 | browser := wv.browser | |
| 2410 | 2430 | wv.mu.RUnlock() | |
| 2411 | 2431 | if browser != nil { | |
| 2412 | - | wv.RunJavaScript(ctx, webutil.BuildScrollAtViewportCenterByJS(request.FallbackDX, request.FallbackDY)) | |
| 2432 | + | wv.enqueuePageScroll(request) | |
| 2413 | 2433 | return nil | |
| 2414 | 2434 | } | |
| 2415 | 2435 | if host != nil { | |
| @@ -2425,6 +2445,112 @@ func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollReques | |||
| 2425 | 2445 | return nil | |
| 2426 | 2446 | } | |
| 2427 | 2447 | ||
| 2448 | + | func (wv *WebView) enqueuePageScroll(request port.PageScrollRequest) { | |
| 2449 | + | q := &wv.pageScrollQueue | |
| 2450 | + | q.mu.Lock() | |
| 2451 | + | if request.Continuous { | |
| 2452 | + | q.heldDX = saturatingPageScrollAdd(q.heldDX, request.FallbackDX) | |
| 2453 | + | q.heldDY = saturatingPageScrollAdd(q.heldDY, request.FallbackDY) | |
| 2454 | + | } else { | |
| 2455 | + | q.tapDX = saturatingPageScrollAdd(q.tapDX, request.FallbackDX) | |
| 2456 | + | q.tapDY = saturatingPageScrollAdd(q.tapDY, request.FallbackDY) | |
| 2457 | + | } | |
| 2458 | + | q.mu.Unlock() | |
| 2459 | + | wv.ensurePageScrollFlush() | |
| 2460 | + | } | |
| 2461 | + | ||
| 2462 | + | func (wv *WebView) ensurePageScrollFlush() { | |
| 2463 | + | q := &wv.pageScrollQueue | |
| 2464 | + | q.mu.Lock() | |
| 2465 | + | if q.flushPending || (q.tapDX == 0 && q.tapDY == 0 && q.heldDX == 0 && q.heldDY == 0) { | |
| 2466 | + | q.mu.Unlock() | |
| 2467 | + | return | |
| 2468 | + | } | |
| 2469 | + | q.flushPending = true | |
| 2470 | + | q.mu.Unlock() | |
| 2471 | + | ||
| 2472 | + | if wv.engine == nil { | |
| 2473 | + | wv.flushPageScroll() | |
| 2474 | + | return | |
| 2475 | + | } | |
| 2476 | + | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | |
| 2477 | + | if task == nil || cefPostTask(purecef.ThreadIDTidUi, task) != 1 { | |
| 2478 | + | q.mu.Lock() | |
| 2479 | + | q.flushPending = false | |
| 2480 | + | q.heldDX = 0 | |
| 2481 | + | q.heldDY = 0 | |
| 2482 | + | q.mu.Unlock() | |
| 2483 | + | } | |
| 2484 | + | } | |
| 2485 | + | ||
| 2486 | + | func (wv *WebView) flushPageScroll() { | |
| 2487 | + | q := &wv.pageScrollQueue | |
| 2488 | + | q.mu.Lock() | |
| 2489 | + | tapDX, tapDY := q.tapDX, q.tapDY | |
| 2490 | + | heldDX, heldDY := q.heldDX, q.heldDY | |
| 2491 | + | heldGeneration := q.generation | |
| 2492 | + | beforeExecute := q.beforeExecute | |
| 2493 | + | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | |
| 2494 | + | q.mu.Unlock() | |
| 2495 | + | ||
| 2496 | + | if beforeExecute != nil { | |
| 2497 | + | beforeExecute() | |
| 2498 | + | } | |
| 2499 | + | ||
| 2500 | + | // commitMu makes the generation check and the start of ExecuteJavaScript | |
| 2501 | + | // linearizable with CancelPageScroll. The queue mutex is released before | |
| 2502 | + | // taking it and before CEF execution, so JavaScript callbacks may enqueue a | |
| 2503 | + | // successor without deadlocking; flushPending still bounds that to one task. | |
| 2504 | + | q.commitMu.Lock() | |
| 2505 | + | q.mu.Lock() | |
| 2506 | + | if heldGeneration != q.generation { | |
| 2507 | + | heldDX, heldDY = 0, 0 | |
| 2508 | + | } | |
| 2509 | + | q.mu.Unlock() | |
| 2510 | + | if dx, dy := tapDX+heldDX, tapDY+heldDY; (dx != 0 || dy != 0) && !wv.destroyed.Load() { | |
| 2511 | + | wv.executeJavaScriptNow(webutil.BuildScrollByJS(dx, dy)) | |
| 2512 | + | } | |
| 2513 | + | q.commitMu.Unlock() | |
| 2514 | + | ||
| 2515 | + | q.mu.Lock() | |
| 2516 | + | q.flushPending = false | |
| 2517 | + | q.mu.Unlock() | |
| 2518 | + | wv.ensurePageScrollFlush() | |
| 2519 | + | } | |
| 2520 | + | ||
| 2521 | + | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | |
| 2522 | + | // Tap deltas remain queued so a quick press always retains its 80/320 px step. | |
| 2523 | + | // Returning is also a barrier: an old held batch cannot start afterward. | |
| 2524 | + | func (wv *WebView) CancelPageScroll(_ context.Context) { | |
| 2525 | + | q := &wv.pageScrollQueue | |
| 2526 | + | q.commitMu.Lock() | |
| 2527 | + | q.mu.Lock() | |
| 2528 | + | q.generation++ | |
| 2529 | + | q.heldDX = 0 | |
| 2530 | + | q.heldDY = 0 | |
| 2531 | + | q.mu.Unlock() | |
| 2532 | + | q.commitMu.Unlock() | |
| 2533 | + | } | |
| 2534 | + | ||
| 2535 | + | func (wv *WebView) resetPageScrollQueue() { | |
| 2536 | + | q := &wv.pageScrollQueue | |
| 2537 | + | q.mu.Lock() | |
| 2538 | + | q.generation++ | |
| 2539 | + | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | |
| 2540 | + | q.mu.Unlock() | |
| 2541 | + | } | |
| 2542 | + | ||
| 2543 | + | func saturatingPageScrollAdd(current, delta int) int { | |
| 2544 | + | result := current + delta | |
| 2545 | + | if result > maxPendingPageScroll { | |
| 2546 | + | return maxPendingPageScroll | |
| 2547 | + | } | |
| 2548 | + | if result < -maxPendingPageScroll { | |
| 2549 | + | return -maxPendingPageScroll | |
| 2550 | + | } | |
| 2551 | + | return result | |
| 2552 | + | } | |
| 2553 | + | ||
| 2428 | 2554 | // beginAudioStreamStart invalidates any prior stream before creating a new | |
| 2429 | 2555 | // one. A stream created for the returned generation may only be published if | |
| 2430 | 2556 | // that generation is still current. | |
| @@ -36,6 +36,7 @@ var ( | |||||
| 36 | _ port.ViewportSyncCapable = (*WebView)(nil) | 36 | _ port.ViewportSyncCapable = (*WebView)(nil) | ||
| 37 | _ port.OAuthCallbackCapable = (*WebView)(nil) | 37 | _ port.OAuthCallbackCapable = (*WebView)(nil) | ||
| 38 | _ port.PageScrollable = (*WebView)(nil) | 38 | _ port.PageScrollable = (*WebView)(nil) | ||
| 39 | + | _ port.PageScrollCanceler = (*WebView)(nil) | |||
| 39 | ) | 40 | ) | ||
| 40 | 41 | ||||
| 41 | // errDestroyed is returned when an operation is attempted on a destroyed WebView. | 42 | // errDestroyed is returned when an operation is attempted on a destroyed WebView. | ||
| @@ -71,8 +72,24 @@ var cefLoadWatchdogDelays = []time.Duration{ | |||||
| 71 | const ( | 72 | const ( | ||
| 72 | cefGTKSyncDispatchTimeout = 2 * time.Second | 73 | cefGTKSyncDispatchTimeout = 2 * time.Second | ||
| 73 | cefGTKSyncDispatchSlowThreshold = 250 * time.Millisecond | 74 | cefGTKSyncDispatchSlowThreshold = 250 * time.Millisecond | ||
| 75 | + | maxPendingPageScroll = 3200 | |||
| 74 | ) | 76 | ) | ||
| 75 | 77 | ||||
| 78 | + | type pageScrollQueue struct { | |||
| 79 | + | mu sync.Mutex | |||
| 80 | + | commitMu sync.Mutex | |||
| 81 | + | tapDX int | |||
| 82 | + | tapDY int | |||
| 83 | + | heldDX int | |||
| 84 | + | heldDY int | |||
| 85 | + | flushPending bool | |||
| 86 | + | generation uint64 | |||
| 87 | + | ||||
| 88 | + | // beforeExecute is a deterministic test seam for the drain-to-commit | |||
| 89 | + | // boundary. Production leaves it nil. | |||
| 90 | + | beforeExecute func() | |||
| 91 | + | } | |||
| 92 | + | ||||
| 76 | var ( | 93 | var ( | ||
| 77 | cefNewTask = purecef.NewTask | 94 | cefNewTask = purecef.NewTask | ||
| 78 | cefNewStringVisitor = purecef.NewStringVisitor | 95 | cefNewStringVisitor = purecef.NewStringVisitor | ||
| @@ -133,6 +150,8 @@ type WebView struct { | |||||
| 133 | adaptiveFrameRatePollID uint | 150 | adaptiveFrameRatePollID uint | ||
| 134 | lastAdaptiveFrameRate int32 | 151 | lastAdaptiveFrameRate int32 | ||
| 135 | 152 | ||||
| 153 | + | pageScrollQueue pageScrollQueue | |||
| 154 | + | ||||
| 136 | // beginFrameTick drives CEF external BeginFrame requests while the GTK | 155 | // beginFrameTick drives CEF external BeginFrame requests while the GTK | ||
| 137 | // widget is visible. Access is guarded by mu. | 156 | // widget is visible. Access is guarded by mu. | ||
| 138 | beginFrameTick *gtk.TickCallback | 157 | beginFrameTick *gtk.TickCallback | ||
| @@ -1203,6 +1222,7 @@ func (wv *WebView) Destroy() { | |||||
| 1203 | if !wv.destroyed.CompareAndSwap(false, true) { | 1222 | if !wv.destroyed.CompareAndSwap(false, true) { | ||
| 1204 | return | 1223 | return | ||
| 1205 | } | 1224 | } | ||
| 1225 | + | wv.resetPageScrollQueue() | |||
| 1206 | wv.syntheticPopupMu.Lock() | 1226 | wv.syntheticPopupMu.Lock() | ||
| 1207 | wv.syntheticPopups = nil | 1227 | wv.syntheticPopups = nil | ||
| 1208 | wv.syntheticPopupMu.Unlock() | 1228 | wv.syntheticPopupMu.Unlock() | ||
| @@ -2409,7 +2429,7 @@ func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollReques | |||||
| 2409 | browser := wv.browser | 2429 | browser := wv.browser | ||
| 2410 | wv.mu.RUnlock() | 2430 | wv.mu.RUnlock() | ||
| 2411 | if browser != nil { | 2431 | if browser != nil { | ||
| 2412 | - | wv.RunJavaScript(ctx, webutil.BuildScrollAtViewportCenterByJS(request.FallbackDX, request.FallbackDY)) | 2432 | + | wv.enqueuePageScroll(request) |
| 2413 | return nil | 2433 | return nil | ||
| 2414 | } | 2434 | } | ||
| 2415 | if host != nil { | 2435 | if host != nil { | ||
| @@ -2425,6 +2445,112 @@ func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollReques | |||||
| 2425 | return nil | 2445 | return nil | ||
| 2426 | } | 2446 | } | ||
| 2427 | 2447 | ||||
| 2448 | + | func (wv *WebView) enqueuePageScroll(request port.PageScrollRequest) { | |||
| 2449 | + | q := &wv.pageScrollQueue | |||
| 2450 | + | q.mu.Lock() | |||
| 2451 | + | if request.Continuous { | |||
| 2452 | + | q.heldDX = saturatingPageScrollAdd(q.heldDX, request.FallbackDX) | |||
| 2453 | + | q.heldDY = saturatingPageScrollAdd(q.heldDY, request.FallbackDY) | |||
| 2454 | + | } else { | |||
| 2455 | + | q.tapDX = saturatingPageScrollAdd(q.tapDX, request.FallbackDX) | |||
| 2456 | + | q.tapDY = saturatingPageScrollAdd(q.tapDY, request.FallbackDY) | |||
| 2457 | + | } | |||
| 2458 | + | q.mu.Unlock() | |||
| 2459 | + | wv.ensurePageScrollFlush() | |||
| 2460 | + | } | |||
| 2461 | + | ||||
| 2462 | + | func (wv *WebView) ensurePageScrollFlush() { | |||
| 2463 | + | q := &wv.pageScrollQueue | |||
| 2464 | + | q.mu.Lock() | |||
| 2465 | + | if q.flushPending || (q.tapDX == 0 && q.tapDY == 0 && q.heldDX == 0 && q.heldDY == 0) { | |||
| 2466 | + | q.mu.Unlock() | |||
| 2467 | + | return | |||
| 2468 | + | } | |||
| 2469 | + | q.flushPending = true | |||
| 2470 | + | q.mu.Unlock() | |||
| 2471 | + | ||||
| 2472 | + | if wv.engine == nil { | |||
| 2473 | + | wv.flushPageScroll() | |||
| 2474 | + | return | |||
| 2475 | + | } | |||
| 2476 | + | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | |||
| 2477 | + | if task == nil || cefPostTask(purecef.ThreadIDTidUi, task) != 1 { | |||
| 2478 | + | q.mu.Lock() | |||
| 2479 | + | q.flushPending = false | |||
| 2480 | + | q.heldDX = 0 | |||
| 2481 | + | q.heldDY = 0 | |||
| 2482 | + | q.mu.Unlock() | |||
| 2483 | + | } | |||
| 2484 | + | } | |||
| 2485 | + | ||||
| 2486 | + | func (wv *WebView) flushPageScroll() { | |||
| 2487 | + | q := &wv.pageScrollQueue | |||
| 2488 | + | q.mu.Lock() | |||
| 2489 | + | tapDX, tapDY := q.tapDX, q.tapDY | |||
| 2490 | + | heldDX, heldDY := q.heldDX, q.heldDY | |||
| 2491 | + | heldGeneration := q.generation | |||
| 2492 | + | beforeExecute := q.beforeExecute | |||
| 2493 | + | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | |||
| 2494 | + | q.mu.Unlock() | |||
| 2495 | + | ||||
| 2496 | + | if beforeExecute != nil { | |||
| 2497 | + | beforeExecute() | |||
| 2498 | + | } | |||
| 2499 | + | ||||
| 2500 | + | // commitMu makes the generation check and the start of ExecuteJavaScript | |||
| 2501 | + | // linearizable with CancelPageScroll. The queue mutex is released before | |||
| 2502 | + | // taking it and before CEF execution, so JavaScript callbacks may enqueue a | |||
| 2503 | + | // successor without deadlocking; flushPending still bounds that to one task. | |||
| 2504 | + | q.commitMu.Lock() | |||
| 2505 | + | q.mu.Lock() | |||
| 2506 | + | if heldGeneration != q.generation { | |||
| 2507 | + | heldDX, heldDY = 0, 0 | |||
| 2508 | + | } | |||
| 2509 | + | q.mu.Unlock() | |||
| 2510 | + | if dx, dy := tapDX+heldDX, tapDY+heldDY; (dx != 0 || dy != 0) && !wv.destroyed.Load() { | |||
| 2511 | + | wv.executeJavaScriptNow(webutil.BuildScrollByJS(dx, dy)) | |||
| 2512 | + | } | |||
| 2513 | + | q.commitMu.Unlock() | |||
| 2514 | + | ||||
| 2515 | + | q.mu.Lock() | |||
| 2516 | + | q.flushPending = false | |||
| 2517 | + | q.mu.Unlock() | |||
| 2518 | + | wv.ensurePageScrollFlush() | |||
| 2519 | + | } | |||
| 2520 | + | ||||
| 2521 | + | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | |||
| 2522 | + | // Tap deltas remain queued so a quick press always retains its 80/320 px step. | |||
| 2523 | + | // Returning is also a barrier: an old held batch cannot start afterward. | |||
| 2524 | + | func (wv *WebView) CancelPageScroll(_ context.Context) { | |||
| 2525 | + | q := &wv.pageScrollQueue | |||
| 2526 | + | q.commitMu.Lock() | |||
| 2527 | + | q.mu.Lock() | |||
| 2528 | + | q.generation++ | |||
| 2529 | + | q.heldDX = 0 | |||
| 2530 | + | q.heldDY = 0 | |||
| 2531 | + | q.mu.Unlock() | |||
| 2532 | + | q.commitMu.Unlock() | |||
| 2533 | + | } | |||
| 2534 | + | ||||
| 2535 | + | func (wv *WebView) resetPageScrollQueue() { | |||
| 2536 | + | q := &wv.pageScrollQueue | |||
| 2537 | + | q.mu.Lock() | |||
| 2538 | + | q.generation++ | |||
| 2539 | + | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | |||
| 2540 | + | q.mu.Unlock() | |||
| 2541 | + | } | |||
| 2542 | + | ||||
| 2543 | + | func saturatingPageScrollAdd(current, delta int) int { | |||
| 2544 | + | result := current + delta | |||
| 2545 | + | if result > maxPendingPageScroll { | |||
| 2546 | + | return maxPendingPageScroll | |||
| 2547 | + | } | |||
| 2548 | + | if result < -maxPendingPageScroll { | |||
| 2549 | + | return -maxPendingPageScroll | |||
| 2550 | + | } | |||
| 2551 | + | return result | |||
| 2552 | + | } | |||
| 2553 | + | ||||
| 2428 | // beginAudioStreamStart invalidates any prior stream before creating a new | 2554 | // beginAudioStreamStart invalidates any prior stream before creating a new | ||
| 2429 | // one. A stream created for the returned generation may only be published if | 2555 | // one. A stream created for the returned generation may only be published if | ||
| 2430 | // that generation is still current. | 2556 | // that generation is still current. | ||
M internal/infrastructure/cef/webview_scroll_test.go +198 -0
| @@ -248,6 +248,204 @@ func TestScrollPage_NativePath_FallbackToJSWhenHostIsNil(t *testing.T) { | |||
| 248 | 248 | } | |
| 249 | 249 | } | |
| 250 | 250 | ||
| 251 | + | func TestScrollPage_CoalescesHeldLoadIntoOneBoundedCEFTask(t *testing.T) { | |
| 252 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |
| 253 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |
| 254 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 255 | + | var scheduled purecef.Task | |
| 256 | + | posts := 0 | |
| 257 | + | cefPostTask = func(threadID purecef.ThreadID, task purecef.Task) int32 { | |
| 258 | + | if threadID != purecef.ThreadIDTidUi { | |
| 259 | + | t.Fatalf("thread=%v, want CEF UI", threadID) | |
| 260 | + | } | |
| 261 | + | posts++ | |
| 262 | + | scheduled = task | |
| 263 | + | return 1 | |
| 264 | + | } | |
| 265 | + | ||
| 266 | + | browser := cefmocks.NewMockBrowser(t) | |
| 267 | + | frame := cefmocks.NewMockFrame(t) | |
| 268 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |
| 269 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |
| 270 | + | return strings.Contains(script, "var dx=0,dy=3200") | |
| 271 | + | }), "", int32(0)).Once() | |
| 272 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |
| 273 | + | ||
| 274 | + | for range 10_000 { | |
| 275 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |
| 276 | + | t.Fatal(err) | |
| 277 | + | } | |
| 278 | + | } | |
| 279 | + | if posts != 1 { | |
| 280 | + | t.Fatalf("CEF flush posts=%d, want exactly 1 under load", posts) | |
| 281 | + | } | |
| 282 | + | if scheduled == nil { | |
| 283 | + | t.Fatal("missing scheduled flush") | |
| 284 | + | } | |
| 285 | + | scheduled.Execute() | |
| 286 | + | if posts != 1 { | |
| 287 | + | t.Fatalf("unexpected replay after drain: posts=%d", posts) | |
| 288 | + | } | |
| 289 | + | } | |
| 290 | + | ||
| 291 | + | func TestCancelPageScroll_DropsHeldDeltaButPreservesTap(t *testing.T) { | |
| 292 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |
| 293 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |
| 294 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 295 | + | var scheduled purecef.Task | |
| 296 | + | posts := 0 | |
| 297 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |
| 298 | + | posts++ | |
| 299 | + | scheduled = task | |
| 300 | + | return 1 | |
| 301 | + | } | |
| 302 | + | ||
| 303 | + | browser := cefmocks.NewMockBrowser(t) | |
| 304 | + | frame := cefmocks.NewMockFrame(t) | |
| 305 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |
| 306 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |
| 307 | + | return strings.Contains(script, "var dx=0,dy=80") && !strings.Contains(script, "requestAnimationFrame") | |
| 308 | + | }), "", int32(0)).Once() | |
| 309 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |
| 310 | + | ||
| 311 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |
| 312 | + | t.Fatal(err) | |
| 313 | + | } | |
| 314 | + | for range 50 { | |
| 315 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |
| 316 | + | t.Fatal(err) | |
| 317 | + | } | |
| 318 | + | } | |
| 319 | + | wv.CancelPageScroll(context.Background()) | |
| 320 | + | if posts != 1 { | |
| 321 | + | t.Fatalf("posts before release=%d, want 1", posts) | |
| 322 | + | } | |
| 323 | + | scheduled.Execute() | |
| 324 | + | if posts != 1 { | |
| 325 | + | t.Fatalf("release must not schedule a trailing replay: posts=%d", posts) | |
| 326 | + | } | |
| 327 | + | } | |
| 328 | + | ||
| 329 | + | func TestCancelPageScroll_AfterDrainDropsOldHeldAndPreservesTapAndNextGesture(t *testing.T) { | |
| 330 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |
| 331 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |
| 332 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 333 | + | var scheduled []purecef.Task | |
| 334 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |
| 335 | + | scheduled = append(scheduled, task) | |
| 336 | + | return 1 | |
| 337 | + | } | |
| 338 | + | ||
| 339 | + | drained := make(chan struct{}) | |
| 340 | + | resume := make(chan struct{}) | |
| 341 | + | var pauseOnce sync.Once | |
| 342 | + | beforeExecute := func() { | |
| 343 | + | pauseOnce.Do(func() { | |
| 344 | + | close(drained) | |
| 345 | + | <-resume | |
| 346 | + | }) | |
| 347 | + | } | |
| 348 | + | ||
| 349 | + | browser := cefmocks.NewMockBrowser(t) | |
| 350 | + | frame := cefmocks.NewMockFrame(t) | |
| 351 | + | browser.EXPECT().GetMainFrame().Return(frame).Twice() | |
| 352 | + | var scripts []string | |
| 353 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(script, _ string, _ int32) { | |
| 354 | + | scripts = append(scripts, script) | |
| 355 | + | }).Twice() | |
| 356 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |
| 357 | + | wv.pageScrollQueue.beforeExecute = beforeExecute | |
| 358 | + | ||
| 359 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |
| 360 | + | t.Fatal(err) | |
| 361 | + | } | |
| 362 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |
| 363 | + | t.Fatal(err) | |
| 364 | + | } | |
| 365 | + | if len(scheduled) != 1 { | |
| 366 | + | t.Fatalf("initial pending tasks=%d, want 1", len(scheduled)) | |
| 367 | + | } | |
| 368 | + | ||
| 369 | + | flushDone := make(chan struct{}) | |
| 370 | + | go func() { | |
| 371 | + | defer close(flushDone) | |
| 372 | + | scheduled[0].Execute() | |
| 373 | + | }() | |
| 374 | + | <-drained | |
| 375 | + | ||
| 376 | + | // Cancel returns while the old batch is drained but has not committed to | |
| 377 | + | // ExecuteJavaScript. Its held component must therefore be invalidated. | |
| 378 | + | wv.CancelPageScroll(context.Background()) | |
| 379 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDX: 80}); err != nil { | |
| 380 | + | t.Fatal(err) | |
| 381 | + | } | |
| 382 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDX: 80, Continuous: true}); err != nil { | |
| 383 | + | t.Fatal(err) | |
| 384 | + | } | |
| 385 | + | if len(scheduled) != 1 { | |
| 386 | + | t.Fatalf("tasks while old flush is paused=%d, want 1", len(scheduled)) | |
| 387 | + | } | |
| 388 | + | ||
| 389 | + | close(resume) | |
| 390 | + | <-flushDone | |
| 391 | + | if len(scripts) != 1 || !strings.Contains(scripts[0], "var dx=0,dy=80") { | |
| 392 | + | t.Fatalf("old generation scripts=%v, want the preserved 80px tap without held delta", scripts) | |
| 393 | + | } | |
| 394 | + | if len(scheduled) != 2 { | |
| 395 | + | t.Fatalf("tasks after old flush=%d, want one successor for the new gesture", len(scheduled)) | |
| 396 | + | } | |
| 397 | + | scheduled[1].Execute() | |
| 398 | + | if len(scripts) != 2 || !strings.Contains(scripts[1], "var dx=160,dy=0") { | |
| 399 | + | t.Fatalf("scripts after new gesture=%v, want a fresh 160px horizontal batch", scripts) | |
| 400 | + | } | |
| 401 | + | if len(scheduled) != 2 { | |
| 402 | + | t.Fatalf("trailing tasks=%d, want 2 total", len(scheduled)) | |
| 403 | + | } | |
| 404 | + | } | |
| 405 | + | ||
| 406 | + | func TestScrollPage_AllowsAtMostOneSuccessorAfterInFlightFlush(t *testing.T) { | |
| 407 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |
| 408 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |
| 409 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 410 | + | var scheduled []purecef.Task | |
| 411 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |
| 412 | + | scheduled = append(scheduled, task) | |
| 413 | + | return 1 | |
| 414 | + | } | |
| 415 | + | ||
| 416 | + | browser := cefmocks.NewMockBrowser(t) | |
| 417 | + | frame := cefmocks.NewMockFrame(t) | |
| 418 | + | browser.EXPECT().GetMainFrame().Return(frame).Twice() | |
| 419 | + | var wv *WebView | |
| 420 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(string, string, int32) { | |
| 421 | + | for range 100 { | |
| 422 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |
| 423 | + | t.Fatal(err) | |
| 424 | + | } | |
| 425 | + | } | |
| 426 | + | if len(scheduled) != 1 { | |
| 427 | + | t.Fatalf("tasks while flush in flight=%d, want 1", len(scheduled)) | |
| 428 | + | } | |
| 429 | + | }).Once() | |
| 430 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Once() | |
| 431 | + | wv = &WebView{engine: &Engine{}, browser: browser} | |
| 432 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |
| 433 | + | t.Fatal(err) | |
| 434 | + | } | |
| 435 | + | if len(scheduled) != 1 { | |
| 436 | + | t.Fatalf("initial pending tasks=%d, want 1", len(scheduled)) | |
| 437 | + | } | |
| 438 | + | ||
| 439 | + | scheduled[0].Execute() | |
| 440 | + | if len(scheduled) != 2 { | |
| 441 | + | t.Fatalf("successor tasks=%d, want one successor", len(scheduled)) | |
| 442 | + | } | |
| 443 | + | scheduled[1].Execute() | |
| 444 | + | if len(scheduled) != 2 { | |
| 445 | + | t.Fatalf("prolonged replay tasks=%d, want 2 total", len(scheduled)) | |
| 446 | + | } | |
| 447 | + | } | |
| 448 | + | ||
| 251 | 449 | func TestScrollPage_Destroyed_ReturnsError(t *testing.T) { | |
| 252 | 450 | wv := &WebView{} | |
| 253 | 451 | wv.destroyed.Store(true) | |
| @@ -248,6 +248,204 @@ func TestScrollPage_NativePath_FallbackToJSWhenHostIsNil(t *testing.T) { | |||||
| 248 | } | 248 | } | ||
| 249 | } | 249 | } | ||
| 250 | 250 | ||||
| 251 | + | func TestScrollPage_CoalescesHeldLoadIntoOneBoundedCEFTask(t *testing.T) { | |||
| 252 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |||
| 253 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |||
| 254 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 255 | + | var scheduled purecef.Task | |||
| 256 | + | posts := 0 | |||
| 257 | + | cefPostTask = func(threadID purecef.ThreadID, task purecef.Task) int32 { | |||
| 258 | + | if threadID != purecef.ThreadIDTidUi { | |||
| 259 | + | t.Fatalf("thread=%v, want CEF UI", threadID) | |||
| 260 | + | } | |||
| 261 | + | posts++ | |||
| 262 | + | scheduled = task | |||
| 263 | + | return 1 | |||
| 264 | + | } | |||
| 265 | + | ||||
| 266 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 267 | + | frame := cefmocks.NewMockFrame(t) | |||
| 268 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |||
| 269 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |||
| 270 | + | return strings.Contains(script, "var dx=0,dy=3200") | |||
| 271 | + | }), "", int32(0)).Once() | |||
| 272 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |||
| 273 | + | ||||
| 274 | + | for range 10_000 { | |||
| 275 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |||
| 276 | + | t.Fatal(err) | |||
| 277 | + | } | |||
| 278 | + | } | |||
| 279 | + | if posts != 1 { | |||
| 280 | + | t.Fatalf("CEF flush posts=%d, want exactly 1 under load", posts) | |||
| 281 | + | } | |||
| 282 | + | if scheduled == nil { | |||
| 283 | + | t.Fatal("missing scheduled flush") | |||
| 284 | + | } | |||
| 285 | + | scheduled.Execute() | |||
| 286 | + | if posts != 1 { | |||
| 287 | + | t.Fatalf("unexpected replay after drain: posts=%d", posts) | |||
| 288 | + | } | |||
| 289 | + | } | |||
| 290 | + | ||||
| 291 | + | func TestCancelPageScroll_DropsHeldDeltaButPreservesTap(t *testing.T) { | |||
| 292 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |||
| 293 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |||
| 294 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 295 | + | var scheduled purecef.Task | |||
| 296 | + | posts := 0 | |||
| 297 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |||
| 298 | + | posts++ | |||
| 299 | + | scheduled = task | |||
| 300 | + | return 1 | |||
| 301 | + | } | |||
| 302 | + | ||||
| 303 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 304 | + | frame := cefmocks.NewMockFrame(t) | |||
| 305 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |||
| 306 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |||
| 307 | + | return strings.Contains(script, "var dx=0,dy=80") && !strings.Contains(script, "requestAnimationFrame") | |||
| 308 | + | }), "", int32(0)).Once() | |||
| 309 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |||
| 310 | + | ||||
| 311 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |||
| 312 | + | t.Fatal(err) | |||
| 313 | + | } | |||
| 314 | + | for range 50 { | |||
| 315 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |||
| 316 | + | t.Fatal(err) | |||
| 317 | + | } | |||
| 318 | + | } | |||
| 319 | + | wv.CancelPageScroll(context.Background()) | |||
| 320 | + | if posts != 1 { | |||
| 321 | + | t.Fatalf("posts before release=%d, want 1", posts) | |||
| 322 | + | } | |||
| 323 | + | scheduled.Execute() | |||
| 324 | + | if posts != 1 { | |||
| 325 | + | t.Fatalf("release must not schedule a trailing replay: posts=%d", posts) | |||
| 326 | + | } | |||
| 327 | + | } | |||
| 328 | + | ||||
| 329 | + | func TestCancelPageScroll_AfterDrainDropsOldHeldAndPreservesTapAndNextGesture(t *testing.T) { | |||
| 330 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |||
| 331 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |||
| 332 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 333 | + | var scheduled []purecef.Task | |||
| 334 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |||
| 335 | + | scheduled = append(scheduled, task) | |||
| 336 | + | return 1 | |||
| 337 | + | } | |||
| 338 | + | ||||
| 339 | + | drained := make(chan struct{}) | |||
| 340 | + | resume := make(chan struct{}) | |||
| 341 | + | var pauseOnce sync.Once | |||
| 342 | + | beforeExecute := func() { | |||
| 343 | + | pauseOnce.Do(func() { | |||
| 344 | + | close(drained) | |||
| 345 | + | <-resume | |||
| 346 | + | }) | |||
| 347 | + | } | |||
| 348 | + | ||||
| 349 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 350 | + | frame := cefmocks.NewMockFrame(t) | |||
| 351 | + | browser.EXPECT().GetMainFrame().Return(frame).Twice() | |||
| 352 | + | var scripts []string | |||
| 353 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(script, _ string, _ int32) { | |||
| 354 | + | scripts = append(scripts, script) | |||
| 355 | + | }).Twice() | |||
| 356 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |||
| 357 | + | wv.pageScrollQueue.beforeExecute = beforeExecute | |||
| 358 | + | ||||
| 359 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |||
| 360 | + | t.Fatal(err) | |||
| 361 | + | } | |||
| 362 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |||
| 363 | + | t.Fatal(err) | |||
| 364 | + | } | |||
| 365 | + | if len(scheduled) != 1 { | |||
| 366 | + | t.Fatalf("initial pending tasks=%d, want 1", len(scheduled)) | |||
| 367 | + | } | |||
| 368 | + | ||||
| 369 | + | flushDone := make(chan struct{}) | |||
| 370 | + | go func() { | |||
| 371 | + | defer close(flushDone) | |||
| 372 | + | scheduled[0].Execute() | |||
| 373 | + | }() | |||
| 374 | + | <-drained | |||
| 375 | + | ||||
| 376 | + | // Cancel returns while the old batch is drained but has not committed to | |||
| 377 | + | // ExecuteJavaScript. Its held component must therefore be invalidated. | |||
| 378 | + | wv.CancelPageScroll(context.Background()) | |||
| 379 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDX: 80}); err != nil { | |||
| 380 | + | t.Fatal(err) | |||
| 381 | + | } | |||
| 382 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDX: 80, Continuous: true}); err != nil { | |||
| 383 | + | t.Fatal(err) | |||
| 384 | + | } | |||
| 385 | + | if len(scheduled) != 1 { | |||
| 386 | + | t.Fatalf("tasks while old flush is paused=%d, want 1", len(scheduled)) | |||
| 387 | + | } | |||
| 388 | + | ||||
| 389 | + | close(resume) | |||
| 390 | + | <-flushDone | |||
| 391 | + | if len(scripts) != 1 || !strings.Contains(scripts[0], "var dx=0,dy=80") { | |||
| 392 | + | t.Fatalf("old generation scripts=%v, want the preserved 80px tap without held delta", scripts) | |||
| 393 | + | } | |||
| 394 | + | if len(scheduled) != 2 { | |||
| 395 | + | t.Fatalf("tasks after old flush=%d, want one successor for the new gesture", len(scheduled)) | |||
| 396 | + | } | |||
| 397 | + | scheduled[1].Execute() | |||
| 398 | + | if len(scripts) != 2 || !strings.Contains(scripts[1], "var dx=160,dy=0") { | |||
| 399 | + | t.Fatalf("scripts after new gesture=%v, want a fresh 160px horizontal batch", scripts) | |||
| 400 | + | } | |||
| 401 | + | if len(scheduled) != 2 { | |||
| 402 | + | t.Fatalf("trailing tasks=%d, want 2 total", len(scheduled)) | |||
| 403 | + | } | |||
| 404 | + | } | |||
| 405 | + | ||||
| 406 | + | func TestScrollPage_AllowsAtMostOneSuccessorAfterInFlightFlush(t *testing.T) { | |||
| 407 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |||
| 408 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |||
| 409 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 410 | + | var scheduled []purecef.Task | |||
| 411 | + | cefPostTask = func(_ purecef.ThreadID, task purecef.Task) int32 { | |||
| 412 | + | scheduled = append(scheduled, task) | |||
| 413 | + | return 1 | |||
| 414 | + | } | |||
| 415 | + | ||||
| 416 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 417 | + | frame := cefmocks.NewMockFrame(t) | |||
| 418 | + | browser.EXPECT().GetMainFrame().Return(frame).Twice() | |||
| 419 | + | var wv *WebView | |||
| 420 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Run(func(string, string, int32) { | |||
| 421 | + | for range 100 { | |||
| 422 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |||
| 423 | + | t.Fatal(err) | |||
| 424 | + | } | |||
| 425 | + | } | |||
| 426 | + | if len(scheduled) != 1 { | |||
| 427 | + | t.Fatalf("tasks while flush in flight=%d, want 1", len(scheduled)) | |||
| 428 | + | } | |||
| 429 | + | }).Once() | |||
| 430 | + | frame.EXPECT().ExecuteJavaScript(mock.Anything, "", int32(0)).Once() | |||
| 431 | + | wv = &WebView{engine: &Engine{}, browser: browser} | |||
| 432 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |||
| 433 | + | t.Fatal(err) | |||
| 434 | + | } | |||
| 435 | + | if len(scheduled) != 1 { | |||
| 436 | + | t.Fatalf("initial pending tasks=%d, want 1", len(scheduled)) | |||
| 437 | + | } | |||
| 438 | + | ||||
| 439 | + | scheduled[0].Execute() | |||
| 440 | + | if len(scheduled) != 2 { | |||
| 441 | + | t.Fatalf("successor tasks=%d, want one successor", len(scheduled)) | |||
| 442 | + | } | |||
| 443 | + | scheduled[1].Execute() | |||
| 444 | + | if len(scheduled) != 2 { | |||
| 445 | + | t.Fatalf("prolonged replay tasks=%d, want 2 total", len(scheduled)) | |||
| 446 | + | } | |||
| 447 | + | } | |||
| 448 | + | ||||
| 251 | func TestScrollPage_Destroyed_ReturnsError(t *testing.T) { | 449 | func TestScrollPage_Destroyed_ReturnsError(t *testing.T) { | ||
| 252 | wv := &WebView{} | 450 | wv := &WebView{} | ||
| 253 | wv.destroyed.Store(true) | 451 | wv.destroyed.Store(true) | ||
M internal/infrastructure/webkit/webview.go +3 -2
| @@ -2415,8 +2415,9 @@ func (wv *WebView) AttachFrontend(ctx context.Context, injector *ContentInjector | |||
| 2415 | 2415 | return nil | |
| 2416 | 2416 | } | |
| 2417 | 2417 | ||
| 2418 | - | // ScrollPage scrolls the page using a semantic page-scroll request. | |
| 2419 | - | // Currently uses the JS fallback with request fallback deltas. | |
| 2418 | + | // ScrollPage scrolls from the viewport center, handing exhausted nested | |
| 2419 | + | // scrollers off to their ancestors and then the document. CEF uses the same | |
| 2420 | + | // shared directional DOM-target resolution once its browser is ready. | |
| 2420 | 2421 | // Implements port.PageScrollable. | |
| 2421 | 2422 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | |
| 2422 | 2423 | if wv.destroyed.Load() { | |
| @@ -2415,8 +2415,9 @@ func (wv *WebView) AttachFrontend(ctx context.Context, injector *ContentInjector | |||||
| 2415 | return nil | 2415 | return nil | ||
| 2416 | } | 2416 | } | ||
| 2417 | 2417 | ||||
| 2418 | - | // ScrollPage scrolls the page using a semantic page-scroll request. | 2418 | + | // ScrollPage scrolls from the viewport center, handing exhausted nested |
| 2419 | - | // Currently uses the JS fallback with request fallback deltas. | 2419 | + | // scrollers off to their ancestors and then the document. CEF uses the same |
| 2420 | + | // shared directional DOM-target resolution once its browser is ready. | |||
| 2420 | // Implements port.PageScrollable. | 2421 | // Implements port.PageScrollable. | ||
| 2421 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | 2422 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | ||
| 2422 | if wv.destroyed.Load() { | 2423 | if wv.destroyed.Load() { | ||
M internal/infrastructure/webutil/pagescroll.go +11 -33
| @@ -2,39 +2,16 @@ package webutil | |||
| 2 | 2 | ||
| 3 | 3 | import "fmt" | |
| 4 | 4 | ||
| 5 | - | // BuildScrollByJS returns a JavaScript string that scrolls a web page by the | |
| 6 | - | // given CSS-pixel delta. | |
| 5 | + | // BuildScrollByJS returns one immediate JavaScript scroll step for the given | |
| 6 | + | // CSS-pixel delta. It resolves the initial target under the viewport center, | |
| 7 | + | // walks toward scrollable ancestors that can move in the requested direction, | |
| 8 | + | // and hands off to the document when a nested scroller reaches its boundary. | |
| 9 | + | // Cross-origin frame contents remain best-effort because elementFromPoint can | |
| 10 | + | // only return the accessible frame element from the top-level document. | |
| 7 | 11 | // | |
| 8 | - | // THIS IS A FALLBACK IMPLEMENTATION. The primary page-scroll abstraction is | |
| 9 | - | // port.PageScrollable.ScrollPage. Engines should attempt native scrolling | |
| 10 | - | // first and call this function only when no native mechanism exists for the | |
| 11 | - | // requested command. | |
| 12 | - | // | |
| 13 | - | // Semantics (frontend scroll-target resolution): | |
| 14 | - | // 1. Start from document.activeElement. | |
| 15 | - | // 2. Walk up the DOM tree to find the nearest scrollable ancestor that can | |
| 16 | - | // still scroll in the requested direction. | |
| 17 | - | // 3. Fall back to document.scrollingElement (or documentElement). | |
| 18 | - | // 4. Fall back to window scrolling. | |
| 19 | - | // | |
| 20 | - | // The app-level Page Mode repeater now owns held-key cadence. This helper | |
| 21 | - | // performs one immediate fallback scroll step for each semantic Page Mode tick | |
| 22 | - | // instead of maintaining its own RAF-based repeat loop. | |
| 12 | + | // The app-level Page Mode repeater owns held-key cadence; this helper performs | |
| 13 | + | // no requestAnimationFrame loop of its own. | |
| 23 | 14 | func BuildScrollByJS(dx, dy int) string { | |
| 24 | - | return buildScrollByJS(dx, dy, "doc.activeElement") | |
| 25 | - | } | |
| 26 | - | ||
| 27 | - | // BuildScrollAtViewportCenterByJS returns one immediate scroll step whose DOM | |
| 28 | - | // target starts under the viewport center. It is used by keyboard-driven CEF | |
| 29 | - | // scrolling to preserve the native wheel path's hit-test location while | |
| 30 | - | // allowing exhausted nested scrollers to hand off to the document. | |
| 31 | - | func BuildScrollAtViewportCenterByJS(dx, dy int) string { | |
| 32 | - | startNode := "(typeof doc.elementFromPoint==='function'?" + | |
| 33 | - | "doc.elementFromPoint(window.innerWidth/2,window.innerHeight/2):null)||doc.activeElement" | |
| 34 | - | return buildScrollByJS(dx, dy, startNode) | |
| 35 | - | } | |
| 36 | - | ||
| 37 | - | func buildScrollByJS(dx, dy int, startNode string) string { | |
| 38 | 15 | return fmt.Sprintf(`(function(){ | |
| 39 | 16 | var dx=%d,dy=%d,doc=document; | |
| 40 | 17 | function hasScrollableOverflow(value){ | |
| @@ -71,7 +48,8 @@ function scrollElement(el){ | |||
| 71 | 48 | return el.scrollLeft!==beforeLeft||el.scrollTop!==beforeTop; | |
| 72 | 49 | } | |
| 73 | 50 | try{ | |
| 74 | - | var node=%s; | |
| 51 | + | var node=typeof doc.elementFromPoint==='function'? | |
| 52 | + | doc.elementFromPoint(window.innerWidth/2,window.innerHeight/2):null; | |
| 75 | 53 | while(node&&node!==doc.body&&node!==doc.documentElement){ | |
| 76 | 54 | if(canScroll(node)&&scrollElement(node))return; | |
| 77 | 55 | node=node.parentElement; | |
| @@ -86,5 +64,5 @@ try{ | |||
| 86 | 64 | window.scrollTo(window.scrollX+dx,window.scrollY+dy); | |
| 87 | 65 | } | |
| 88 | 66 | }catch(e){} | |
| 89 | - | })()`, dx, dy, startNode) | |
| 67 | + | })()`, dx, dy) | |
| 90 | 68 | } | |
| @@ -2,39 +2,16 @@ package webutil | |||||
| 2 | 2 | ||||
| 3 | import "fmt" | 3 | import "fmt" | ||
| 4 | 4 | ||||
| 5 | - | // BuildScrollByJS returns a JavaScript string that scrolls a web page by the | 5 | + | // BuildScrollByJS returns one immediate JavaScript scroll step for the given |
| 6 | - | // given CSS-pixel delta. | 6 | + | // CSS-pixel delta. It resolves the initial target under the viewport center, |
| 7 | + | // walks toward scrollable ancestors that can move in the requested direction, | |||
| 8 | + | // and hands off to the document when a nested scroller reaches its boundary. | |||
| 9 | + | // Cross-origin frame contents remain best-effort because elementFromPoint can | |||
| 10 | + | // only return the accessible frame element from the top-level document. | |||
| 7 | // | 11 | // | ||
| 8 | - | // THIS IS A FALLBACK IMPLEMENTATION. The primary page-scroll abstraction is | 12 | + | // The app-level Page Mode repeater owns held-key cadence; this helper performs |
| 9 | - | // port.PageScrollable.ScrollPage. Engines should attempt native scrolling | 13 | + | // no requestAnimationFrame loop of its own. |
| 10 | - | // first and call this function only when no native mechanism exists for the | |||
| 11 | - | // requested command. | |||
| 12 | - | // | |||
| 13 | - | // Semantics (frontend scroll-target resolution): | |||
| 14 | - | // 1. Start from document.activeElement. | |||
| 15 | - | // 2. Walk up the DOM tree to find the nearest scrollable ancestor that can | |||
| 16 | - | // still scroll in the requested direction. | |||
| 17 | - | // 3. Fall back to document.scrollingElement (or documentElement). | |||
| 18 | - | // 4. Fall back to window scrolling. | |||
| 19 | - | // | |||
| 20 | - | // The app-level Page Mode repeater now owns held-key cadence. This helper | |||
| 21 | - | // performs one immediate fallback scroll step for each semantic Page Mode tick | |||
| 22 | - | // instead of maintaining its own RAF-based repeat loop. | |||
| 23 | func BuildScrollByJS(dx, dy int) string { | 14 | func BuildScrollByJS(dx, dy int) string { | ||
| 24 | - | return buildScrollByJS(dx, dy, "doc.activeElement") | |||
| 25 | - | } | |||
| 26 | - | ||||
| 27 | - | // BuildScrollAtViewportCenterByJS returns one immediate scroll step whose DOM | |||
| 28 | - | // target starts under the viewport center. It is used by keyboard-driven CEF | |||
| 29 | - | // scrolling to preserve the native wheel path's hit-test location while | |||
| 30 | - | // allowing exhausted nested scrollers to hand off to the document. | |||
| 31 | - | func BuildScrollAtViewportCenterByJS(dx, dy int) string { | |||
| 32 | - | startNode := "(typeof doc.elementFromPoint==='function'?" + | |||
| 33 | - | "doc.elementFromPoint(window.innerWidth/2,window.innerHeight/2):null)||doc.activeElement" | |||
| 34 | - | return buildScrollByJS(dx, dy, startNode) | |||
| 35 | - | } | |||
| 36 | - | ||||
| 37 | - | func buildScrollByJS(dx, dy int, startNode string) string { | |||
| 38 | return fmt.Sprintf(`(function(){ | 15 | return fmt.Sprintf(`(function(){ | ||
| 39 | var dx=%d,dy=%d,doc=document; | 16 | var dx=%d,dy=%d,doc=document; | ||
| 40 | function hasScrollableOverflow(value){ | 17 | function hasScrollableOverflow(value){ | ||
| @@ -71,7 +48,8 @@ function scrollElement(el){ | |||||
| 71 | return el.scrollLeft!==beforeLeft||el.scrollTop!==beforeTop; | 48 | return el.scrollLeft!==beforeLeft||el.scrollTop!==beforeTop; | ||
| 72 | } | 49 | } | ||
| 73 | try{ | 50 | try{ | ||
| 74 | - | var node=%s; | 51 | + | var node=typeof doc.elementFromPoint==='function'? |
| 52 | + | doc.elementFromPoint(window.innerWidth/2,window.innerHeight/2):null; | |||
| 75 | while(node&&node!==doc.body&&node!==doc.documentElement){ | 53 | while(node&&node!==doc.body&&node!==doc.documentElement){ | ||
| 76 | if(canScroll(node)&&scrollElement(node))return; | 54 | if(canScroll(node)&&scrollElement(node))return; | ||
| 77 | node=node.parentElement; | 55 | node=node.parentElement; | ||
| @@ -86,5 +64,5 @@ try{ | |||||
| 86 | window.scrollTo(window.scrollX+dx,window.scrollY+dy); | 64 | window.scrollTo(window.scrollX+dx,window.scrollY+dy); | ||
| 87 | } | 65 | } | ||
| 88 | }catch(e){} | 66 | }catch(e){} | ||
| 89 | - | })()`, dx, dy, startNode) | 67 | + | })()`, dx, dy) |
| 90 | } | 68 | } | ||
M internal/infrastructure/webutil/pagescroll_test.go +12 -1
| @@ -5,6 +5,17 @@ import ( | |||
| 5 | 5 | "testing" | |
| 6 | 6 | ) | |
| 7 | 7 | ||
| 8 | + | func TestBuildScrollByJS_TargetsViewportCenterInsteadOfActiveElement(t *testing.T) { | |
| 9 | + | js := BuildScrollByJS(0, 80) | |
| 10 | + | ||
| 11 | + | if !strings.Contains(js, "doc.elementFromPoint(window.innerWidth/2,window.innerHeight/2)") { | |
| 12 | + | t.Fatalf("page scroll must resolve its initial target from the viewport center, got: %s", js) | |
| 13 | + | } | |
| 14 | + | if strings.Contains(js, "activeElement") { | |
| 15 | + | t.Fatalf("page scroll must not use the legacy active-element target, got: %s", js) | |
| 16 | + | } | |
| 17 | + | } | |
| 18 | + | ||
| 8 | 19 | func TestBuildScrollByJS_ContainsRequiredOperations(t *testing.T) { | |
| 9 | 20 | tests := []struct { | |
| 10 | 21 | name string | |
| @@ -22,7 +33,7 @@ func TestBuildScrollByJS_ContainsRequiredOperations(t *testing.T) { | |||
| 22 | 33 | message string | |
| 23 | 34 | snippets []string | |
| 24 | 35 | }{ | |
| 25 | - | {"JS must reference document.activeElement", []string{"activeElement"}}, | |
| 36 | + | {"JS must resolve the viewport-center element", []string{"elementFromPoint", "window.innerWidth/2", "window.innerHeight/2"}}, | |
| 26 | 37 | {"JS must walk parentElement in a loop", []string{"parentElement"}}, | |
| 27 | 38 | {"JS must reference document.scrollingElement", []string{"scrollingElement"}}, | |
| 28 | 39 | {"JS must fall back to window.scrollBy", []string{"window.scrollBy"}}, | |
| @@ -5,6 +5,17 @@ import ( | |||||
| 5 | "testing" | 5 | "testing" | ||
| 6 | ) | 6 | ) | ||
| 7 | 7 | ||||
| 8 | + | func TestBuildScrollByJS_TargetsViewportCenterInsteadOfActiveElement(t *testing.T) { | |||
| 9 | + | js := BuildScrollByJS(0, 80) | |||
| 10 | + | ||||
| 11 | + | if !strings.Contains(js, "doc.elementFromPoint(window.innerWidth/2,window.innerHeight/2)") { | |||
| 12 | + | t.Fatalf("page scroll must resolve its initial target from the viewport center, got: %s", js) | |||
| 13 | + | } | |||
| 14 | + | if strings.Contains(js, "activeElement") { | |||
| 15 | + | t.Fatalf("page scroll must not use the legacy active-element target, got: %s", js) | |||
| 16 | + | } | |||
| 17 | + | } | |||
| 18 | + | ||||
| 8 | func TestBuildScrollByJS_ContainsRequiredOperations(t *testing.T) { | 19 | func TestBuildScrollByJS_ContainsRequiredOperations(t *testing.T) { | ||
| 9 | tests := []struct { | 20 | tests := []struct { | ||
| 10 | name string | 21 | name string | ||
| @@ -22,7 +33,7 @@ func TestBuildScrollByJS_ContainsRequiredOperations(t *testing.T) { | |||||
| 22 | message string | 33 | message string | ||
| 23 | snippets []string | 34 | snippets []string | ||
| 24 | }{ | 35 | }{ | ||
| 25 | - | {"JS must reference document.activeElement", []string{"activeElement"}}, | 36 | + | {"JS must resolve the viewport-center element", []string{"elementFromPoint", "window.innerWidth/2", "window.innerHeight/2"}}, |
| 26 | {"JS must walk parentElement in a loop", []string{"parentElement"}}, | 37 | {"JS must walk parentElement in a loop", []string{"parentElement"}}, | ||
| 27 | {"JS must reference document.scrollingElement", []string{"scrollingElement"}}, | 38 | {"JS must reference document.scrollingElement", []string{"scrollingElement"}}, | ||
| 28 | {"JS must fall back to window.scrollBy", []string{"window.scrollBy"}}, | 39 | {"JS must fall back to window.scrollBy", []string{"window.scrollBy"}}, | ||
M internal/ui/app.go +4 -0
| @@ -1088,6 +1088,10 @@ func (a *App) initBrowserWindowInput(ctx context.Context, bw *browserWindow) { | |||
| 1088 | 1088 | } | |
| 1089 | 1089 | return a.dispatchBrowserWindowAction(actionCtx, bw, action) | |
| 1090 | 1090 | }) | |
| 1091 | + | bw.keyboardHandler.SetOnPageScrollLifecycle(func(scrollCtx context.Context, action input.Action, phase input.PageScrollPhase) error { | |
| 1092 | + | a.activateBrowserWindow(bw) | |
| 1093 | + | return a.kbDispatcher.DispatchPageScrollLifecycle(scrollCtx, action, phase) | |
| 1094 | + | }) | |
| 1091 | 1095 | bw.keyboardHandler.SetOnEscape(func(escapeCtx context.Context) bool { | |
| 1092 | 1096 | a.activateBrowserWindow(bw) | |
| 1093 | 1097 | return a.handleGlobalEscape(escapeCtx) | |
| @@ -1088,6 +1088,10 @@ func (a *App) initBrowserWindowInput(ctx context.Context, bw *browserWindow) { | |||||
| 1088 | } | 1088 | } | ||
| 1089 | return a.dispatchBrowserWindowAction(actionCtx, bw, action) | 1089 | return a.dispatchBrowserWindowAction(actionCtx, bw, action) | ||
| 1090 | }) | 1090 | }) | ||
| 1091 | + | bw.keyboardHandler.SetOnPageScrollLifecycle(func(scrollCtx context.Context, action input.Action, phase input.PageScrollPhase) error { | |||
| 1092 | + | a.activateBrowserWindow(bw) | |||
| 1093 | + | return a.kbDispatcher.DispatchPageScrollLifecycle(scrollCtx, action, phase) | |||
| 1094 | + | }) | |||
| 1091 | bw.keyboardHandler.SetOnEscape(func(escapeCtx context.Context) bool { | 1095 | bw.keyboardHandler.SetOnEscape(func(escapeCtx context.Context) bool { | ||
| 1092 | a.activateBrowserWindow(bw) | 1096 | a.activateBrowserWindow(bw) | ||
| 1093 | return a.handleGlobalEscape(escapeCtx) | 1097 | return a.handleGlobalEscape(escapeCtx) | ||
M internal/ui/coordinator/navigation.go +22 -0
| @@ -304,6 +304,28 @@ func (c *NavigationCoordinator) ScrollWebView(ctx context.Context, wv port.WebVi | |||
| 304 | 304 | return c.pageScrollUC.Scroll(ctx, wv, cmd) | |
| 305 | 305 | } | |
| 306 | 306 | ||
| 307 | + | // ScrollWebViewContinuous applies one autonomous held-key page-scroll step. | |
| 308 | + | func (c *NavigationCoordinator) ScrollWebViewContinuous(ctx context.Context, wv port.WebView, cmd usecase.PageScrollCommand) error { | |
| 309 | + | if err := requireWebView(wv); err != nil { | |
| 310 | + | return err | |
| 311 | + | } | |
| 312 | + | if c.pageScrollUC == nil { | |
| 313 | + | return fmt.Errorf("page scroll usecase not set") | |
| 314 | + | } | |
| 315 | + | return c.pageScrollUC.ScrollContinuous(ctx, wv, cmd) | |
| 316 | + | } | |
| 317 | + | ||
| 318 | + | // StopPageScroll discards queued continuous deltas for adapters that support it. | |
| 319 | + | func (c *NavigationCoordinator) StopPageScroll(ctx context.Context, wv port.WebView) error { | |
| 320 | + | if err := requireWebView(wv); err != nil { | |
| 321 | + | return err | |
| 322 | + | } | |
| 323 | + | if c.pageScrollUC == nil { | |
| 324 | + | return fmt.Errorf("page scroll usecase not set") | |
| 325 | + | } | |
| 326 | + | return c.pageScrollUC.Stop(ctx, wv) | |
| 327 | + | } | |
| 328 | + | ||
| 307 | 329 | // NotifyZoomChanged updates the omnibox zoom indicator. | |
| 308 | 330 | func (c *NavigationCoordinator) NotifyZoomChanged(ctx context.Context, factor float64) { | |
| 309 | 331 | if c.omniboxProvider != nil { | |
| @@ -304,6 +304,28 @@ func (c *NavigationCoordinator) ScrollWebView(ctx context.Context, wv port.WebVi | |||||
| 304 | return c.pageScrollUC.Scroll(ctx, wv, cmd) | 304 | return c.pageScrollUC.Scroll(ctx, wv, cmd) | ||
| 305 | } | 305 | } | ||
| 306 | 306 | ||||
| 307 | + | // ScrollWebViewContinuous applies one autonomous held-key page-scroll step. | |||
| 308 | + | func (c *NavigationCoordinator) ScrollWebViewContinuous(ctx context.Context, wv port.WebView, cmd usecase.PageScrollCommand) error { | |||
| 309 | + | if err := requireWebView(wv); err != nil { | |||
| 310 | + | return err | |||
| 311 | + | } | |||
| 312 | + | if c.pageScrollUC == nil { | |||
| 313 | + | return fmt.Errorf("page scroll usecase not set") | |||
| 314 | + | } | |||
| 315 | + | return c.pageScrollUC.ScrollContinuous(ctx, wv, cmd) | |||
| 316 | + | } | |||
| 317 | + | ||||
| 318 | + | // StopPageScroll discards queued continuous deltas for adapters that support it. | |||
| 319 | + | func (c *NavigationCoordinator) StopPageScroll(ctx context.Context, wv port.WebView) error { | |||
| 320 | + | if err := requireWebView(wv); err != nil { | |||
| 321 | + | return err | |||
| 322 | + | } | |||
| 323 | + | if c.pageScrollUC == nil { | |||
| 324 | + | return fmt.Errorf("page scroll usecase not set") | |||
| 325 | + | } | |||
| 326 | + | return c.pageScrollUC.Stop(ctx, wv) | |||
| 327 | + | } | |||
| 328 | + | ||||
| 307 | // NotifyZoomChanged updates the omnibox zoom indicator. | 329 | // NotifyZoomChanged updates the omnibox zoom indicator. | ||
| 308 | func (c *NavigationCoordinator) NotifyZoomChanged(ctx context.Context, factor float64) { | 330 | func (c *NavigationCoordinator) NotifyZoomChanged(ctx context.Context, factor float64) { | ||
| 309 | if c.omniboxProvider != nil { | 331 | if c.omniboxProvider != nil { | ||
M internal/ui/dispatcher/keyboard.go +35 -0
| @@ -372,6 +372,41 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||
| 372 | 372 | } | |
| 373 | 373 | } | |
| 374 | 374 | ||
| 375 | + | // DispatchPageScrollLifecycle routes autonomous held-key page scroll events. | |
| 376 | + | // Stop deliberately avoids the normal action map so it emits neither another | |
| 377 | + | // scroll tap nor visual pulse. | |
| 378 | + | func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, action input.Action, phase input.PageScrollPhase) error { | |
| 379 | + | cmd, ok := pageScrollCommand(action) | |
| 380 | + | if !ok { | |
| 381 | + | return nil | |
| 382 | + | } | |
| 383 | + | return d.withActiveWebView(ctx, "page scroll lifecycle", func(wv port.WebView) error { | |
| 384 | + | if phase == input.PageScrollStop { | |
| 385 | + | return d.navCoord.StopPageScroll(ctx, wv) | |
| 386 | + | } | |
| 387 | + | return d.navCoord.ScrollWebViewContinuous(ctx, wv, cmd) | |
| 388 | + | }) | |
| 389 | + | } | |
| 390 | + | ||
| 391 | + | func pageScrollCommand(action input.Action) (usecase.PageScrollCommand, bool) { | |
| 392 | + | switch action { | |
| 393 | + | case input.ActionPageScrollLeft: | |
| 394 | + | return usecase.PageScrollLeft, true | |
| 395 | + | case input.ActionPageScrollRight: | |
| 396 | + | return usecase.PageScrollRight, true | |
| 397 | + | case input.ActionPageScrollUp: | |
| 398 | + | return usecase.PageScrollUp, true | |
| 399 | + | case input.ActionPageScrollDown: | |
| 400 | + | return usecase.PageScrollDown, true | |
| 401 | + | case input.ActionPageScrollUpFast: | |
| 402 | + | return usecase.PageScrollUpFast, true | |
| 403 | + | case input.ActionPageScrollDownFast: | |
| 404 | + | return usecase.PageScrollDownFast, true | |
| 405 | + | default: | |
| 406 | + | return 0, false | |
| 407 | + | } | |
| 408 | + | } | |
| 409 | + | ||
| 375 | 410 | // Dispatch routes a keyboard action to the appropriate coordinator. | |
| 376 | 411 | func (d *KeyboardDispatcher) Dispatch(ctx context.Context, action input.Action) error { | |
| 377 | 412 | log := logging.FromContext(ctx) | |
| @@ -372,6 +372,41 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||||
| 372 | } | 372 | } | ||
| 373 | } | 373 | } | ||
| 374 | 374 | ||||
| 375 | + | // DispatchPageScrollLifecycle routes autonomous held-key page scroll events. | |||
| 376 | + | // Stop deliberately avoids the normal action map so it emits neither another | |||
| 377 | + | // scroll tap nor visual pulse. | |||
| 378 | + | func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, action input.Action, phase input.PageScrollPhase) error { | |||
| 379 | + | cmd, ok := pageScrollCommand(action) | |||
| 380 | + | if !ok { | |||
| 381 | + | return nil | |||
| 382 | + | } | |||
| 383 | + | return d.withActiveWebView(ctx, "page scroll lifecycle", func(wv port.WebView) error { | |||
| 384 | + | if phase == input.PageScrollStop { | |||
| 385 | + | return d.navCoord.StopPageScroll(ctx, wv) | |||
| 386 | + | } | |||
| 387 | + | return d.navCoord.ScrollWebViewContinuous(ctx, wv, cmd) | |||
| 388 | + | }) | |||
| 389 | + | } | |||
| 390 | + | ||||
| 391 | + | func pageScrollCommand(action input.Action) (usecase.PageScrollCommand, bool) { | |||
| 392 | + | switch action { | |||
| 393 | + | case input.ActionPageScrollLeft: | |||
| 394 | + | return usecase.PageScrollLeft, true | |||
| 395 | + | case input.ActionPageScrollRight: | |||
| 396 | + | return usecase.PageScrollRight, true | |||
| 397 | + | case input.ActionPageScrollUp: | |||
| 398 | + | return usecase.PageScrollUp, true | |||
| 399 | + | case input.ActionPageScrollDown: | |||
| 400 | + | return usecase.PageScrollDown, true | |||
| 401 | + | case input.ActionPageScrollUpFast: | |||
| 402 | + | return usecase.PageScrollUpFast, true | |||
| 403 | + | case input.ActionPageScrollDownFast: | |||
| 404 | + | return usecase.PageScrollDownFast, true | |||
| 405 | + | default: | |||
| 406 | + | return 0, false | |||
| 407 | + | } | |||
| 408 | + | } | |||
| 409 | + | ||||
| 375 | // Dispatch routes a keyboard action to the appropriate coordinator. | 410 | // Dispatch routes a keyboard action to the appropriate coordinator. | ||
| 376 | func (d *KeyboardDispatcher) Dispatch(ctx context.Context, action input.Action) error { | 411 | func (d *KeyboardDispatcher) Dispatch(ctx context.Context, action input.Action) error { | ||
| 377 | log := logging.FromContext(ctx) | 412 | log := logging.FromContext(ctx) | ||
M internal/ui/dispatcher/keyboard_test.go +28 -0
| @@ -211,6 +211,15 @@ type mockScrollableWebView struct { | |||
| 211 | 211 | *mocks.MockPageScrollable | |
| 212 | 212 | } | |
| 213 | 213 | ||
| 214 | + | type lifecycleScrollableWebView struct { | |
| 215 | + | *mockScrollableWebView | |
| 216 | + | cancelCalls int | |
| 217 | + | } | |
| 218 | + | ||
| 219 | + | func (wv *lifecycleScrollableWebView) CancelPageScroll(context.Context) { | |
| 220 | + | wv.cancelCalls++ | |
| 221 | + | } | |
| 222 | + | ||
| 214 | 223 | func TestKeyboardDispatcher_PageModeActionsRouteToCorrectScrollCommand(t *testing.T) { | |
| 215 | 224 | ctx := context.Background() | |
| 216 | 225 | ||
| @@ -259,6 +268,25 @@ func TestKeyboardDispatcher_PageModeActionsRouteToCorrectScrollCommand(t *testin | |||
| 259 | 268 | } | |
| 260 | 269 | } | |
| 261 | 270 | ||
| 271 | + | func TestKeyboardDispatcher_PageScrollLifecycleRoutesContinuousAndStop(t *testing.T) { | |
| 272 | + | ctx := context.Background() | |
| 273 | + | base := mocks.NewMockWebView(t) | |
| 274 | + | scroller := mocks.NewMockPageScrollable(t) | |
| 275 | + | wv := &lifecycleScrollableWebView{mockScrollableWebView: &mockScrollableWebView{MockWebView: base, MockPageScrollable: scroller}} | |
| 276 | + | navCoord := &coordinator.NavigationCoordinator{} | |
| 277 | + | navCoord.SetPageScrollUseCase(usecase.NewPageScrollUseCase()) | |
| 278 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, navCoord, nil, nil, KeyboardActions{ | |
| 279 | + | ActiveWebView: func(context.Context) port.WebView { return wv }, | |
| 280 | + | }, func(context.Context) entity.PaneID { return "" }) | |
| 281 | + | ||
| 282 | + | scroller.EXPECT().ScrollPage(ctx, port.PageScrollRequest{ | |
| 283 | + | Command: port.PageScrollCommandDown, FallbackDY: 80, Continuous: true, | |
| 284 | + | }).Return(nil).Once() | |
| 285 | + | require.NoError(t, d.DispatchPageScrollLifecycle(ctx, input.ActionPageScrollDown, input.PageScrollContinuous)) | |
| 286 | + | require.NoError(t, d.DispatchPageScrollLifecycle(ctx, input.ActionPageScrollDown, input.PageScrollStop)) | |
| 287 | + | assert.Equal(t, 1, wv.cancelCalls) | |
| 288 | + | } | |
| 289 | + | ||
| 262 | 290 | func TestKeyboardDispatcher_PageModeNoopWhenNoActiveWebView(t *testing.T) { | |
| 263 | 291 | ctx := context.Background() | |
| 264 | 292 | ||
| @@ -211,6 +211,15 @@ type mockScrollableWebView struct { | |||||
| 211 | *mocks.MockPageScrollable | 211 | *mocks.MockPageScrollable | ||
| 212 | } | 212 | } | ||
| 213 | 213 | ||||
| 214 | + | type lifecycleScrollableWebView struct { | |||
| 215 | + | *mockScrollableWebView | |||
| 216 | + | cancelCalls int | |||
| 217 | + | } | |||
| 218 | + | ||||
| 219 | + | func (wv *lifecycleScrollableWebView) CancelPageScroll(context.Context) { | |||
| 220 | + | wv.cancelCalls++ | |||
| 221 | + | } | |||
| 222 | + | ||||
| 214 | func TestKeyboardDispatcher_PageModeActionsRouteToCorrectScrollCommand(t *testing.T) { | 223 | func TestKeyboardDispatcher_PageModeActionsRouteToCorrectScrollCommand(t *testing.T) { | ||
| 215 | ctx := context.Background() | 224 | ctx := context.Background() | ||
| 216 | 225 | ||||
| @@ -259,6 +268,25 @@ func TestKeyboardDispatcher_PageModeActionsRouteToCorrectScrollCommand(t *testin | |||||
| 259 | } | 268 | } | ||
| 260 | } | 269 | } | ||
| 261 | 270 | ||||
| 271 | + | func TestKeyboardDispatcher_PageScrollLifecycleRoutesContinuousAndStop(t *testing.T) { | |||
| 272 | + | ctx := context.Background() | |||
| 273 | + | base := mocks.NewMockWebView(t) | |||
| 274 | + | scroller := mocks.NewMockPageScrollable(t) | |||
| 275 | + | wv := &lifecycleScrollableWebView{mockScrollableWebView: &mockScrollableWebView{MockWebView: base, MockPageScrollable: scroller}} | |||
| 276 | + | navCoord := &coordinator.NavigationCoordinator{} | |||
| 277 | + | navCoord.SetPageScrollUseCase(usecase.NewPageScrollUseCase()) | |||
| 278 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, navCoord, nil, nil, KeyboardActions{ | |||
| 279 | + | ActiveWebView: func(context.Context) port.WebView { return wv }, | |||
| 280 | + | }, func(context.Context) entity.PaneID { return "" }) | |||
| 281 | + | ||||
| 282 | + | scroller.EXPECT().ScrollPage(ctx, port.PageScrollRequest{ | |||
| 283 | + | Command: port.PageScrollCommandDown, FallbackDY: 80, Continuous: true, | |||
| 284 | + | }).Return(nil).Once() | |||
| 285 | + | require.NoError(t, d.DispatchPageScrollLifecycle(ctx, input.ActionPageScrollDown, input.PageScrollContinuous)) | |||
| 286 | + | require.NoError(t, d.DispatchPageScrollLifecycle(ctx, input.ActionPageScrollDown, input.PageScrollStop)) | |||
| 287 | + | assert.Equal(t, 1, wv.cancelCalls) | |||
| 288 | + | } | |||
| 289 | + | ||||
| 262 | func TestKeyboardDispatcher_PageModeNoopWhenNoActiveWebView(t *testing.T) { | 290 | func TestKeyboardDispatcher_PageModeNoopWhenNoActiveWebView(t *testing.T) { | ||
| 263 | ctx := context.Background() | 291 | ctx := context.Background() | ||
| 264 | 292 | ||||
M internal/ui/input/keyboard.go +99 -82
| @@ -36,11 +36,22 @@ import ( | |||
| 36 | 36 | // Return an error if the action fails. | |
| 37 | 37 | type ActionHandler func(ctx context.Context, action Action) error | |
| 38 | 38 | ||
| 39 | + | // PageScrollPhase identifies held-scroll lifecycle events. It is internal input | |
| 40 | + | // state, not a configurable action or shortcut. | |
| 41 | + | type PageScrollPhase uint8 | |
| 42 | + | ||
| 43 | + | const ( | |
| 44 | + | PageScrollContinuous PageScrollPhase = iota | |
| 45 | + | PageScrollStop | |
| 46 | + | ) | |
| 47 | + | ||
| 48 | + | // PageScrollLifecycleHandler receives autonomous held-scroll ticks and the | |
| 49 | + | // matching stop event without changing the normal configurable action API. | |
| 50 | + | type PageScrollLifecycleHandler func(ctx context.Context, action Action, phase PageScrollPhase) error | |
| 51 | + | ||
| 39 | 52 | const ( | |
| 40 | - | // pageScrollRepeatInterval is the fallback cadence when the handler is not | |
| 41 | - | // attached to a GTK widget. Attached handlers use GTK tick callbacks instead, | |
| 42 | - | // which run at the output device's frame rate. | |
| 43 | - | pageScrollRepeatInterval = time.Second / 60 | |
| 53 | + | pageScrollHoldDelay = 250 * time.Millisecond | |
| 54 | + | pageScrollCadence = 16 * time.Millisecond | |
| 44 | 55 | ) | |
| 45 | 56 | ||
| 46 | 57 | // AccentHandler handles long-press accent detection. | |
| @@ -65,8 +76,9 @@ type KeyboardHandler struct { | |||
| 65 | 76 | workspace *entity.WorkspaceConfig | |
| 66 | 77 | session *entity.SessionConfig | |
| 67 | 78 | ||
| 68 | - | // Action handler callback | |
| 69 | - | onAction ActionHandler | |
| 79 | + | // Action handler callbacks. | |
| 80 | + | onAction ActionHandler | |
| 81 | + | onPageScrollLifecycle PageScrollLifecycleHandler | |
| 70 | 82 | // Optional routing callback that determines how a key should be handled. | |
| 71 | 83 | // Returns RouteHandleShortcuts (default), RoutePassToWidget (let focused | |
| 72 | 84 | // widget handle it), or RouteAccentDetection (long-press accent for GTK entries). | |
| @@ -93,14 +105,15 @@ type KeyboardHandler struct { | |||
| 93 | 105 | ctx context.Context | |
| 94 | 106 | mu sync.RWMutex | |
| 95 | 107 | ||
| 96 | - | pageScrollRepeatAction Action | |
| 97 | - | pageScrollRepeatKeyval uint | |
| 98 | - | pageScrollRepeatTimer uint | |
| 99 | - | pageScrollRepeatCb glib.SourceFunc | |
| 100 | - | pageScrollRepeatTickCb gtk.TickCallback | |
| 101 | - | pageScrollRepeatAdd func(intervalMS uint, cb *glib.SourceFunc) uint | |
| 102 | - | pageScrollRepeatRemove func(id uint) bool | |
| 103 | - | pageScrollRepeatActiveRemove func(id uint) bool | |
| 108 | + | pageScrollRepeatAction Action | |
| 109 | + | pageScrollRepeatKeyval uint | |
| 110 | + | pageScrollRepeatTimer uint | |
| 111 | + | pageScrollRepeatGeneration uint64 | |
| 112 | + | pageScrollRepeatPressedAt time.Time | |
| 113 | + | pageScrollRepeatCb glib.SourceFunc | |
| 114 | + | pageScrollRepeatAdd func(intervalMS uint, cb *glib.SourceFunc) uint | |
| 115 | + | pageScrollRepeatRemove func(id uint) bool | |
| 116 | + | pageScrollNow func() time.Time | |
| 104 | 117 | } | |
| 105 | 118 | ||
| 106 | 119 | // NewKeyboardHandler creates a new keyboard handler. | |
| @@ -120,6 +133,7 @@ func NewKeyboardHandler(ctx context.Context, workspace *entity.WorkspaceConfig, | |||
| 120 | 133 | return glib.TimeoutAdd(intervalMS, cb, 0) | |
| 121 | 134 | }, | |
| 122 | 135 | pageScrollRepeatRemove: glib.SourceRemove, | |
| 136 | + | pageScrollNow: time.Now, | |
| 123 | 137 | } | |
| 124 | 138 | h.SetOnModeChange(nil) | |
| 125 | 139 | ||
| @@ -133,6 +147,14 @@ func (h *KeyboardHandler) SetOnAction(fn ActionHandler) { | |||
| 133 | 147 | h.onAction = fn | |
| 134 | 148 | } | |
| 135 | 149 | ||
| 150 | + | // SetOnPageScrollLifecycle sets the callback for autonomous held-scroll ticks | |
| 151 | + | // and stop notification. Tap actions continue through SetOnAction. | |
| 152 | + | func (h *KeyboardHandler) SetOnPageScrollLifecycle(fn PageScrollLifecycleHandler) { | |
| 153 | + | h.mu.Lock() | |
| 154 | + | defer h.mu.Unlock() | |
| 155 | + | h.onPageScrollLifecycle = fn | |
| 156 | + | } | |
| 157 | + | ||
| 136 | 158 | // SetPageModeActivationPassthrough sets the callback that can block Page mode | |
| 137 | 159 | // activation and let the original key event propagate instead. | |
| 138 | 160 | func (h *KeyboardHandler) SetPageModeActivationPassthrough(fn func() bool) { | |
| @@ -293,6 +315,7 @@ func (h *KeyboardHandler) DetachForDestroy() { | |||
| 293 | 315 | } | |
| 294 | 316 | ||
| 295 | 317 | func (h *KeyboardHandler) detach(removeController bool) { | |
| 318 | + | h.stopPageScrollRepeat() | |
| 296 | 319 | h.mu.Lock() | |
| 297 | 320 | if h.controller != nil && h.keyPressedHandlerID != 0 { | |
| 298 | 321 | h.controller.DisconnectSignal(h.keyPressedHandlerID) | |
| @@ -310,21 +333,7 @@ func (h *KeyboardHandler) detach(removeController bool) { | |||
| 310 | 333 | h.keyPressedHandlerID = 0 | |
| 311 | 334 | h.keyReleasedHandlerID = 0 | |
| 312 | 335 | h.activePressedActions = nil | |
| 313 | - | timerID := h.pageScrollRepeatTimer | |
| 314 | - | remove := h.pageScrollRepeatRemove | |
| 315 | - | h.pageScrollRepeatAction = "" | |
| 316 | - | h.pageScrollRepeatKeyval = 0 | |
| 317 | - | h.pageScrollRepeatTimer = 0 | |
| 318 | - | h.pageScrollRepeatCb = nil | |
| 319 | - | h.pageScrollRepeatTickCb = nil | |
| 320 | - | if h.pageScrollRepeatActiveRemove != nil { | |
| 321 | - | remove = h.pageScrollRepeatActiveRemove | |
| 322 | - | } | |
| 323 | - | h.pageScrollRepeatActiveRemove = nil | |
| 324 | 336 | h.mu.Unlock() | |
| 325 | - | if timerID != 0 && remove != nil { | |
| 326 | - | remove(timerID) | |
| 327 | - | } | |
| 328 | 337 | } | |
| 329 | 338 | ||
| 330 | 339 | // handleKeyPress processes a key press event. | |
| @@ -437,94 +446,102 @@ func (h *KeyboardHandler) handleShortcutLookupResult( | |||
| 437 | 446 | } | |
| 438 | 447 | ||
| 439 | 448 | func (h *KeyboardHandler) handlePageScrollAction(log *zerolog.Logger, action Action, mode Mode, keyval uint) bool { | |
| 440 | - | h.mu.Lock() | |
| 441 | - | repeatAction := h.pageScrollRepeatAction | |
| 442 | - | repeatKeyval := h.pageScrollRepeatKeyval | |
| 443 | - | repeatTimer := h.pageScrollRepeatTimer | |
| 444 | - | h.mu.Unlock() | |
| 445 | - | ||
| 446 | - | if repeatAction == action && repeatKeyval == keyval { | |
| 447 | - | if repeatTimer != 0 { | |
| 448 | - | log.Trace().Str("action", string(action)).Msg("page scroll key repeat handled by smooth repeater") | |
| 449 | - | return true | |
| 450 | - | } | |
| 451 | - | if h.dispatchAction(action, mode) { | |
| 452 | - | h.startPageScrollRepeat(action, keyval) | |
| 453 | - | return true | |
| 454 | - | } | |
| 455 | - | return false | |
| 449 | + | h.mu.RLock() | |
| 450 | + | sameHeldKey := h.pageScrollRepeatAction == action && | |
| 451 | + | h.pageScrollRepeatKeyval == keyval && h.pageScrollRepeatTimer != 0 | |
| 452 | + | h.mu.RUnlock() | |
| 453 | + | if sameHeldKey { | |
| 454 | + | log.Trace().Str("action", string(action)).Msg("page scroll OS repeat suppressed") | |
| 455 | + | return true | |
| 456 | 456 | } | |
| 457 | 457 | ||
| 458 | 458 | h.stopPageScrollRepeat() | |
| 459 | - | h.mu.Lock() | |
| 460 | - | h.pageScrollRepeatAction = action | |
| 461 | - | h.pageScrollRepeatKeyval = keyval | |
| 462 | - | h.mu.Unlock() | |
| 463 | - | return h.dispatchAction(action, mode) | |
| 459 | + | if !h.dispatchAction(action, mode) { | |
| 460 | + | return false | |
| 461 | + | } | |
| 462 | + | h.startPageScrollRepeat(action, keyval) | |
| 463 | + | return true | |
| 464 | 464 | } | |
| 465 | 465 | ||
| 466 | 466 | func (h *KeyboardHandler) startPageScrollRepeat(action Action, keyval uint) { | |
| 467 | 467 | h.mu.Lock() | |
| 468 | - | if h.pageScrollRepeatAction != action || h.pageScrollRepeatKeyval != keyval || h.pageScrollRepeatTimer != 0 { | |
| 468 | + | if h.pageScrollRepeatTimer != 0 || h.pageScrollRepeatAdd == nil { | |
| 469 | 469 | h.mu.Unlock() | |
| 470 | 470 | return | |
| 471 | 471 | } | |
| 472 | - | if h.window != nil { | |
| 473 | - | window := h.window | |
| 474 | - | cb := gtk.TickCallback(func(_, _, _ uintptr) bool { | |
| 475 | - | return h.pageScrollRepeatTick(action, keyval) | |
| 476 | - | }) | |
| 477 | - | h.pageScrollRepeatTickCb = cb | |
| 478 | - | h.pageScrollRepeatActiveRemove = func(id uint) bool { | |
| 479 | - | window.RemoveTickCallback(id) | |
| 480 | - | return true | |
| 481 | - | } | |
| 482 | - | h.pageScrollRepeatTimer = window.AddTickCallback(&h.pageScrollRepeatTickCb, 0, nil) | |
| 483 | - | h.mu.Unlock() | |
| 484 | - | return | |
| 485 | - | } | |
| 486 | - | ||
| 472 | + | h.pageScrollRepeatGeneration++ | |
| 473 | + | generation := h.pageScrollRepeatGeneration | |
| 474 | + | h.pageScrollRepeatAction = action | |
| 475 | + | h.pageScrollRepeatKeyval = keyval | |
| 476 | + | h.pageScrollRepeatPressedAt = h.pageScrollNow() | |
| 477 | + | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 478 | + | return h.pageScrollRepeatTick(action, keyval, generation) | |
| 479 | + | }) | |
| 480 | + | h.pageScrollRepeatCb = cb | |
| 487 | 481 | add := h.pageScrollRepeatAdd | |
| 488 | - | if add == nil { | |
| 482 | + | id := add(uint(pageScrollCadence/time.Millisecond), &h.pageScrollRepeatCb) | |
| 483 | + | if id == 0 { | |
| 484 | + | h.clearPageScrollRepeatLocked() | |
| 489 | 485 | h.mu.Unlock() | |
| 490 | 486 | return | |
| 491 | 487 | } | |
| 492 | - | cb := glib.SourceFunc(func(_ uintptr) bool { | |
| 493 | - | return h.pageScrollRepeatTick(action, keyval) | |
| 494 | - | }) | |
| 495 | - | h.pageScrollRepeatCb = cb | |
| 496 | - | h.pageScrollRepeatTimer = add(uint(pageScrollRepeatInterval/time.Millisecond), &h.pageScrollRepeatCb) | |
| 488 | + | h.pageScrollRepeatTimer = id | |
| 497 | 489 | h.mu.Unlock() | |
| 498 | 490 | } | |
| 499 | 491 | ||
| 500 | - | func (h *KeyboardHandler) pageScrollRepeatTick(action Action, keyval uint) bool { | |
| 492 | + | func (h *KeyboardHandler) pageScrollRepeatTick(action Action, keyval uint, generation uint64) bool { | |
| 501 | 493 | h.mu.RLock() | |
| 502 | - | active := h.pageScrollRepeatAction == action && h.pageScrollRepeatKeyval == keyval && h.pageScrollRepeatTimer != 0 | |
| 494 | + | active := h.pageScrollRepeatAction == action && h.pageScrollRepeatKeyval == keyval && | |
| 495 | + | h.pageScrollRepeatTimer != 0 && h.pageScrollRepeatGeneration == generation | |
| 496 | + | pressedAt := h.pageScrollRepeatPressedAt | |
| 497 | + | now := h.pageScrollNow | |
| 503 | 498 | h.mu.RUnlock() | |
| 504 | 499 | if !active || h.modal.Mode() != ModePage { | |
| 505 | - | h.stopPageScrollRepeat() | |
| 506 | 500 | return false | |
| 507 | 501 | } | |
| 508 | - | h.dispatchAction(action, ModePage) | |
| 502 | + | if now().Sub(pressedAt) < pageScrollHoldDelay { | |
| 503 | + | return true | |
| 504 | + | } | |
| 505 | + | h.dispatchPageScrollLifecycle(action, PageScrollContinuous) | |
| 509 | 506 | return true | |
| 510 | 507 | } | |
| 511 | 508 | ||
| 512 | 509 | func (h *KeyboardHandler) stopPageScrollRepeat() { | |
| 513 | 510 | h.mu.Lock() | |
| 511 | + | action := h.pageScrollRepeatAction | |
| 514 | 512 | timerID := h.pageScrollRepeatTimer | |
| 515 | 513 | remove := h.pageScrollRepeatRemove | |
| 514 | + | h.clearPageScrollRepeatLocked() | |
| 515 | + | h.mu.Unlock() | |
| 516 | + | if timerID != 0 && remove != nil { | |
| 517 | + | remove(timerID) | |
| 518 | + | } | |
| 519 | + | if action != "" { | |
| 520 | + | h.dispatchPageScrollLifecycle(action, PageScrollStop) | |
| 521 | + | } | |
| 522 | + | } | |
| 523 | + | ||
| 524 | + | func (h *KeyboardHandler) clearPageScrollRepeatLocked() { | |
| 525 | + | h.pageScrollRepeatGeneration++ | |
| 516 | 526 | h.pageScrollRepeatAction = "" | |
| 517 | 527 | h.pageScrollRepeatKeyval = 0 | |
| 518 | 528 | h.pageScrollRepeatTimer = 0 | |
| 529 | + | h.pageScrollRepeatPressedAt = time.Time{} | |
| 519 | 530 | h.pageScrollRepeatCb = nil | |
| 520 | - | h.pageScrollRepeatTickCb = nil | |
| 521 | - | if h.pageScrollRepeatActiveRemove != nil { | |
| 522 | - | remove = h.pageScrollRepeatActiveRemove | |
| 531 | + | } | |
| 532 | + | ||
| 533 | + | func (h *KeyboardHandler) dispatchPageScrollLifecycle(action Action, phase PageScrollPhase) { | |
| 534 | + | h.mu.RLock() | |
| 535 | + | handler := h.onPageScrollLifecycle | |
| 536 | + | h.mu.RUnlock() | |
| 537 | + | if handler == nil { | |
| 538 | + | if phase == PageScrollContinuous { | |
| 539 | + | h.dispatchAction(action, ModePage) | |
| 540 | + | } | |
| 541 | + | return | |
| 523 | 542 | } | |
| 524 | - | h.pageScrollRepeatActiveRemove = nil | |
| 525 | - | h.mu.Unlock() | |
| 526 | - | if timerID != 0 && remove != nil { | |
| 527 | - | remove(timerID) | |
| 543 | + | if err := handler(h.ctx, action, phase); err != nil { | |
| 544 | + | logging.FromContext(h.ctx).Error().Err(err).Str("action", string(action)).Msg("page scroll lifecycle handler error") | |
| 528 | 545 | } | |
| 529 | 546 | } | |
| 530 | 547 | ||
| @@ -36,11 +36,22 @@ import ( | |||||
| 36 | // Return an error if the action fails. | 36 | // Return an error if the action fails. | ||
| 37 | type ActionHandler func(ctx context.Context, action Action) error | 37 | type ActionHandler func(ctx context.Context, action Action) error | ||
| 38 | 38 | ||||
| 39 | + | // PageScrollPhase identifies held-scroll lifecycle events. It is internal input | |||
| 40 | + | // state, not a configurable action or shortcut. | |||
| 41 | + | type PageScrollPhase uint8 | |||
| 42 | + | ||||
| 43 | + | const ( | |||
| 44 | + | PageScrollContinuous PageScrollPhase = iota | |||
| 45 | + | PageScrollStop | |||
| 46 | + | ) | |||
| 47 | + | ||||
| 48 | + | // PageScrollLifecycleHandler receives autonomous held-scroll ticks and the | |||
| 49 | + | // matching stop event without changing the normal configurable action API. | |||
| 50 | + | type PageScrollLifecycleHandler func(ctx context.Context, action Action, phase PageScrollPhase) error | |||
| 51 | + | ||||
| 39 | const ( | 52 | const ( | ||
| 40 | - | // pageScrollRepeatInterval is the fallback cadence when the handler is not | 53 | + | pageScrollHoldDelay = 250 * time.Millisecond |
| 41 | - | // attached to a GTK widget. Attached handlers use GTK tick callbacks instead, | 54 | + | pageScrollCadence = 16 * time.Millisecond |
| 42 | - | // which run at the output device's frame rate. | |||
| 43 | - | pageScrollRepeatInterval = time.Second / 60 | |||
| 44 | ) | 55 | ) | ||
| 45 | 56 | ||||
| 46 | // AccentHandler handles long-press accent detection. | 57 | // AccentHandler handles long-press accent detection. | ||
| @@ -65,8 +76,9 @@ type KeyboardHandler struct { | |||||
| 65 | workspace *entity.WorkspaceConfig | 76 | workspace *entity.WorkspaceConfig | ||
| 66 | session *entity.SessionConfig | 77 | session *entity.SessionConfig | ||
| 67 | 78 | ||||
| 68 | - | // Action handler callback | 79 | + | // Action handler callbacks. |
| 69 | - | onAction ActionHandler | 80 | + | onAction ActionHandler |
| 81 | + | onPageScrollLifecycle PageScrollLifecycleHandler | |||
| 70 | // Optional routing callback that determines how a key should be handled. | 82 | // Optional routing callback that determines how a key should be handled. | ||
| 71 | // Returns RouteHandleShortcuts (default), RoutePassToWidget (let focused | 83 | // Returns RouteHandleShortcuts (default), RoutePassToWidget (let focused | ||
| 72 | // widget handle it), or RouteAccentDetection (long-press accent for GTK entries). | 84 | // widget handle it), or RouteAccentDetection (long-press accent for GTK entries). | ||
| @@ -93,14 +105,15 @@ type KeyboardHandler struct { | |||||
| 93 | ctx context.Context | 105 | ctx context.Context | ||
| 94 | mu sync.RWMutex | 106 | mu sync.RWMutex | ||
| 95 | 107 | ||||
| 96 | - | pageScrollRepeatAction Action | 108 | + | pageScrollRepeatAction Action |
| 97 | - | pageScrollRepeatKeyval uint | 109 | + | pageScrollRepeatKeyval uint |
| 98 | - | pageScrollRepeatTimer uint | 110 | + | pageScrollRepeatTimer uint |
| 99 | - | pageScrollRepeatCb glib.SourceFunc | 111 | + | pageScrollRepeatGeneration uint64 |
| 100 | - | pageScrollRepeatTickCb gtk.TickCallback | 112 | + | pageScrollRepeatPressedAt time.Time |
| 101 | - | pageScrollRepeatAdd func(intervalMS uint, cb *glib.SourceFunc) uint | 113 | + | pageScrollRepeatCb glib.SourceFunc |
| 102 | - | pageScrollRepeatRemove func(id uint) bool | 114 | + | pageScrollRepeatAdd func(intervalMS uint, cb *glib.SourceFunc) uint |
| 103 | - | pageScrollRepeatActiveRemove func(id uint) bool | 115 | + | pageScrollRepeatRemove func(id uint) bool |
| 116 | + | pageScrollNow func() time.Time | |||
| 104 | } | 117 | } | ||
| 105 | 118 | ||||
| 106 | // NewKeyboardHandler creates a new keyboard handler. | 119 | // NewKeyboardHandler creates a new keyboard handler. | ||
| @@ -120,6 +133,7 @@ func NewKeyboardHandler(ctx context.Context, workspace *entity.WorkspaceConfig, | |||||
| 120 | return glib.TimeoutAdd(intervalMS, cb, 0) | 133 | return glib.TimeoutAdd(intervalMS, cb, 0) | ||
| 121 | }, | 134 | }, | ||
| 122 | pageScrollRepeatRemove: glib.SourceRemove, | 135 | pageScrollRepeatRemove: glib.SourceRemove, | ||
| 136 | + | pageScrollNow: time.Now, | |||
| 123 | } | 137 | } | ||
| 124 | h.SetOnModeChange(nil) | 138 | h.SetOnModeChange(nil) | ||
| 125 | 139 | ||||
| @@ -133,6 +147,14 @@ func (h *KeyboardHandler) SetOnAction(fn ActionHandler) { | |||||
| 133 | h.onAction = fn | 147 | h.onAction = fn | ||
| 134 | } | 148 | } | ||
| 135 | 149 | ||||
| 150 | + | // SetOnPageScrollLifecycle sets the callback for autonomous held-scroll ticks | |||
| 151 | + | // and stop notification. Tap actions continue through SetOnAction. | |||
| 152 | + | func (h *KeyboardHandler) SetOnPageScrollLifecycle(fn PageScrollLifecycleHandler) { | |||
| 153 | + | h.mu.Lock() | |||
| 154 | + | defer h.mu.Unlock() | |||
| 155 | + | h.onPageScrollLifecycle = fn | |||
| 156 | + | } | |||
| 157 | + | ||||
| 136 | // SetPageModeActivationPassthrough sets the callback that can block Page mode | 158 | // SetPageModeActivationPassthrough sets the callback that can block Page mode | ||
| 137 | // activation and let the original key event propagate instead. | 159 | // activation and let the original key event propagate instead. | ||
| 138 | func (h *KeyboardHandler) SetPageModeActivationPassthrough(fn func() bool) { | 160 | func (h *KeyboardHandler) SetPageModeActivationPassthrough(fn func() bool) { | ||
| @@ -293,6 +315,7 @@ func (h *KeyboardHandler) DetachForDestroy() { | |||||
| 293 | } | 315 | } | ||
| 294 | 316 | ||||
| 295 | func (h *KeyboardHandler) detach(removeController bool) { | 317 | func (h *KeyboardHandler) detach(removeController bool) { | ||
| 318 | + | h.stopPageScrollRepeat() | |||
| 296 | h.mu.Lock() | 319 | h.mu.Lock() | ||
| 297 | if h.controller != nil && h.keyPressedHandlerID != 0 { | 320 | if h.controller != nil && h.keyPressedHandlerID != 0 { | ||
| 298 | h.controller.DisconnectSignal(h.keyPressedHandlerID) | 321 | h.controller.DisconnectSignal(h.keyPressedHandlerID) | ||
| @@ -310,21 +333,7 @@ func (h *KeyboardHandler) detach(removeController bool) { | |||||
| 310 | h.keyPressedHandlerID = 0 | 333 | h.keyPressedHandlerID = 0 | ||
| 311 | h.keyReleasedHandlerID = 0 | 334 | h.keyReleasedHandlerID = 0 | ||
| 312 | h.activePressedActions = nil | 335 | h.activePressedActions = nil | ||
| 313 | - | timerID := h.pageScrollRepeatTimer | |||
| 314 | - | remove := h.pageScrollRepeatRemove | |||
| 315 | - | h.pageScrollRepeatAction = "" | |||
| 316 | - | h.pageScrollRepeatKeyval = 0 | |||
| 317 | - | h.pageScrollRepeatTimer = 0 | |||
| 318 | - | h.pageScrollRepeatCb = nil | |||
| 319 | - | h.pageScrollRepeatTickCb = nil | |||
| 320 | - | if h.pageScrollRepeatActiveRemove != nil { | |||
| 321 | - | remove = h.pageScrollRepeatActiveRemove | |||
| 322 | - | } | |||
| 323 | - | h.pageScrollRepeatActiveRemove = nil | |||
| 324 | h.mu.Unlock() | 336 | h.mu.Unlock() | ||
| 325 | - | if timerID != 0 && remove != nil { | |||
| 326 | - | remove(timerID) | |||
| 327 | - | } | |||
| 328 | } | 337 | } | ||
| 329 | 338 | ||||
| 330 | // handleKeyPress processes a key press event. | 339 | // handleKeyPress processes a key press event. | ||
| @@ -437,94 +446,102 @@ func (h *KeyboardHandler) handleShortcutLookupResult( | |||||
| 437 | } | 446 | } | ||
| 438 | 447 | ||||
| 439 | func (h *KeyboardHandler) handlePageScrollAction(log *zerolog.Logger, action Action, mode Mode, keyval uint) bool { | 448 | func (h *KeyboardHandler) handlePageScrollAction(log *zerolog.Logger, action Action, mode Mode, keyval uint) bool { | ||
| 440 | - | h.mu.Lock() | 449 | + | h.mu.RLock() |
| 441 | - | repeatAction := h.pageScrollRepeatAction | 450 | + | sameHeldKey := h.pageScrollRepeatAction == action && |
| 442 | - | repeatKeyval := h.pageScrollRepeatKeyval | 451 | + | h.pageScrollRepeatKeyval == keyval && h.pageScrollRepeatTimer != 0 |
| 443 | - | repeatTimer := h.pageScrollRepeatTimer | 452 | + | h.mu.RUnlock() |
| 444 | - | h.mu.Unlock() | 453 | + | if sameHeldKey { |
| 445 | - | 454 | + | log.Trace().Str("action", string(action)).Msg("page scroll OS repeat suppressed") | |
| 446 | - | if repeatAction == action && repeatKeyval == keyval { | 455 | + | return true |
| 447 | - | if repeatTimer != 0 { | |||
| 448 | - | log.Trace().Str("action", string(action)).Msg("page scroll key repeat handled by smooth repeater") | |||
| 449 | - | return true | |||
| 450 | - | } | |||
| 451 | - | if h.dispatchAction(action, mode) { | |||
| 452 | - | h.startPageScrollRepeat(action, keyval) | |||
| 453 | - | return true | |||
| 454 | - | } | |||
| 455 | - | return false | |||
| 456 | } | 456 | } | ||
| 457 | 457 | ||||
| 458 | h.stopPageScrollRepeat() | 458 | h.stopPageScrollRepeat() | ||
| 459 | - | h.mu.Lock() | 459 | + | if !h.dispatchAction(action, mode) { |
| 460 | - | h.pageScrollRepeatAction = action | 460 | + | return false |
| 461 | - | h.pageScrollRepeatKeyval = keyval | 461 | + | } |
| 462 | - | h.mu.Unlock() | 462 | + | h.startPageScrollRepeat(action, keyval) |
| 463 | - | return h.dispatchAction(action, mode) | 463 | + | return true |
| 464 | } | 464 | } | ||
| 465 | 465 | ||||
| 466 | func (h *KeyboardHandler) startPageScrollRepeat(action Action, keyval uint) { | 466 | func (h *KeyboardHandler) startPageScrollRepeat(action Action, keyval uint) { | ||
| 467 | h.mu.Lock() | 467 | h.mu.Lock() | ||
| 468 | - | if h.pageScrollRepeatAction != action || h.pageScrollRepeatKeyval != keyval || h.pageScrollRepeatTimer != 0 { | 468 | + | if h.pageScrollRepeatTimer != 0 || h.pageScrollRepeatAdd == nil { |
| 469 | h.mu.Unlock() | 469 | h.mu.Unlock() | ||
| 470 | return | 470 | return | ||
| 471 | } | 471 | } | ||
| 472 | - | if h.window != nil { | 472 | + | h.pageScrollRepeatGeneration++ |
| 473 | - | window := h.window | 473 | + | generation := h.pageScrollRepeatGeneration |
| 474 | - | cb := gtk.TickCallback(func(_, _, _ uintptr) bool { | 474 | + | h.pageScrollRepeatAction = action |
| 475 | - | return h.pageScrollRepeatTick(action, keyval) | 475 | + | h.pageScrollRepeatKeyval = keyval |
| 476 | - | }) | 476 | + | h.pageScrollRepeatPressedAt = h.pageScrollNow() |
| 477 | - | h.pageScrollRepeatTickCb = cb | 477 | + | cb := glib.SourceFunc(func(_ uintptr) bool { |
| 478 | - | h.pageScrollRepeatActiveRemove = func(id uint) bool { | 478 | + | return h.pageScrollRepeatTick(action, keyval, generation) |
| 479 | - | window.RemoveTickCallback(id) | 479 | + | }) |
| 480 | - | return true | 480 | + | h.pageScrollRepeatCb = cb |
| 481 | - | } | |||
| 482 | - | h.pageScrollRepeatTimer = window.AddTickCallback(&h.pageScrollRepeatTickCb, 0, nil) | |||
| 483 | - | h.mu.Unlock() | |||
| 484 | - | return | |||
| 485 | - | } | |||
| 486 | - | ||||
| 487 | add := h.pageScrollRepeatAdd | 481 | add := h.pageScrollRepeatAdd | ||
| 488 | - | if add == nil { | 482 | + | id := add(uint(pageScrollCadence/time.Millisecond), &h.pageScrollRepeatCb) |
| 483 | + | if id == 0 { | |||
| 484 | + | h.clearPageScrollRepeatLocked() | |||
| 489 | h.mu.Unlock() | 485 | h.mu.Unlock() | ||
| 490 | return | 486 | return | ||
| 491 | } | 487 | } | ||
| 492 | - | cb := glib.SourceFunc(func(_ uintptr) bool { | 488 | + | h.pageScrollRepeatTimer = id |
| 493 | - | return h.pageScrollRepeatTick(action, keyval) | |||
| 494 | - | }) | |||
| 495 | - | h.pageScrollRepeatCb = cb | |||
| 496 | - | h.pageScrollRepeatTimer = add(uint(pageScrollRepeatInterval/time.Millisecond), &h.pageScrollRepeatCb) | |||
| 497 | h.mu.Unlock() | 489 | h.mu.Unlock() | ||
| 498 | } | 490 | } | ||
| 499 | 491 | ||||
| 500 | - | func (h *KeyboardHandler) pageScrollRepeatTick(action Action, keyval uint) bool { | 492 | + | func (h *KeyboardHandler) pageScrollRepeatTick(action Action, keyval uint, generation uint64) bool { |
| 501 | h.mu.RLock() | 493 | h.mu.RLock() | ||
| 502 | - | active := h.pageScrollRepeatAction == action && h.pageScrollRepeatKeyval == keyval && h.pageScrollRepeatTimer != 0 | 494 | + | active := h.pageScrollRepeatAction == action && h.pageScrollRepeatKeyval == keyval && |
| 495 | + | h.pageScrollRepeatTimer != 0 && h.pageScrollRepeatGeneration == generation | |||
| 496 | + | pressedAt := h.pageScrollRepeatPressedAt | |||
| 497 | + | now := h.pageScrollNow | |||
| 503 | h.mu.RUnlock() | 498 | h.mu.RUnlock() | ||
| 504 | if !active || h.modal.Mode() != ModePage { | 499 | if !active || h.modal.Mode() != ModePage { | ||
| 505 | - | h.stopPageScrollRepeat() | |||
| 506 | return false | 500 | return false | ||
| 507 | } | 501 | } | ||
| 508 | - | h.dispatchAction(action, ModePage) | 502 | + | if now().Sub(pressedAt) < pageScrollHoldDelay { |
| 503 | + | return true | |||
| 504 | + | } | |||
| 505 | + | h.dispatchPageScrollLifecycle(action, PageScrollContinuous) | |||
| 509 | return true | 506 | return true | ||
| 510 | } | 507 | } | ||
| 511 | 508 | ||||
| 512 | func (h *KeyboardHandler) stopPageScrollRepeat() { | 509 | func (h *KeyboardHandler) stopPageScrollRepeat() { | ||
| 513 | h.mu.Lock() | 510 | h.mu.Lock() | ||
| 511 | + | action := h.pageScrollRepeatAction | |||
| 514 | timerID := h.pageScrollRepeatTimer | 512 | timerID := h.pageScrollRepeatTimer | ||
| 515 | remove := h.pageScrollRepeatRemove | 513 | remove := h.pageScrollRepeatRemove | ||
| 514 | + | h.clearPageScrollRepeatLocked() | |||
| 515 | + | h.mu.Unlock() | |||
| 516 | + | if timerID != 0 && remove != nil { | |||
| 517 | + | remove(timerID) | |||
| 518 | + | } | |||
| 519 | + | if action != "" { | |||
| 520 | + | h.dispatchPageScrollLifecycle(action, PageScrollStop) | |||
| 521 | + | } | |||
| 522 | + | } | |||
| 523 | + | ||||
| 524 | + | func (h *KeyboardHandler) clearPageScrollRepeatLocked() { | |||
| 525 | + | h.pageScrollRepeatGeneration++ | |||
| 516 | h.pageScrollRepeatAction = "" | 526 | h.pageScrollRepeatAction = "" | ||
| 517 | h.pageScrollRepeatKeyval = 0 | 527 | h.pageScrollRepeatKeyval = 0 | ||
| 518 | h.pageScrollRepeatTimer = 0 | 528 | h.pageScrollRepeatTimer = 0 | ||
| 529 | + | h.pageScrollRepeatPressedAt = time.Time{} | |||
| 519 | h.pageScrollRepeatCb = nil | 530 | h.pageScrollRepeatCb = nil | ||
| 520 | - | h.pageScrollRepeatTickCb = nil | 531 | + | } |
| 521 | - | if h.pageScrollRepeatActiveRemove != nil { | 532 | + | |
| 522 | - | remove = h.pageScrollRepeatActiveRemove | 533 | + | func (h *KeyboardHandler) dispatchPageScrollLifecycle(action Action, phase PageScrollPhase) { |
| 534 | + | h.mu.RLock() | |||
| 535 | + | handler := h.onPageScrollLifecycle | |||
| 536 | + | h.mu.RUnlock() | |||
| 537 | + | if handler == nil { | |||
| 538 | + | if phase == PageScrollContinuous { | |||
| 539 | + | h.dispatchAction(action, ModePage) | |||
| 540 | + | } | |||
| 541 | + | return | |||
| 523 | } | 542 | } | ||
| 524 | - | h.pageScrollRepeatActiveRemove = nil | 543 | + | if err := handler(h.ctx, action, phase); err != nil { |
| 525 | - | h.mu.Unlock() | 544 | + | logging.FromContext(h.ctx).Error().Err(err).Str("action", string(action)).Msg("page scroll lifecycle handler error") |
| 526 | - | if timerID != 0 && remove != nil { | |||
| 527 | - | remove(timerID) | |||
| 528 | } | 545 | } | ||
| 529 | } | 546 | } | ||
| 530 | 547 | ||||
M internal/ui/input/keyboard_test.go +131 -64
| @@ -418,14 +418,16 @@ func TestHandleKeyPress_PageModeStaysActiveAfterScroll(t *testing.T) { | |||
| 418 | 418 | } | |
| 419 | 419 | ||
| 420 | 420 | var repeatTick glib.SourceFunc | |
| 421 | + | repeatNow := time.Unix(2_000, 0) | |
| 422 | + | h.pageScrollNow = func() time.Time { return repeatNow } | |
| 421 | 423 | h.pageScrollRepeatAdd = func(_ uint, cb *glib.SourceFunc) uint { | |
| 422 | 424 | repeatTick = *cb | |
| 423 | 425 | return 1 | |
| 424 | 426 | } | |
| 425 | 427 | h.pageScrollRepeatRemove = func(uint) bool { return true } | |
| 426 | 428 | ||
| 427 | - | // Repeated scroll actions should stay in page mode and transition onto the | |
| 428 | - | // smooth repeater instead of depending on every physical key repeat. | |
| 429 | + | // Scroll actions stay in page mode; the first press arms the smooth | |
| 430 | + | // repeater and later physical auto-repeat events are ignored. | |
| 429 | 431 | for i := 0; i < 2; i++ { | |
| 430 | 432 | scrolled := h.handleKeyPress(uint('j'), 0, 0) | |
| 431 | 433 | if !scrolled { | |
| @@ -438,6 +440,7 @@ func TestHandleKeyPress_PageModeStaysActiveAfterScroll(t *testing.T) { | |||
| 438 | 440 | if repeatTick == nil { | |
| 439 | 441 | t.Fatal("expected held page scroll to register smooth repeater") | |
| 440 | 442 | } | |
| 443 | + | repeatNow = repeatNow.Add(pageScrollHoldDelay) | |
| 441 | 444 | for i := 0; i < 3; i++ { | |
| 442 | 445 | if !repeatTick(0) { | |
| 443 | 446 | t.Fatalf("repeat tick %d stopped unexpectedly", i) | |
| @@ -455,8 +458,8 @@ func TestHandleKeyPress_PageModeStaysActiveAfterScroll(t *testing.T) { | |||
| 455 | 458 | } | |
| 456 | 459 | ||
| 457 | 460 | // Ensure scroll actions were dispatched | |
| 458 | - | if actionCalls != 6 { | |
| 459 | - | t.Fatalf("action calls = %d, want 6 (5 down + 1 up)", actionCalls) | |
| 461 | + | if actionCalls != 5 { | |
| 462 | + | t.Fatalf("action calls = %d, want 5 (4 down + 1 up)", actionCalls) | |
| 460 | 463 | } | |
| 461 | 464 | } | |
| 462 | 465 | ||
| @@ -855,33 +858,10 @@ func TestHandleKeyPress_PageModeBlocksGlobalShortcutFallback(t *testing.T) { | |||
| 855 | 858 | } | |
| 856 | 859 | } | |
| 857 | 860 | ||
| 858 | - | func TestHandleKeyPress_PageModeSmoothRepeaterStartsAfterHeldRepeat(t *testing.T) { | |
| 859 | - | ctx := context.Background() | |
| 860 | - | workspace := newTestWorkspace() | |
| 861 | - | workspace.PageMode = entity.PageModeConfig{ | |
| 862 | - | ActivationShortcut: "ctrl+y", | |
| 863 | - | Actions: map[string]entity.ActionBinding{ | |
| 864 | - | "page-scroll-down": {Keys: []string{"j"}}, | |
| 865 | - | }, | |
| 866 | - | } | |
| 867 | - | ||
| 868 | - | h := NewKeyboardHandler(ctx, workspace, newTestSession()) | |
| 869 | - | var repeatTick glib.SourceFunc | |
| 870 | - | var removedTimer uint | |
| 871 | - | h.pageScrollRepeatAdd = func(intervalMS uint, cb *glib.SourceFunc) uint { | |
| 872 | - | if intervalMS != uint(pageScrollRepeatInterval/time.Millisecond) { | |
| 873 | - | t.Fatalf("repeat interval = %dms, want %dms", intervalMS, uint(pageScrollRepeatInterval/time.Millisecond)) | |
| 874 | - | } | |
| 875 | - | repeatTick = *cb | |
| 876 | - | return 99 | |
| 877 | - | } | |
| 878 | - | h.pageScrollRepeatRemove = func(id uint) bool { | |
| 879 | - | removedTimer = id | |
| 880 | - | return true | |
| 881 | - | } | |
| 882 | - | ||
| 861 | + | func TestPageScrollRepeat_FirstKeyDownDispatchesTapAndArmsHold(t *testing.T) { | |
| 862 | + | h, now, tick := newPageScrollRepeatTestHandler(t) | |
| 883 | 863 | actionCalls := 0 | |
| 884 | - | h.SetOnAction(func(ctx context.Context, action Action) error { | |
| 864 | + | h.SetOnAction(func(_ context.Context, action Action) error { | |
| 885 | 865 | actionCalls++ | |
| 886 | 866 | if action != ActionPageScrollDown { | |
| 887 | 867 | t.Fatalf("action = %s, want %s", action, ActionPageScrollDown) | |
| @@ -889,57 +869,144 @@ func TestHandleKeyPress_PageModeSmoothRepeaterStartsAfterHeldRepeat(t *testing.T | |||
| 889 | 869 | return nil | |
| 890 | 870 | }) | |
| 891 | 871 | ||
| 892 | - | if !h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue) { | |
| 893 | - | t.Fatal("Ctrl+Y should be consumed") | |
| 894 | - | } | |
| 895 | 872 | if !h.handleKeyPress(uint('j'), 0, 0) { | |
| 896 | 873 | t.Fatal("first J press should be consumed") | |
| 897 | 874 | } | |
| 898 | 875 | if actionCalls != 1 { | |
| 899 | - | t.Fatalf("actionCalls after first press = %d, want 1", actionCalls) | |
| 876 | + | t.Fatalf("tap dispatches = %d, want 1", actionCalls) | |
| 900 | 877 | } | |
| 901 | - | if h.pageScrollRepeatTimer != 0 { | |
| 902 | - | t.Fatal("smooth repeater should not start before the first held repeat arrives") | |
| 878 | + | if *tick == nil { | |
| 879 | + | t.Fatal("first keydown must arm the hold source without an OS repeat") | |
| 903 | 880 | } | |
| 904 | 881 | ||
| 905 | - | if !h.handleKeyPress(uint('j'), 0, 0) { | |
| 906 | - | t.Fatal("held J repeat should be consumed") | |
| 882 | + | *now = now.Add(pageScrollHoldDelay - time.Millisecond) | |
| 883 | + | if !(*tick)(0) { | |
| 884 | + | t.Fatal("hold source should remain armed before the delay") | |
| 907 | 885 | } | |
| 908 | - | if actionCalls != 2 { | |
| 909 | - | t.Fatalf("actionCalls after held repeat = %d, want 2", actionCalls) | |
| 910 | - | } | |
| 911 | - | if h.pageScrollRepeatTimer != 99 { | |
| 912 | - | t.Fatalf("pageScrollRepeatTimer = %d, want 99", h.pageScrollRepeatTimer) | |
| 913 | - | } | |
| 914 | - | if repeatTick == nil { | |
| 915 | - | t.Fatal("smooth repeater callback should be registered") | |
| 886 | + | if actionCalls != 1 { | |
| 887 | + | t.Fatalf("dispatches before hold delay = %d, want 1 tap", actionCalls) | |
| 916 | 888 | } | |
| 889 | + | } | |
| 917 | 890 | ||
| 918 | - | if !repeatTick(0) { | |
| 919 | - | t.Fatal("smooth repeater tick should continue while key remains held") | |
| 891 | + | func TestPageScrollRepeat_HoldStartsWithoutOSRepeat(t *testing.T) { | |
| 892 | + | h, now, tick := newPageScrollRepeatTestHandler(t) | |
| 893 | + | continuous := 0 | |
| 894 | + | h.SetOnAction(func(context.Context, Action) error { return nil }) | |
| 895 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, action Action, phase PageScrollPhase) error { | |
| 896 | + | if action != ActionPageScrollDown || phase != PageScrollContinuous { | |
| 897 | + | t.Fatalf("lifecycle = (%s, %v), want (%s, continuous)", action, phase, ActionPageScrollDown) | |
| 898 | + | } | |
| 899 | + | continuous++ | |
| 900 | + | return nil | |
| 901 | + | }) | |
| 902 | + | ||
| 903 | + | h.handleKeyPress(uint('j'), 0, 0) | |
| 904 | + | *now = now.Add(pageScrollHoldDelay) | |
| 905 | + | if !(*tick)(0) { | |
| 906 | + | t.Fatal("hold source stopped at the delay") | |
| 920 | 907 | } | |
| 921 | - | if actionCalls != 3 { | |
| 922 | - | t.Fatalf("actionCalls after smooth repeater tick = %d, want 3", actionCalls) | |
| 908 | + | if continuous != 1 { | |
| 909 | + | t.Fatalf("continuous dispatches = %d, want 1 without a second keydown", continuous) | |
| 923 | 910 | } | |
| 911 | + | } | |
| 924 | 912 | ||
| 925 | - | if !h.handleKeyPress(uint('j'), 0, 0) { | |
| 926 | - | t.Fatal("subsequent physical repeats should remain consumed") | |
| 927 | - | } | |
| 928 | - | if actionCalls != 3 { | |
| 929 | - | t.Fatalf("physical repeats should defer to smooth repeater; got %d calls", actionCalls) | |
| 913 | + | func TestPageScrollRepeat_FixedCadenceIgnoresDisplayAndOSRepeatRates(t *testing.T) { | |
| 914 | + | const simulatedHold = time.Second | |
| 915 | + | simulate := func(t *testing.T, externalHz int) int { | |
| 916 | + | t.Helper() | |
| 917 | + | h, now, tick := newPageScrollRepeatTestHandler(t) | |
| 918 | + | tapDistance := 0 | |
| 919 | + | continuousDistance := 0 | |
| 920 | + | h.SetOnAction(func(context.Context, Action) error { | |
| 921 | + | tapDistance += 80 | |
| 922 | + | return nil | |
| 923 | + | }) | |
| 924 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, _ Action, phase PageScrollPhase) error { | |
| 925 | + | if phase == PageScrollContinuous { | |
| 926 | + | continuousDistance += 80 | |
| 927 | + | } | |
| 928 | + | return nil | |
| 929 | + | }) | |
| 930 | + | h.handleKeyPress(uint('j'), 0, 0) | |
| 931 | + | ||
| 932 | + | start := *now | |
| 933 | + | nextExternal := time.Second / time.Duration(externalHz) | |
| 934 | + | for elapsed := pageScrollCadence; elapsed <= simulatedHold; elapsed += pageScrollCadence { | |
| 935 | + | *now = start.Add(elapsed) | |
| 936 | + | for elapsed >= nextExternal { | |
| 937 | + | h.handleKeyPress(uint('j'), 0, 0) // ignored OS auto-repeat | |
| 938 | + | nextExternal += time.Second / time.Duration(externalHz) | |
| 939 | + | } | |
| 940 | + | if !(*tick)(0) { | |
| 941 | + | t.Fatalf("source stopped at %s", elapsed) | |
| 942 | + | } | |
| 943 | + | } | |
| 944 | + | return tapDistance + continuousDistance | |
| 930 | 945 | } | |
| 931 | 946 | ||
| 932 | - | h.handleKeyRelease(uint('j')) | |
| 933 | - | if removedTimer != 99 { | |
| 934 | - | t.Fatalf("removed timer id = %d, want 99", removedTimer) | |
| 947 | + | distance60 := simulate(t, 60) | |
| 948 | + | distance144 := simulate(t, 144) | |
| 949 | + | t.Logf("simulated one-second distances: 60 Hz=%d px, 144 Hz=%d px; tolerance=80 px", distance60, distance144) | |
| 950 | + | // A fixed 16 ms source makes the results exact in this deterministic model; | |
| 951 | + | // the documented runtime tolerance is one 80 px cadence step at timer edges. | |
| 952 | + | if diff := distance60 - distance144; diff < -80 || diff > 80 { | |
| 953 | + | t.Fatalf("60 Hz distance=%d, 144 Hz distance=%d; tolerance=80 px", distance60, distance144) | |
| 935 | 954 | } | |
| 936 | - | if h.pageScrollRepeatTimer != 0 { | |
| 937 | - | t.Fatal("smooth repeater should stop on key release") | |
| 955 | + | } | |
| 956 | + | ||
| 957 | + | func TestPageScrollRepeat_ReleaseAndEscapeStopImmediately(t *testing.T) { | |
| 958 | + | for _, tc := range []struct { | |
| 959 | + | name string | |
| 960 | + | stop func(*KeyboardHandler) | |
| 961 | + | }{ | |
| 962 | + | {name: "release", stop: func(h *KeyboardHandler) { h.handleKeyRelease(uint('j')) }}, | |
| 963 | + | {name: "escape", stop: func(h *KeyboardHandler) { h.handleKeyPress(uint(gdk.KEY_Escape), 0, 0) }}, | |
| 964 | + | } { | |
| 965 | + | t.Run(tc.name, func(t *testing.T) { | |
| 966 | + | h, now, tick := newPageScrollRepeatTestHandler(t) | |
| 967 | + | removed := 0 | |
| 968 | + | stops := 0 | |
| 969 | + | h.pageScrollRepeatRemove = func(uint) bool { removed++; return true } | |
| 970 | + | h.SetOnAction(func(context.Context, Action) error { return nil }) | |
| 971 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, _ Action, phase PageScrollPhase) error { | |
| 972 | + | if phase == PageScrollStop { | |
| 973 | + | stops++ | |
| 974 | + | } | |
| 975 | + | return nil | |
| 976 | + | }) | |
| 977 | + | h.handleKeyPress(uint('j'), 0, 0) | |
| 978 | + | *now = now.Add(pageScrollHoldDelay) | |
| 979 | + | (*tick)(0) | |
| 980 | + | ||
| 981 | + | tc.stop(h) | |
| 982 | + | if removed != 1 || stops != 1 { | |
| 983 | + | t.Fatalf("removed=%d stops=%d, want 1/1", removed, stops) | |
| 984 | + | } | |
| 985 | + | if (*tick)(0) { | |
| 986 | + | t.Fatal("retained callback must stop immediately") | |
| 987 | + | } | |
| 988 | + | }) | |
| 938 | 989 | } | |
| 939 | - | if repeatTick(0) { | |
| 940 | - | t.Fatal("smooth repeater tick should stop after key release") | |
| 990 | + | } | |
| 991 | + | ||
| 992 | + | func newPageScrollRepeatTestHandler(t *testing.T) (*KeyboardHandler, *time.Time, *glib.SourceFunc) { | |
| 993 | + | t.Helper() | |
| 994 | + | workspace := newTestWorkspace() | |
| 995 | + | workspace.PageMode = entity.PageModeConfig{ | |
| 996 | + | Actions: map[string]entity.ActionBinding{"page-scroll-down": {Keys: []string{"j"}}}, | |
| 941 | 997 | } | |
| 942 | - | if actionCalls != 3 { | |
| 943 | - | t.Fatalf("no extra actions expected after release, got %d", actionCalls) | |
| 998 | + | h := NewKeyboardHandler(context.Background(), workspace, newTestSession()) | |
| 999 | + | h.EnterPageMode() | |
| 1000 | + | now := time.Unix(1_000, 0) | |
| 1001 | + | var tick glib.SourceFunc | |
| 1002 | + | h.pageScrollNow = func() time.Time { return now } | |
| 1003 | + | h.pageScrollRepeatAdd = func(intervalMS uint, cb *glib.SourceFunc) uint { | |
| 1004 | + | if intervalMS != uint(pageScrollCadence/time.Millisecond) { | |
| 1005 | + | t.Fatalf("cadence = %dms, want %dms", intervalMS, pageScrollCadence/time.Millisecond) | |
| 1006 | + | } | |
| 1007 | + | tick = *cb | |
| 1008 | + | return 99 | |
| 944 | 1009 | } | |
| 1010 | + | h.pageScrollRepeatRemove = func(uint) bool { return true } | |
| 1011 | + | return h, &now, &tick | |
| 945 | 1012 | } | |
| @@ -418,14 +418,16 @@ func TestHandleKeyPress_PageModeStaysActiveAfterScroll(t *testing.T) { | |||||
| 418 | } | 418 | } | ||
| 419 | 419 | ||||
| 420 | var repeatTick glib.SourceFunc | 420 | var repeatTick glib.SourceFunc | ||
| 421 | + | repeatNow := time.Unix(2_000, 0) | |||
| 422 | + | h.pageScrollNow = func() time.Time { return repeatNow } | |||
| 421 | h.pageScrollRepeatAdd = func(_ uint, cb *glib.SourceFunc) uint { | 423 | h.pageScrollRepeatAdd = func(_ uint, cb *glib.SourceFunc) uint { | ||
| 422 | repeatTick = *cb | 424 | repeatTick = *cb | ||
| 423 | return 1 | 425 | return 1 | ||
| 424 | } | 426 | } | ||
| 425 | h.pageScrollRepeatRemove = func(uint) bool { return true } | 427 | h.pageScrollRepeatRemove = func(uint) bool { return true } | ||
| 426 | 428 | ||||
| 427 | - | // Repeated scroll actions should stay in page mode and transition onto the | 429 | + | // Scroll actions stay in page mode; the first press arms the smooth |
| 428 | - | // smooth repeater instead of depending on every physical key repeat. | 430 | + | // repeater and later physical auto-repeat events are ignored. |
| 429 | for i := 0; i < 2; i++ { | 431 | for i := 0; i < 2; i++ { | ||
| 430 | scrolled := h.handleKeyPress(uint('j'), 0, 0) | 432 | scrolled := h.handleKeyPress(uint('j'), 0, 0) | ||
| 431 | if !scrolled { | 433 | if !scrolled { | ||
| @@ -438,6 +440,7 @@ func TestHandleKeyPress_PageModeStaysActiveAfterScroll(t *testing.T) { | |||||
| 438 | if repeatTick == nil { | 440 | if repeatTick == nil { | ||
| 439 | t.Fatal("expected held page scroll to register smooth repeater") | 441 | t.Fatal("expected held page scroll to register smooth repeater") | ||
| 440 | } | 442 | } | ||
| 443 | + | repeatNow = repeatNow.Add(pageScrollHoldDelay) | |||
| 441 | for i := 0; i < 3; i++ { | 444 | for i := 0; i < 3; i++ { | ||
| 442 | if !repeatTick(0) { | 445 | if !repeatTick(0) { | ||
| 443 | t.Fatalf("repeat tick %d stopped unexpectedly", i) | 446 | t.Fatalf("repeat tick %d stopped unexpectedly", i) | ||
| @@ -455,8 +458,8 @@ func TestHandleKeyPress_PageModeStaysActiveAfterScroll(t *testing.T) { | |||||
| 455 | } | 458 | } | ||
| 456 | 459 | ||||
| 457 | // Ensure scroll actions were dispatched | 460 | // Ensure scroll actions were dispatched | ||
| 458 | - | if actionCalls != 6 { | 461 | + | if actionCalls != 5 { |
| 459 | - | t.Fatalf("action calls = %d, want 6 (5 down + 1 up)", actionCalls) | 462 | + | t.Fatalf("action calls = %d, want 5 (4 down + 1 up)", actionCalls) |
| 460 | } | 463 | } | ||
| 461 | } | 464 | } | ||
| 462 | 465 | ||||
| @@ -855,33 +858,10 @@ func TestHandleKeyPress_PageModeBlocksGlobalShortcutFallback(t *testing.T) { | |||||
| 855 | } | 858 | } | ||
| 856 | } | 859 | } | ||
| 857 | 860 | ||||
| 858 | - | func TestHandleKeyPress_PageModeSmoothRepeaterStartsAfterHeldRepeat(t *testing.T) { | 861 | + | func TestPageScrollRepeat_FirstKeyDownDispatchesTapAndArmsHold(t *testing.T) { |
| 859 | - | ctx := context.Background() | 862 | + | h, now, tick := newPageScrollRepeatTestHandler(t) |
| 860 | - | workspace := newTestWorkspace() | |||
| 861 | - | workspace.PageMode = entity.PageModeConfig{ | |||
| 862 | - | ActivationShortcut: "ctrl+y", | |||
| 863 | - | Actions: map[string]entity.ActionBinding{ | |||
| 864 | - | "page-scroll-down": {Keys: []string{"j"}}, | |||
| 865 | - | }, | |||
| 866 | - | } | |||
| 867 | - | ||||
| 868 | - | h := NewKeyboardHandler(ctx, workspace, newTestSession()) | |||
| 869 | - | var repeatTick glib.SourceFunc | |||
| 870 | - | var removedTimer uint | |||
| 871 | - | h.pageScrollRepeatAdd = func(intervalMS uint, cb *glib.SourceFunc) uint { | |||
| 872 | - | if intervalMS != uint(pageScrollRepeatInterval/time.Millisecond) { | |||
| 873 | - | t.Fatalf("repeat interval = %dms, want %dms", intervalMS, uint(pageScrollRepeatInterval/time.Millisecond)) | |||
| 874 | - | } | |||
| 875 | - | repeatTick = *cb | |||
| 876 | - | return 99 | |||
| 877 | - | } | |||
| 878 | - | h.pageScrollRepeatRemove = func(id uint) bool { | |||
| 879 | - | removedTimer = id | |||
| 880 | - | return true | |||
| 881 | - | } | |||
| 882 | - | ||||
| 883 | actionCalls := 0 | 863 | actionCalls := 0 | ||
| 884 | - | h.SetOnAction(func(ctx context.Context, action Action) error { | 864 | + | h.SetOnAction(func(_ context.Context, action Action) error { |
| 885 | actionCalls++ | 865 | actionCalls++ | ||
| 886 | if action != ActionPageScrollDown { | 866 | if action != ActionPageScrollDown { | ||
| 887 | t.Fatalf("action = %s, want %s", action, ActionPageScrollDown) | 867 | t.Fatalf("action = %s, want %s", action, ActionPageScrollDown) | ||
| @@ -889,57 +869,144 @@ func TestHandleKeyPress_PageModeSmoothRepeaterStartsAfterHeldRepeat(t *testing.T | |||||
| 889 | return nil | 869 | return nil | ||
| 890 | }) | 870 | }) | ||
| 891 | 871 | ||||
| 892 | - | if !h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue) { | |||
| 893 | - | t.Fatal("Ctrl+Y should be consumed") | |||
| 894 | - | } | |||
| 895 | if !h.handleKeyPress(uint('j'), 0, 0) { | 872 | if !h.handleKeyPress(uint('j'), 0, 0) { | ||
| 896 | t.Fatal("first J press should be consumed") | 873 | t.Fatal("first J press should be consumed") | ||
| 897 | } | 874 | } | ||
| 898 | if actionCalls != 1 { | 875 | if actionCalls != 1 { | ||
| 899 | - | t.Fatalf("actionCalls after first press = %d, want 1", actionCalls) | 876 | + | t.Fatalf("tap dispatches = %d, want 1", actionCalls) |
| 900 | } | 877 | } | ||
| 901 | - | if h.pageScrollRepeatTimer != 0 { | 878 | + | if *tick == nil { |
| 902 | - | t.Fatal("smooth repeater should not start before the first held repeat arrives") | 879 | + | t.Fatal("first keydown must arm the hold source without an OS repeat") |
| 903 | } | 880 | } | ||
| 904 | 881 | ||||
| 905 | - | if !h.handleKeyPress(uint('j'), 0, 0) { | 882 | + | *now = now.Add(pageScrollHoldDelay - time.Millisecond) |
| 906 | - | t.Fatal("held J repeat should be consumed") | 883 | + | if !(*tick)(0) { |
| 884 | + | t.Fatal("hold source should remain armed before the delay") | |||
| 907 | } | 885 | } | ||
| 908 | - | if actionCalls != 2 { | 886 | + | if actionCalls != 1 { |
| 909 | - | t.Fatalf("actionCalls after held repeat = %d, want 2", actionCalls) | 887 | + | t.Fatalf("dispatches before hold delay = %d, want 1 tap", actionCalls) |
| 910 | - | } | |||
| 911 | - | if h.pageScrollRepeatTimer != 99 { | |||
| 912 | - | t.Fatalf("pageScrollRepeatTimer = %d, want 99", h.pageScrollRepeatTimer) | |||
| 913 | - | } | |||
| 914 | - | if repeatTick == nil { | |||
| 915 | - | t.Fatal("smooth repeater callback should be registered") | |||
| 916 | } | 888 | } | ||
| 889 | + | } | |||
| 917 | 890 | ||||
| 918 | - | if !repeatTick(0) { | 891 | + | func TestPageScrollRepeat_HoldStartsWithoutOSRepeat(t *testing.T) { |
| 919 | - | t.Fatal("smooth repeater tick should continue while key remains held") | 892 | + | h, now, tick := newPageScrollRepeatTestHandler(t) |
| 893 | + | continuous := 0 | |||
| 894 | + | h.SetOnAction(func(context.Context, Action) error { return nil }) | |||
| 895 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, action Action, phase PageScrollPhase) error { | |||
| 896 | + | if action != ActionPageScrollDown || phase != PageScrollContinuous { | |||
| 897 | + | t.Fatalf("lifecycle = (%s, %v), want (%s, continuous)", action, phase, ActionPageScrollDown) | |||
| 898 | + | } | |||
| 899 | + | continuous++ | |||
| 900 | + | return nil | |||
| 901 | + | }) | |||
| 902 | + | ||||
| 903 | + | h.handleKeyPress(uint('j'), 0, 0) | |||
| 904 | + | *now = now.Add(pageScrollHoldDelay) | |||
| 905 | + | if !(*tick)(0) { | |||
| 906 | + | t.Fatal("hold source stopped at the delay") | |||
| 920 | } | 907 | } | ||
| 921 | - | if actionCalls != 3 { | 908 | + | if continuous != 1 { |
| 922 | - | t.Fatalf("actionCalls after smooth repeater tick = %d, want 3", actionCalls) | 909 | + | t.Fatalf("continuous dispatches = %d, want 1 without a second keydown", continuous) |
| 923 | } | 910 | } | ||
| 911 | + | } | |||
| 924 | 912 | ||||
| 925 | - | if !h.handleKeyPress(uint('j'), 0, 0) { | 913 | + | func TestPageScrollRepeat_FixedCadenceIgnoresDisplayAndOSRepeatRates(t *testing.T) { |
| 926 | - | t.Fatal("subsequent physical repeats should remain consumed") | 914 | + | const simulatedHold = time.Second |
| 927 | - | } | 915 | + | simulate := func(t *testing.T, externalHz int) int { |
| 928 | - | if actionCalls != 3 { | 916 | + | t.Helper() |
| 929 | - | t.Fatalf("physical repeats should defer to smooth repeater; got %d calls", actionCalls) | 917 | + | h, now, tick := newPageScrollRepeatTestHandler(t) |
| 918 | + | tapDistance := 0 | |||
| 919 | + | continuousDistance := 0 | |||
| 920 | + | h.SetOnAction(func(context.Context, Action) error { | |||
| 921 | + | tapDistance += 80 | |||
| 922 | + | return nil | |||
| 923 | + | }) | |||
| 924 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, _ Action, phase PageScrollPhase) error { | |||
| 925 | + | if phase == PageScrollContinuous { | |||
| 926 | + | continuousDistance += 80 | |||
| 927 | + | } | |||
| 928 | + | return nil | |||
| 929 | + | }) | |||
| 930 | + | h.handleKeyPress(uint('j'), 0, 0) | |||
| 931 | + | ||||
| 932 | + | start := *now | |||
| 933 | + | nextExternal := time.Second / time.Duration(externalHz) | |||
| 934 | + | for elapsed := pageScrollCadence; elapsed <= simulatedHold; elapsed += pageScrollCadence { | |||
| 935 | + | *now = start.Add(elapsed) | |||
| 936 | + | for elapsed >= nextExternal { | |||
| 937 | + | h.handleKeyPress(uint('j'), 0, 0) // ignored OS auto-repeat | |||
| 938 | + | nextExternal += time.Second / time.Duration(externalHz) | |||
| 939 | + | } | |||
| 940 | + | if !(*tick)(0) { | |||
| 941 | + | t.Fatalf("source stopped at %s", elapsed) | |||
| 942 | + | } | |||
| 943 | + | } | |||
| 944 | + | return tapDistance + continuousDistance | |||
| 930 | } | 945 | } | ||
| 931 | 946 | ||||
| 932 | - | h.handleKeyRelease(uint('j')) | 947 | + | distance60 := simulate(t, 60) |
| 933 | - | if removedTimer != 99 { | 948 | + | distance144 := simulate(t, 144) |
| 934 | - | t.Fatalf("removed timer id = %d, want 99", removedTimer) | 949 | + | t.Logf("simulated one-second distances: 60 Hz=%d px, 144 Hz=%d px; tolerance=80 px", distance60, distance144) |
| 950 | + | // A fixed 16 ms source makes the results exact in this deterministic model; | |||
| 951 | + | // the documented runtime tolerance is one 80 px cadence step at timer edges. | |||
| 952 | + | if diff := distance60 - distance144; diff < -80 || diff > 80 { | |||
| 953 | + | t.Fatalf("60 Hz distance=%d, 144 Hz distance=%d; tolerance=80 px", distance60, distance144) | |||
| 935 | } | 954 | } | ||
| 936 | - | if h.pageScrollRepeatTimer != 0 { | 955 | + | } |
| 937 | - | t.Fatal("smooth repeater should stop on key release") | 956 | + | |
| 957 | + | func TestPageScrollRepeat_ReleaseAndEscapeStopImmediately(t *testing.T) { | |||
| 958 | + | for _, tc := range []struct { | |||
| 959 | + | name string | |||
| 960 | + | stop func(*KeyboardHandler) | |||
| 961 | + | }{ | |||
| 962 | + | {name: "release", stop: func(h *KeyboardHandler) { h.handleKeyRelease(uint('j')) }}, | |||
| 963 | + | {name: "escape", stop: func(h *KeyboardHandler) { h.handleKeyPress(uint(gdk.KEY_Escape), 0, 0) }}, | |||
| 964 | + | } { | |||
| 965 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 966 | + | h, now, tick := newPageScrollRepeatTestHandler(t) | |||
| 967 | + | removed := 0 | |||
| 968 | + | stops := 0 | |||
| 969 | + | h.pageScrollRepeatRemove = func(uint) bool { removed++; return true } | |||
| 970 | + | h.SetOnAction(func(context.Context, Action) error { return nil }) | |||
| 971 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, _ Action, phase PageScrollPhase) error { | |||
| 972 | + | if phase == PageScrollStop { | |||
| 973 | + | stops++ | |||
| 974 | + | } | |||
| 975 | + | return nil | |||
| 976 | + | }) | |||
| 977 | + | h.handleKeyPress(uint('j'), 0, 0) | |||
| 978 | + | *now = now.Add(pageScrollHoldDelay) | |||
| 979 | + | (*tick)(0) | |||
| 980 | + | ||||
| 981 | + | tc.stop(h) | |||
| 982 | + | if removed != 1 || stops != 1 { | |||
| 983 | + | t.Fatalf("removed=%d stops=%d, want 1/1", removed, stops) | |||
| 984 | + | } | |||
| 985 | + | if (*tick)(0) { | |||
| 986 | + | t.Fatal("retained callback must stop immediately") | |||
| 987 | + | } | |||
| 988 | + | }) | |||
| 938 | } | 989 | } | ||
| 939 | - | if repeatTick(0) { | 990 | + | } |
| 940 | - | t.Fatal("smooth repeater tick should stop after key release") | 991 | + | |
| 992 | + | func newPageScrollRepeatTestHandler(t *testing.T) (*KeyboardHandler, *time.Time, *glib.SourceFunc) { | |||
| 993 | + | t.Helper() | |||
| 994 | + | workspace := newTestWorkspace() | |||
| 995 | + | workspace.PageMode = entity.PageModeConfig{ | |||
| 996 | + | Actions: map[string]entity.ActionBinding{"page-scroll-down": {Keys: []string{"j"}}}, | |||
| 941 | } | 997 | } | ||
| 942 | - | if actionCalls != 3 { | 998 | + | h := NewKeyboardHandler(context.Background(), workspace, newTestSession()) |
| 943 | - | t.Fatalf("no extra actions expected after release, got %d", actionCalls) | 999 | + | h.EnterPageMode() |
| 1000 | + | now := time.Unix(1_000, 0) | |||
| 1001 | + | var tick glib.SourceFunc | |||
| 1002 | + | h.pageScrollNow = func() time.Time { return now } | |||
| 1003 | + | h.pageScrollRepeatAdd = func(intervalMS uint, cb *glib.SourceFunc) uint { | |||
| 1004 | + | if intervalMS != uint(pageScrollCadence/time.Millisecond) { | |||
| 1005 | + | t.Fatalf("cadence = %dms, want %dms", intervalMS, pageScrollCadence/time.Millisecond) | |||
| 1006 | + | } | |||
| 1007 | + | tick = *cb | |||
| 1008 | + | return 99 | |||
| 944 | } | 1009 | } | ||
| 1010 | + | h.pageScrollRepeatRemove = func(uint) bool { return true } | |||
| 1011 | + | return h, &now, &tick | |||
| 945 | } | 1012 | } | ||