fix(page-mode): harden scroll scheduling and repeat
- commit
- 3ad27cd15d5a570ef019e547dcbf6ba95a0c8c4a
- author
- brice · · 2026-07-29 05:04 UTC
- parent
- 5b1cfa8
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +209 -67
M internal/infrastructure/cef/webview.go +3 -1
| @@ -2475,11 +2475,13 @@ func (wv *WebView) ensurePageScrollFlush() { | |||
| 2475 | 2475 | } | |
| 2476 | 2476 | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | |
| 2477 | 2477 | if task == nil || cefPostTask(purecef.ThreadIDTidUi, task) != 1 { | |
| 2478 | + | // Schedule failed: drop held (cancel semantics) and synchronously drain | |
| 2479 | + | // any queued tap so it cannot remain stranded with flushPending cleared. | |
| 2478 | 2480 | q.mu.Lock() | |
| 2479 | - | q.flushPending = false | |
| 2480 | 2481 | q.heldDX = 0 | |
| 2481 | 2482 | q.heldDY = 0 | |
| 2482 | 2483 | q.mu.Unlock() | |
| 2484 | + | wv.flushPageScroll() | |
| 2483 | 2485 | } | |
| 2484 | 2486 | } | |
| 2485 | 2487 | ||
| @@ -2475,11 +2475,13 @@ func (wv *WebView) ensurePageScrollFlush() { | |||||
| 2475 | } | 2475 | } | ||
| 2476 | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | 2476 | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | ||
| 2477 | if task == nil || cefPostTask(purecef.ThreadIDTidUi, task) != 1 { | 2477 | if task == nil || cefPostTask(purecef.ThreadIDTidUi, task) != 1 { | ||
| 2478 | + | // Schedule failed: drop held (cancel semantics) and synchronously drain | |||
| 2479 | + | // any queued tap so it cannot remain stranded with flushPending cleared. | |||
| 2478 | q.mu.Lock() | 2480 | q.mu.Lock() | ||
| 2479 | - | q.flushPending = false | |||
| 2480 | q.heldDX = 0 | 2481 | q.heldDX = 0 | ||
| 2481 | q.heldDY = 0 | 2482 | q.heldDY = 0 | ||
| 2482 | q.mu.Unlock() | 2483 | q.mu.Unlock() | ||
| 2484 | + | wv.flushPageScroll() | |||
| 2483 | } | 2485 | } | ||
| 2484 | } | 2486 | } | ||
| 2485 | 2487 | ||||
M internal/infrastructure/cef/webview_scroll_test.go +38 -0
| @@ -446,6 +446,44 @@ func TestScrollPage_AllowsAtMostOneSuccessorAfterInFlightFlush(t *testing.T) { | |||
| 446 | 446 | } | |
| 447 | 447 | } | |
| 448 | 448 | ||
| 449 | + | func TestEnsurePageScrollFlush_PostTaskFailureDrainsTapAndDropsHeld(t *testing.T) { | |
| 450 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |
| 451 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |
| 452 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |
| 453 | + | cefPostTask = func(_ purecef.ThreadID, _ purecef.Task) int32 { return 0 } | |
| 454 | + | ||
| 455 | + | browser := cefmocks.NewMockBrowser(t) | |
| 456 | + | frame := cefmocks.NewMockFrame(t) | |
| 457 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |
| 458 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |
| 459 | + | return strings.Contains(script, "var dx=0,dy=80") && !strings.Contains(script, "dy=160") | |
| 460 | + | }), "", int32(0)).Once() | |
| 461 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |
| 462 | + | ||
| 463 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |
| 464 | + | t.Fatal(err) | |
| 465 | + | } | |
| 466 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |
| 467 | + | t.Fatal(err) | |
| 468 | + | } | |
| 469 | + | ||
| 470 | + | q := &wv.pageScrollQueue | |
| 471 | + | q.mu.Lock() | |
| 472 | + | tapDX, tapDY := q.tapDX, q.tapDY | |
| 473 | + | heldDX, heldDY := q.heldDX, q.heldDY | |
| 474 | + | pending := q.flushPending | |
| 475 | + | q.mu.Unlock() | |
| 476 | + | if tapDX != 0 || tapDY != 0 { | |
| 477 | + | t.Fatalf("tap deltas stranded after PostTask failure: tap=(%d,%d)", tapDX, tapDY) | |
| 478 | + | } | |
| 479 | + | if heldDX != 0 || heldDY != 0 { | |
| 480 | + | t.Fatalf("held deltas must keep cancel-on-schedule-failure semantics: held=(%d,%d)", heldDX, heldDY) | |
| 481 | + | } | |
| 482 | + | if pending { | |
| 483 | + | t.Fatal("flushPending must clear after PostTask failure drain") | |
| 484 | + | } | |
| 485 | + | } | |
| 486 | + | ||
| 449 | 487 | func TestScrollPage_Destroyed_ReturnsError(t *testing.T) { | |
| 450 | 488 | wv := &WebView{} | |
| 451 | 489 | wv.destroyed.Store(true) | |
| @@ -446,6 +446,44 @@ func TestScrollPage_AllowsAtMostOneSuccessorAfterInFlightFlush(t *testing.T) { | |||||
| 446 | } | 446 | } | ||
| 447 | } | 447 | } | ||
| 448 | 448 | ||||
| 449 | + | func TestEnsurePageScrollFlush_PostTaskFailureDrainsTapAndDropsHeld(t *testing.T) { | |||
| 450 | + | oldNewTask, oldPostTask := cefNewTask, cefPostTask | |||
| 451 | + | defer func() { cefNewTask, cefPostTask = oldNewTask, oldPostTask }() | |||
| 452 | + | cefNewTask = func(task purecef.Task) purecef.Task { return task } | |||
| 453 | + | cefPostTask = func(_ purecef.ThreadID, _ purecef.Task) int32 { return 0 } | |||
| 454 | + | ||||
| 455 | + | browser := cefmocks.NewMockBrowser(t) | |||
| 456 | + | frame := cefmocks.NewMockFrame(t) | |||
| 457 | + | browser.EXPECT().GetMainFrame().Return(frame).Once() | |||
| 458 | + | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(func(script string) bool { | |||
| 459 | + | return strings.Contains(script, "var dx=0,dy=80") && !strings.Contains(script, "dy=160") | |||
| 460 | + | }), "", int32(0)).Once() | |||
| 461 | + | wv := &WebView{engine: &Engine{}, browser: browser} | |||
| 462 | + | ||||
| 463 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80}); err != nil { | |||
| 464 | + | t.Fatal(err) | |||
| 465 | + | } | |||
| 466 | + | if err := wv.ScrollPage(context.Background(), port.PageScrollRequest{FallbackDY: 80, Continuous: true}); err != nil { | |||
| 467 | + | t.Fatal(err) | |||
| 468 | + | } | |||
| 469 | + | ||||
| 470 | + | q := &wv.pageScrollQueue | |||
| 471 | + | q.mu.Lock() | |||
| 472 | + | tapDX, tapDY := q.tapDX, q.tapDY | |||
| 473 | + | heldDX, heldDY := q.heldDX, q.heldDY | |||
| 474 | + | pending := q.flushPending | |||
| 475 | + | q.mu.Unlock() | |||
| 476 | + | if tapDX != 0 || tapDY != 0 { | |||
| 477 | + | t.Fatalf("tap deltas stranded after PostTask failure: tap=(%d,%d)", tapDX, tapDY) | |||
| 478 | + | } | |||
| 479 | + | if heldDX != 0 || heldDY != 0 { | |||
| 480 | + | t.Fatalf("held deltas must keep cancel-on-schedule-failure semantics: held=(%d,%d)", heldDX, heldDY) | |||
| 481 | + | } | |||
| 482 | + | if pending { | |||
| 483 | + | t.Fatal("flushPending must clear after PostTask failure drain") | |||
| 484 | + | } | |||
| 485 | + | } | |||
| 486 | + | ||||
| 449 | func TestScrollPage_Destroyed_ReturnsError(t *testing.T) { | 487 | func TestScrollPage_Destroyed_ReturnsError(t *testing.T) { | ||
| 450 | wv := &WebView{} | 488 | wv := &WebView{} | ||
| 451 | wv.destroyed.Store(true) | 489 | wv.destroyed.Store(true) | ||
M internal/ui/dispatcher/keyboard.go +48 -66
| @@ -266,63 +266,13 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||
| 266 | 266 | input.ActionZoomIn: func(ctx context.Context) error { return d.handleZoom(ctx, "in") }, | |
| 267 | 267 | input.ActionZoomOut: func(ctx context.Context) error { return d.handleZoom(ctx, "out") }, | |
| 268 | 268 | input.ActionZoomReset: func(ctx context.Context) error { return d.handleZoom(ctx, "reset") }, | |
| 269 | - | // Page mode scroll actions | |
| 270 | - | // Each maps a keyboard action to its semantic PageScrollCommand | |
| 271 | - | // and triggers a pane-local visual pulse via the registered callback. | |
| 272 | - | input.ActionPageScrollLeft: func(ctx context.Context) error { | |
| 273 | - | return d.withActiveWebView(ctx, "page scroll left", func(wv port.WebView) error { | |
| 274 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollLeft); err != nil { | |
| 275 | - | return err | |
| 276 | - | } | |
| 277 | - | d.triggerPulse(ctx, false) | |
| 278 | - | return nil | |
| 279 | - | }) | |
| 280 | - | }, | |
| 281 | - | input.ActionPageScrollRight: func(ctx context.Context) error { | |
| 282 | - | return d.withActiveWebView(ctx, "page scroll right", func(wv port.WebView) error { | |
| 283 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollRight); err != nil { | |
| 284 | - | return err | |
| 285 | - | } | |
| 286 | - | d.triggerPulse(ctx, false) | |
| 287 | - | return nil | |
| 288 | - | }) | |
| 289 | - | }, | |
| 290 | - | input.ActionPageScrollUp: func(ctx context.Context) error { | |
| 291 | - | return d.withActiveWebView(ctx, "page scroll up", func(wv port.WebView) error { | |
| 292 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollUp); err != nil { | |
| 293 | - | return err | |
| 294 | - | } | |
| 295 | - | d.triggerPulse(ctx, false) | |
| 296 | - | return nil | |
| 297 | - | }) | |
| 298 | - | }, | |
| 299 | - | input.ActionPageScrollDown: func(ctx context.Context) error { | |
| 300 | - | return d.withActiveWebView(ctx, "page scroll down", func(wv port.WebView) error { | |
| 301 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollDown); err != nil { | |
| 302 | - | return err | |
| 303 | - | } | |
| 304 | - | d.triggerPulse(ctx, false) | |
| 305 | - | return nil | |
| 306 | - | }) | |
| 307 | - | }, | |
| 308 | - | input.ActionPageScrollUpFast: func(ctx context.Context) error { | |
| 309 | - | return d.withActiveWebView(ctx, "page scroll up fast", func(wv port.WebView) error { | |
| 310 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollUpFast); err != nil { | |
| 311 | - | return err | |
| 312 | - | } | |
| 313 | - | d.triggerPulse(ctx, true) | |
| 314 | - | return nil | |
| 315 | - | }) | |
| 316 | - | }, | |
| 317 | - | input.ActionPageScrollDownFast: func(ctx context.Context) error { | |
| 318 | - | return d.withActiveWebView(ctx, "page scroll down fast", func(wv port.WebView) error { | |
| 319 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollDownFast); err != nil { | |
| 320 | - | return err | |
| 321 | - | } | |
| 322 | - | d.triggerPulse(ctx, true) | |
| 323 | - | return nil | |
| 324 | - | }) | |
| 325 | - | }, | |
| 269 | + | // Page mode scroll actions — single mapping drives tap pulse + command. | |
| 270 | + | input.ActionPageScrollLeft: d.pageScrollTapHandler(input.ActionPageScrollLeft), | |
| 271 | + | input.ActionPageScrollRight: d.pageScrollTapHandler(input.ActionPageScrollRight), | |
| 272 | + | input.ActionPageScrollUp: d.pageScrollTapHandler(input.ActionPageScrollUp), | |
| 273 | + | input.ActionPageScrollDown: d.pageScrollTapHandler(input.ActionPageScrollDown), | |
| 274 | + | input.ActionPageScrollUpFast: d.pageScrollTapHandler(input.ActionPageScrollUpFast), | |
| 275 | + | input.ActionPageScrollDownFast: d.pageScrollTapHandler(input.ActionPageScrollDownFast), | |
| 326 | 276 | // UI | |
| 327 | 277 | input.ActionOpenOmnibox: d.navCoord.OpenOmnibox, | |
| 328 | 278 | input.ActionOpenFind: d.handleFindOpen, | |
| @@ -376,7 +326,7 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||
| 376 | 326 | // Stop deliberately avoids the normal action map so it emits neither another | |
| 377 | 327 | // scroll tap nor visual pulse. | |
| 378 | 328 | func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, action input.Action, phase input.PageScrollPhase) error { | |
| 379 | - | cmd, ok := pageScrollCommand(action) | |
| 329 | + | spec, ok := pageScrollSpec(action) | |
| 380 | 330 | if !ok { | |
| 381 | 331 | return nil | |
| 382 | 332 | } | |
| @@ -384,27 +334,59 @@ func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, ac | |||
| 384 | 334 | if phase == input.PageScrollStop { | |
| 385 | 335 | return d.navCoord.StopPageScroll(ctx, wv) | |
| 386 | 336 | } | |
| 387 | - | return d.navCoord.ScrollWebViewContinuous(ctx, wv, cmd) | |
| 337 | + | return d.navCoord.ScrollWebViewContinuous(ctx, wv, spec.cmd) | |
| 388 | 338 | }) | |
| 389 | 339 | } | |
| 390 | 340 | ||
| 391 | - | func pageScrollCommand(action input.Action) (usecase.PageScrollCommand, bool) { | |
| 341 | + | type pageScrollActionSpec struct { | |
| 342 | + | cmd usecase.PageScrollCommand | |
| 343 | + | fast bool | |
| 344 | + | } | |
| 345 | + | ||
| 346 | + | // pageScrollSpec is the single source of truth for Page Mode action→command | |
| 347 | + | // and fast/slow pulse used by tap handlers and held lifecycle routing. | |
| 348 | + | func pageScrollSpec(action input.Action) (pageScrollActionSpec, bool) { | |
| 392 | 349 | switch action { | |
| 393 | 350 | case input.ActionPageScrollLeft: | |
| 394 | - | return usecase.PageScrollLeft, true | |
| 351 | + | return pageScrollActionSpec{cmd: usecase.PageScrollLeft, fast: false}, true | |
| 395 | 352 | case input.ActionPageScrollRight: | |
| 396 | - | return usecase.PageScrollRight, true | |
| 353 | + | return pageScrollActionSpec{cmd: usecase.PageScrollRight, fast: false}, true | |
| 397 | 354 | case input.ActionPageScrollUp: | |
| 398 | - | return usecase.PageScrollUp, true | |
| 355 | + | return pageScrollActionSpec{cmd: usecase.PageScrollUp, fast: false}, true | |
| 399 | 356 | case input.ActionPageScrollDown: | |
| 400 | - | return usecase.PageScrollDown, true | |
| 357 | + | return pageScrollActionSpec{cmd: usecase.PageScrollDown, fast: false}, true | |
| 401 | 358 | case input.ActionPageScrollUpFast: | |
| 402 | - | return usecase.PageScrollUpFast, true | |
| 359 | + | return pageScrollActionSpec{cmd: usecase.PageScrollUpFast, fast: true}, true | |
| 403 | 360 | case input.ActionPageScrollDownFast: | |
| 404 | - | return usecase.PageScrollDownFast, true | |
| 361 | + | return pageScrollActionSpec{cmd: usecase.PageScrollDownFast, fast: true}, true | |
| 405 | 362 | default: | |
| 363 | + | return pageScrollActionSpec{}, false | |
| 364 | + | } | |
| 365 | + | } | |
| 366 | + | ||
| 367 | + | func (d *KeyboardDispatcher) pageScrollTapHandler(action input.Action) func(context.Context) error { | |
| 368 | + | spec, ok := pageScrollSpec(action) | |
| 369 | + | if !ok { | |
| 370 | + | return func(context.Context) error { return nil } | |
| 371 | + | } | |
| 372 | + | label := "page scroll " + string(action) | |
| 373 | + | return func(ctx context.Context) error { | |
| 374 | + | return d.withActiveWebView(ctx, label, func(wv port.WebView) error { | |
| 375 | + | if err := d.navCoord.ScrollWebView(ctx, wv, spec.cmd); err != nil { | |
| 376 | + | return err | |
| 377 | + | } | |
| 378 | + | d.triggerPulse(ctx, spec.fast) | |
| 379 | + | return nil | |
| 380 | + | }) | |
| 381 | + | } | |
| 382 | + | } | |
| 383 | + | ||
| 384 | + | func pageScrollCommand(action input.Action) (usecase.PageScrollCommand, bool) { | |
| 385 | + | spec, ok := pageScrollSpec(action) | |
| 386 | + | if !ok { | |
| 406 | 387 | return 0, false | |
| 407 | 388 | } | |
| 389 | + | return spec.cmd, true | |
| 408 | 390 | } | |
| 409 | 391 | ||
| 410 | 392 | // Dispatch routes a keyboard action to the appropriate coordinator. | |
| @@ -266,63 +266,13 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||||
| 266 | input.ActionZoomIn: func(ctx context.Context) error { return d.handleZoom(ctx, "in") }, | 266 | input.ActionZoomIn: func(ctx context.Context) error { return d.handleZoom(ctx, "in") }, | ||
| 267 | input.ActionZoomOut: func(ctx context.Context) error { return d.handleZoom(ctx, "out") }, | 267 | input.ActionZoomOut: func(ctx context.Context) error { return d.handleZoom(ctx, "out") }, | ||
| 268 | input.ActionZoomReset: func(ctx context.Context) error { return d.handleZoom(ctx, "reset") }, | 268 | input.ActionZoomReset: func(ctx context.Context) error { return d.handleZoom(ctx, "reset") }, | ||
| 269 | - | // Page mode scroll actions | 269 | + | // Page mode scroll actions — single mapping drives tap pulse + command. |
| 270 | - | // Each maps a keyboard action to its semantic PageScrollCommand | 270 | + | input.ActionPageScrollLeft: d.pageScrollTapHandler(input.ActionPageScrollLeft), |
| 271 | - | // and triggers a pane-local visual pulse via the registered callback. | 271 | + | input.ActionPageScrollRight: d.pageScrollTapHandler(input.ActionPageScrollRight), |
| 272 | - | input.ActionPageScrollLeft: func(ctx context.Context) error { | 272 | + | input.ActionPageScrollUp: d.pageScrollTapHandler(input.ActionPageScrollUp), |
| 273 | - | return d.withActiveWebView(ctx, "page scroll left", func(wv port.WebView) error { | 273 | + | input.ActionPageScrollDown: d.pageScrollTapHandler(input.ActionPageScrollDown), |
| 274 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollLeft); err != nil { | 274 | + | input.ActionPageScrollUpFast: d.pageScrollTapHandler(input.ActionPageScrollUpFast), |
| 275 | - | return err | 275 | + | input.ActionPageScrollDownFast: d.pageScrollTapHandler(input.ActionPageScrollDownFast), |
| 276 | - | } | |||
| 277 | - | d.triggerPulse(ctx, false) | |||
| 278 | - | return nil | |||
| 279 | - | }) | |||
| 280 | - | }, | |||
| 281 | - | input.ActionPageScrollRight: func(ctx context.Context) error { | |||
| 282 | - | return d.withActiveWebView(ctx, "page scroll right", func(wv port.WebView) error { | |||
| 283 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollRight); err != nil { | |||
| 284 | - | return err | |||
| 285 | - | } | |||
| 286 | - | d.triggerPulse(ctx, false) | |||
| 287 | - | return nil | |||
| 288 | - | }) | |||
| 289 | - | }, | |||
| 290 | - | input.ActionPageScrollUp: func(ctx context.Context) error { | |||
| 291 | - | return d.withActiveWebView(ctx, "page scroll up", func(wv port.WebView) error { | |||
| 292 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollUp); err != nil { | |||
| 293 | - | return err | |||
| 294 | - | } | |||
| 295 | - | d.triggerPulse(ctx, false) | |||
| 296 | - | return nil | |||
| 297 | - | }) | |||
| 298 | - | }, | |||
| 299 | - | input.ActionPageScrollDown: func(ctx context.Context) error { | |||
| 300 | - | return d.withActiveWebView(ctx, "page scroll down", func(wv port.WebView) error { | |||
| 301 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollDown); err != nil { | |||
| 302 | - | return err | |||
| 303 | - | } | |||
| 304 | - | d.triggerPulse(ctx, false) | |||
| 305 | - | return nil | |||
| 306 | - | }) | |||
| 307 | - | }, | |||
| 308 | - | input.ActionPageScrollUpFast: func(ctx context.Context) error { | |||
| 309 | - | return d.withActiveWebView(ctx, "page scroll up fast", func(wv port.WebView) error { | |||
| 310 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollUpFast); err != nil { | |||
| 311 | - | return err | |||
| 312 | - | } | |||
| 313 | - | d.triggerPulse(ctx, true) | |||
| 314 | - | return nil | |||
| 315 | - | }) | |||
| 316 | - | }, | |||
| 317 | - | input.ActionPageScrollDownFast: func(ctx context.Context) error { | |||
| 318 | - | return d.withActiveWebView(ctx, "page scroll down fast", func(wv port.WebView) error { | |||
| 319 | - | if err := d.navCoord.ScrollWebView(ctx, wv, usecase.PageScrollDownFast); err != nil { | |||
| 320 | - | return err | |||
| 321 | - | } | |||
| 322 | - | d.triggerPulse(ctx, true) | |||
| 323 | - | return nil | |||
| 324 | - | }) | |||
| 325 | - | }, | |||
| 326 | // UI | 276 | // UI | ||
| 327 | input.ActionOpenOmnibox: d.navCoord.OpenOmnibox, | 277 | input.ActionOpenOmnibox: d.navCoord.OpenOmnibox, | ||
| 328 | input.ActionOpenFind: d.handleFindOpen, | 278 | input.ActionOpenFind: d.handleFindOpen, | ||
| @@ -376,7 +326,7 @@ func (d *KeyboardDispatcher) initActionHandlers() { | |||||
| 376 | // Stop deliberately avoids the normal action map so it emits neither another | 326 | // Stop deliberately avoids the normal action map so it emits neither another | ||
| 377 | // scroll tap nor visual pulse. | 327 | // scroll tap nor visual pulse. | ||
| 378 | func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, action input.Action, phase input.PageScrollPhase) error { | 328 | func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, action input.Action, phase input.PageScrollPhase) error { | ||
| 379 | - | cmd, ok := pageScrollCommand(action) | 329 | + | spec, ok := pageScrollSpec(action) |
| 380 | if !ok { | 330 | if !ok { | ||
| 381 | return nil | 331 | return nil | ||
| 382 | } | 332 | } | ||
| @@ -384,27 +334,59 @@ func (d *KeyboardDispatcher) DispatchPageScrollLifecycle(ctx context.Context, ac | |||||
| 384 | if phase == input.PageScrollStop { | 334 | if phase == input.PageScrollStop { | ||
| 385 | return d.navCoord.StopPageScroll(ctx, wv) | 335 | return d.navCoord.StopPageScroll(ctx, wv) | ||
| 386 | } | 336 | } | ||
| 387 | - | return d.navCoord.ScrollWebViewContinuous(ctx, wv, cmd) | 337 | + | return d.navCoord.ScrollWebViewContinuous(ctx, wv, spec.cmd) |
| 388 | }) | 338 | }) | ||
| 389 | } | 339 | } | ||
| 390 | 340 | ||||
| 391 | - | func pageScrollCommand(action input.Action) (usecase.PageScrollCommand, bool) { | 341 | + | type pageScrollActionSpec struct { |
| 342 | + | cmd usecase.PageScrollCommand | |||
| 343 | + | fast bool | |||
| 344 | + | } | |||
| 345 | + | ||||
| 346 | + | // pageScrollSpec is the single source of truth for Page Mode action→command | |||
| 347 | + | // and fast/slow pulse used by tap handlers and held lifecycle routing. | |||
| 348 | + | func pageScrollSpec(action input.Action) (pageScrollActionSpec, bool) { | |||
| 392 | switch action { | 349 | switch action { | ||
| 393 | case input.ActionPageScrollLeft: | 350 | case input.ActionPageScrollLeft: | ||
| 394 | - | return usecase.PageScrollLeft, true | 351 | + | return pageScrollActionSpec{cmd: usecase.PageScrollLeft, fast: false}, true |
| 395 | case input.ActionPageScrollRight: | 352 | case input.ActionPageScrollRight: | ||
| 396 | - | return usecase.PageScrollRight, true | 353 | + | return pageScrollActionSpec{cmd: usecase.PageScrollRight, fast: false}, true |
| 397 | case input.ActionPageScrollUp: | 354 | case input.ActionPageScrollUp: | ||
| 398 | - | return usecase.PageScrollUp, true | 355 | + | return pageScrollActionSpec{cmd: usecase.PageScrollUp, fast: false}, true |
| 399 | case input.ActionPageScrollDown: | 356 | case input.ActionPageScrollDown: | ||
| 400 | - | return usecase.PageScrollDown, true | 357 | + | return pageScrollActionSpec{cmd: usecase.PageScrollDown, fast: false}, true |
| 401 | case input.ActionPageScrollUpFast: | 358 | case input.ActionPageScrollUpFast: | ||
| 402 | - | return usecase.PageScrollUpFast, true | 359 | + | return pageScrollActionSpec{cmd: usecase.PageScrollUpFast, fast: true}, true |
| 403 | case input.ActionPageScrollDownFast: | 360 | case input.ActionPageScrollDownFast: | ||
| 404 | - | return usecase.PageScrollDownFast, true | 361 | + | return pageScrollActionSpec{cmd: usecase.PageScrollDownFast, fast: true}, true |
| 405 | default: | 362 | default: | ||
| 363 | + | return pageScrollActionSpec{}, false | |||
| 364 | + | } | |||
| 365 | + | } | |||
| 366 | + | ||||
| 367 | + | func (d *KeyboardDispatcher) pageScrollTapHandler(action input.Action) func(context.Context) error { | |||
| 368 | + | spec, ok := pageScrollSpec(action) | |||
| 369 | + | if !ok { | |||
| 370 | + | return func(context.Context) error { return nil } | |||
| 371 | + | } | |||
| 372 | + | label := "page scroll " + string(action) | |||
| 373 | + | return func(ctx context.Context) error { | |||
| 374 | + | return d.withActiveWebView(ctx, label, func(wv port.WebView) error { | |||
| 375 | + | if err := d.navCoord.ScrollWebView(ctx, wv, spec.cmd); err != nil { | |||
| 376 | + | return err | |||
| 377 | + | } | |||
| 378 | + | d.triggerPulse(ctx, spec.fast) | |||
| 379 | + | return nil | |||
| 380 | + | }) | |||
| 381 | + | } | |||
| 382 | + | } | |||
| 383 | + | ||||
| 384 | + | func pageScrollCommand(action input.Action) (usecase.PageScrollCommand, bool) { | |||
| 385 | + | spec, ok := pageScrollSpec(action) | |||
| 386 | + | if !ok { | |||
| 406 | return 0, false | 387 | return 0, false | ||
| 407 | } | 388 | } | ||
| 389 | + | return spec.cmd, true | |||
| 408 | } | 390 | } | ||
| 409 | 391 | ||||
| 410 | // Dispatch routes a keyboard action to the appropriate coordinator. | 392 | // Dispatch routes a keyboard action to the appropriate coordinator. | ||
M internal/ui/dispatcher/keyboard_test.go +55 -0
| @@ -329,3 +329,58 @@ func TestKeyboardDispatcher_PageModeNoopWhenActiveWebViewReturnsNil(t *testing.T | |||
| 329 | 329 | err := d.Dispatch(ctx, input.ActionPageScrollDown) | |
| 330 | 330 | require.NoError(t, err) | |
| 331 | 331 | } | |
| 332 | + | ||
| 333 | + | func TestPageScrollSpec_IsSingleSourceForCommandAndPulse(t *testing.T) { | |
| 334 | + | tests := []struct { | |
| 335 | + | action input.Action | |
| 336 | + | cmd usecase.PageScrollCommand | |
| 337 | + | fast bool | |
| 338 | + | }{ | |
| 339 | + | {input.ActionPageScrollLeft, usecase.PageScrollLeft, false}, | |
| 340 | + | {input.ActionPageScrollRight, usecase.PageScrollRight, false}, | |
| 341 | + | {input.ActionPageScrollUp, usecase.PageScrollUp, false}, | |
| 342 | + | {input.ActionPageScrollDown, usecase.PageScrollDown, false}, | |
| 343 | + | {input.ActionPageScrollUpFast, usecase.PageScrollUpFast, true}, | |
| 344 | + | {input.ActionPageScrollDownFast, usecase.PageScrollDownFast, true}, | |
| 345 | + | } | |
| 346 | + | for _, tc := range tests { | |
| 347 | + | spec, ok := pageScrollSpec(tc.action) | |
| 348 | + | require.True(t, ok, string(tc.action)) | |
| 349 | + | assert.Equal(t, tc.cmd, spec.cmd) | |
| 350 | + | assert.Equal(t, tc.fast, spec.fast) | |
| 351 | + | cmd, ok := pageScrollCommand(tc.action) | |
| 352 | + | require.True(t, ok) | |
| 353 | + | assert.Equal(t, tc.cmd, cmd) | |
| 354 | + | } | |
| 355 | + | _, ok := pageScrollSpec(input.ActionQuit) | |
| 356 | + | assert.False(t, ok) | |
| 357 | + | } | |
| 358 | + | ||
| 359 | + | func TestKeyboardDispatcher_PageScrollTapUsesSharedSpecPulse(t *testing.T) { | |
| 360 | + | ctx := context.Background() | |
| 361 | + | base := mocks.NewMockWebView(t) | |
| 362 | + | scroller := mocks.NewMockPageScrollable(t) | |
| 363 | + | wv := &mockScrollableWebView{MockWebView: base, MockPageScrollable: scroller} | |
| 364 | + | navCoord := &coordinator.NavigationCoordinator{} | |
| 365 | + | navCoord.SetPageScrollUseCase(usecase.NewPageScrollUseCase()) | |
| 366 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, navCoord, nil, nil, KeyboardActions{ | |
| 367 | + | ActiveWebView: func(context.Context) port.WebView { return wv }, | |
| 368 | + | }, func(context.Context) entity.PaneID { return "" }) | |
| 369 | + | ||
| 370 | + | var pulses []bool | |
| 371 | + | d.SetOnPageModePulse(func(_ context.Context, fast bool) { | |
| 372 | + | pulses = append(pulses, fast) | |
| 373 | + | }) | |
| 374 | + | ||
| 375 | + | base.EXPECT().ID().Return(port.WebViewID(7)).Twice() | |
| 376 | + | scroller.EXPECT().ScrollPage(ctx, port.PageScrollRequest{ | |
| 377 | + | Command: port.PageScrollCommandDown, FallbackDY: 80, | |
| 378 | + | }).Return(nil).Once() | |
| 379 | + | scroller.EXPECT().ScrollPage(ctx, port.PageScrollRequest{ | |
| 380 | + | Command: port.PageScrollCommandDownFast, FallbackDY: 320, | |
| 381 | + | }).Return(nil).Once() | |
| 382 | + | ||
| 383 | + | require.NoError(t, d.Dispatch(ctx, input.ActionPageScrollDown)) | |
| 384 | + | require.NoError(t, d.Dispatch(ctx, input.ActionPageScrollDownFast)) | |
| 385 | + | assert.Equal(t, []bool{false, true}, pulses) | |
| 386 | + | } | |
| @@ -329,3 +329,58 @@ func TestKeyboardDispatcher_PageModeNoopWhenActiveWebViewReturnsNil(t *testing.T | |||||
| 329 | err := d.Dispatch(ctx, input.ActionPageScrollDown) | 329 | err := d.Dispatch(ctx, input.ActionPageScrollDown) | ||
| 330 | require.NoError(t, err) | 330 | require.NoError(t, err) | ||
| 331 | } | 331 | } | ||
| 332 | + | ||||
| 333 | + | func TestPageScrollSpec_IsSingleSourceForCommandAndPulse(t *testing.T) { | |||
| 334 | + | tests := []struct { | |||
| 335 | + | action input.Action | |||
| 336 | + | cmd usecase.PageScrollCommand | |||
| 337 | + | fast bool | |||
| 338 | + | }{ | |||
| 339 | + | {input.ActionPageScrollLeft, usecase.PageScrollLeft, false}, | |||
| 340 | + | {input.ActionPageScrollRight, usecase.PageScrollRight, false}, | |||
| 341 | + | {input.ActionPageScrollUp, usecase.PageScrollUp, false}, | |||
| 342 | + | {input.ActionPageScrollDown, usecase.PageScrollDown, false}, | |||
| 343 | + | {input.ActionPageScrollUpFast, usecase.PageScrollUpFast, true}, | |||
| 344 | + | {input.ActionPageScrollDownFast, usecase.PageScrollDownFast, true}, | |||
| 345 | + | } | |||
| 346 | + | for _, tc := range tests { | |||
| 347 | + | spec, ok := pageScrollSpec(tc.action) | |||
| 348 | + | require.True(t, ok, string(tc.action)) | |||
| 349 | + | assert.Equal(t, tc.cmd, spec.cmd) | |||
| 350 | + | assert.Equal(t, tc.fast, spec.fast) | |||
| 351 | + | cmd, ok := pageScrollCommand(tc.action) | |||
| 352 | + | require.True(t, ok) | |||
| 353 | + | assert.Equal(t, tc.cmd, cmd) | |||
| 354 | + | } | |||
| 355 | + | _, ok := pageScrollSpec(input.ActionQuit) | |||
| 356 | + | assert.False(t, ok) | |||
| 357 | + | } | |||
| 358 | + | ||||
| 359 | + | func TestKeyboardDispatcher_PageScrollTapUsesSharedSpecPulse(t *testing.T) { | |||
| 360 | + | ctx := context.Background() | |||
| 361 | + | base := mocks.NewMockWebView(t) | |||
| 362 | + | scroller := mocks.NewMockPageScrollable(t) | |||
| 363 | + | wv := &mockScrollableWebView{MockWebView: base, MockPageScrollable: scroller} | |||
| 364 | + | navCoord := &coordinator.NavigationCoordinator{} | |||
| 365 | + | navCoord.SetPageScrollUseCase(usecase.NewPageScrollUseCase()) | |||
| 366 | + | d := NewKeyboardDispatcher(ctx, &coordinator.WorkspaceCoordinator{}, navCoord, nil, nil, KeyboardActions{ | |||
| 367 | + | ActiveWebView: func(context.Context) port.WebView { return wv }, | |||
| 368 | + | }, func(context.Context) entity.PaneID { return "" }) | |||
| 369 | + | ||||
| 370 | + | var pulses []bool | |||
| 371 | + | d.SetOnPageModePulse(func(_ context.Context, fast bool) { | |||
| 372 | + | pulses = append(pulses, fast) | |||
| 373 | + | }) | |||
| 374 | + | ||||
| 375 | + | base.EXPECT().ID().Return(port.WebViewID(7)).Twice() | |||
| 376 | + | scroller.EXPECT().ScrollPage(ctx, port.PageScrollRequest{ | |||
| 377 | + | Command: port.PageScrollCommandDown, FallbackDY: 80, | |||
| 378 | + | }).Return(nil).Once() | |||
| 379 | + | scroller.EXPECT().ScrollPage(ctx, port.PageScrollRequest{ | |||
| 380 | + | Command: port.PageScrollCommandDownFast, FallbackDY: 320, | |||
| 381 | + | }).Return(nil).Once() | |||
| 382 | + | ||||
| 383 | + | require.NoError(t, d.Dispatch(ctx, input.ActionPageScrollDown)) | |||
| 384 | + | require.NoError(t, d.Dispatch(ctx, input.ActionPageScrollDownFast)) | |||
| 385 | + | assert.Equal(t, []bool{false, true}, pulses) | |||
| 386 | + | } | |||
M internal/ui/input/keyboard.go +2 -0
| @@ -502,6 +502,8 @@ func (h *KeyboardHandler) pageScrollRepeatTick(action Action, keyval uint, gener | |||
| 502 | 502 | if now().Sub(pressedAt) < pageScrollHoldDelay { | |
| 503 | 503 | return true | |
| 504 | 504 | } | |
| 505 | + | // Continuous dispatch bypasses dispatchAction, so refresh Page Mode timeout here. | |
| 506 | + | h.modal.ResetTimeout(h.ctx) | |
| 505 | 507 | h.dispatchPageScrollLifecycle(action, PageScrollContinuous) | |
| 506 | 508 | return true | |
| 507 | 509 | } | |
| @@ -502,6 +502,8 @@ func (h *KeyboardHandler) pageScrollRepeatTick(action Action, keyval uint, gener | |||||
| 502 | if now().Sub(pressedAt) < pageScrollHoldDelay { | 502 | if now().Sub(pressedAt) < pageScrollHoldDelay { | ||
| 503 | return true | 503 | return true | ||
| 504 | } | 504 | } | ||
| 505 | + | // Continuous dispatch bypasses dispatchAction, so refresh Page Mode timeout here. | |||
| 506 | + | h.modal.ResetTimeout(h.ctx) | |||
| 505 | h.dispatchPageScrollLifecycle(action, PageScrollContinuous) | 507 | h.dispatchPageScrollLifecycle(action, PageScrollContinuous) | ||
| 506 | return true | 508 | return true | ||
| 507 | } | 509 | } | ||
M internal/ui/input/keyboard_test.go +63 -0
| @@ -505,6 +505,69 @@ func TestHandleKeyPress_PageModeTimeoutReset(t *testing.T) { | |||
| 505 | 505 | } | |
| 506 | 506 | } | |
| 507 | 507 | ||
| 508 | + | func TestPageScrollRepeatTick_RefreshesConfiguredPageModeTimeout(t *testing.T) { | |
| 509 | + | ctx := context.Background() | |
| 510 | + | workspace := newTestWorkspace() | |
| 511 | + | workspace.PageMode = entity.PageModeConfig{ | |
| 512 | + | ActivationShortcut: "ctrl+y", | |
| 513 | + | TimeoutMilliseconds: 100, | |
| 514 | + | Actions: map[string]entity.ActionBinding{ | |
| 515 | + | "page-scroll-down": {Keys: []string{"j"}}, | |
| 516 | + | }, | |
| 517 | + | } | |
| 518 | + | ||
| 519 | + | h := NewKeyboardHandler(ctx, workspace, newTestSession()) | |
| 520 | + | timerStarts := 0 | |
| 521 | + | h.modal.afterFunc = func(time.Duration, func()) modalTimer { | |
| 522 | + | timerStarts++ | |
| 523 | + | return &fakeModalTimer{} | |
| 524 | + | } | |
| 525 | + | var continuous int | |
| 526 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, _ Action, phase PageScrollPhase) error { | |
| 527 | + | if phase == PageScrollContinuous { | |
| 528 | + | continuous++ | |
| 529 | + | } | |
| 530 | + | return nil | |
| 531 | + | }) | |
| 532 | + | ||
| 533 | + | var repeatTick glib.SourceFunc | |
| 534 | + | repeatNow := time.Unix(3_000, 0) | |
| 535 | + | h.pageScrollNow = func() time.Time { return repeatNow } | |
| 536 | + | h.pageScrollRepeatAdd = func(_ uint, cb *glib.SourceFunc) uint { | |
| 537 | + | repeatTick = *cb | |
| 538 | + | return 1 | |
| 539 | + | } | |
| 540 | + | h.pageScrollRepeatRemove = func(uint) bool { return true } | |
| 541 | + | ||
| 542 | + | h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue) | |
| 543 | + | if h.Mode() != ModePage { | |
| 544 | + | t.Fatal("failed to enter page mode") | |
| 545 | + | } | |
| 546 | + | enterStarts := timerStarts | |
| 547 | + | h.handleKeyPress(uint('j'), 0, 0) | |
| 548 | + | if repeatTick == nil { | |
| 549 | + | t.Fatal("expected repeater registration") | |
| 550 | + | } | |
| 551 | + | afterTap := timerStarts | |
| 552 | + | if afterTap <= enterStarts { | |
| 553 | + | t.Fatalf("tap should refresh timeout: enter=%d afterTap=%d", enterStarts, afterTap) | |
| 554 | + | } | |
| 555 | + | ||
| 556 | + | repeatNow = repeatNow.Add(pageScrollHoldDelay) | |
| 557 | + | if !repeatTick(0) { | |
| 558 | + | t.Fatal("continuous tick stopped unexpectedly") | |
| 559 | + | } | |
| 560 | + | if continuous != 1 { | |
| 561 | + | t.Fatalf("continuous dispatches=%d, want 1", continuous) | |
| 562 | + | } | |
| 563 | + | if timerStarts <= afterTap { | |
| 564 | + | t.Fatalf("continuous tick must refresh page-mode timeout: afterTap=%d afterTick=%d", afterTap, timerStarts) | |
| 565 | + | } | |
| 566 | + | if h.Mode() != ModePage { | |
| 567 | + | t.Fatalf("mode after continuous tick = %v, want ModePage", h.Mode()) | |
| 568 | + | } | |
| 569 | + | } | |
| 570 | + | ||
| 508 | 571 | func TestHandleKeyPress_PageModeEscapeExits(t *testing.T) { | |
| 509 | 572 | ctx := context.Background() | |
| 510 | 573 | workspace := newTestWorkspace() | |
| @@ -505,6 +505,69 @@ func TestHandleKeyPress_PageModeTimeoutReset(t *testing.T) { | |||||
| 505 | } | 505 | } | ||
| 506 | } | 506 | } | ||
| 507 | 507 | ||||
| 508 | + | func TestPageScrollRepeatTick_RefreshesConfiguredPageModeTimeout(t *testing.T) { | |||
| 509 | + | ctx := context.Background() | |||
| 510 | + | workspace := newTestWorkspace() | |||
| 511 | + | workspace.PageMode = entity.PageModeConfig{ | |||
| 512 | + | ActivationShortcut: "ctrl+y", | |||
| 513 | + | TimeoutMilliseconds: 100, | |||
| 514 | + | Actions: map[string]entity.ActionBinding{ | |||
| 515 | + | "page-scroll-down": {Keys: []string{"j"}}, | |||
| 516 | + | }, | |||
| 517 | + | } | |||
| 518 | + | ||||
| 519 | + | h := NewKeyboardHandler(ctx, workspace, newTestSession()) | |||
| 520 | + | timerStarts := 0 | |||
| 521 | + | h.modal.afterFunc = func(time.Duration, func()) modalTimer { | |||
| 522 | + | timerStarts++ | |||
| 523 | + | return &fakeModalTimer{} | |||
| 524 | + | } | |||
| 525 | + | var continuous int | |||
| 526 | + | h.SetOnPageScrollLifecycle(func(_ context.Context, _ Action, phase PageScrollPhase) error { | |||
| 527 | + | if phase == PageScrollContinuous { | |||
| 528 | + | continuous++ | |||
| 529 | + | } | |||
| 530 | + | return nil | |||
| 531 | + | }) | |||
| 532 | + | ||||
| 533 | + | var repeatTick glib.SourceFunc | |||
| 534 | + | repeatNow := time.Unix(3_000, 0) | |||
| 535 | + | h.pageScrollNow = func() time.Time { return repeatNow } | |||
| 536 | + | h.pageScrollRepeatAdd = func(_ uint, cb *glib.SourceFunc) uint { | |||
| 537 | + | repeatTick = *cb | |||
| 538 | + | return 1 | |||
| 539 | + | } | |||
| 540 | + | h.pageScrollRepeatRemove = func(uint) bool { return true } | |||
| 541 | + | ||||
| 542 | + | h.handleKeyPress(uint('y'), 0, gdk.ControlMaskValue) | |||
| 543 | + | if h.Mode() != ModePage { | |||
| 544 | + | t.Fatal("failed to enter page mode") | |||
| 545 | + | } | |||
| 546 | + | enterStarts := timerStarts | |||
| 547 | + | h.handleKeyPress(uint('j'), 0, 0) | |||
| 548 | + | if repeatTick == nil { | |||
| 549 | + | t.Fatal("expected repeater registration") | |||
| 550 | + | } | |||
| 551 | + | afterTap := timerStarts | |||
| 552 | + | if afterTap <= enterStarts { | |||
| 553 | + | t.Fatalf("tap should refresh timeout: enter=%d afterTap=%d", enterStarts, afterTap) | |||
| 554 | + | } | |||
| 555 | + | ||||
| 556 | + | repeatNow = repeatNow.Add(pageScrollHoldDelay) | |||
| 557 | + | if !repeatTick(0) { | |||
| 558 | + | t.Fatal("continuous tick stopped unexpectedly") | |||
| 559 | + | } | |||
| 560 | + | if continuous != 1 { | |||
| 561 | + | t.Fatalf("continuous dispatches=%d, want 1", continuous) | |||
| 562 | + | } | |||
| 563 | + | if timerStarts <= afterTap { | |||
| 564 | + | t.Fatalf("continuous tick must refresh page-mode timeout: afterTap=%d afterTick=%d", afterTap, timerStarts) | |||
| 565 | + | } | |||
| 566 | + | if h.Mode() != ModePage { | |||
| 567 | + | t.Fatalf("mode after continuous tick = %v, want ModePage", h.Mode()) | |||
| 568 | + | } | |||
| 569 | + | } | |||
| 570 | + | ||||
| 508 | func TestHandleKeyPress_PageModeEscapeExits(t *testing.T) { | 571 | func TestHandleKeyPress_PageModeEscapeExits(t *testing.T) { | ||
| 509 | ctx := context.Background() | 572 | ctx := context.Background() | ||
| 510 | workspace := newTestWorkspace() | 573 | workspace := newTestWorkspace() | ||