fix(zoom): prevent zoom shift during navigation via LoadCommitted event
- commit
- 5af29c5b6e26be7aa8544414ba01666cd014eac4
- author
- brice · · 2025-12-16 18:15 UTC
- committer
- brice · · 2025-12-16 20:09 UTC
- parent
- 5c92d2f
- signature
- unsigned
3 files · +36 -27
M internal/application/usecase/navigate.go +4 -27
| @@ -47,7 +47,8 @@ type NavigateOutput struct { | |||
| 47 | 47 | AppliedZoom float64 | |
| 48 | 48 | } | |
| 49 | 49 | ||
| 50 | - | // Execute navigates to a URL, applies zoom, and records history. | |
| 50 | + | // Execute navigates to a URL and records history. | |
| 51 | + | // Zoom is applied later via LoadCommitted event in ContentCoordinator. | |
| 51 | 52 | func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*NavigateOutput, error) { | |
| 52 | 53 | log := logging.FromContext(ctx) | |
| 53 | 54 | log.Debug(). | |
| @@ -55,30 +56,7 @@ func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*N | |||
| 55 | 56 | Str("pane_id", input.PaneID). | |
| 56 | 57 | Msg("navigating to URL") | |
| 57 | 58 | ||
| 58 | - | // Extract domain for zoom lookup | |
| 59 | - | domain, err := ExtractDomain(input.URL) | |
| 60 | - | if err != nil { | |
| 61 | - | log.Debug().Str("url", input.URL).Msg("could not extract domain, using default zoom") | |
| 62 | - | domain = "" | |
| 63 | - | } | |
| 64 | - | ||
| 65 | - | // Load and apply zoom level before navigation (for DOM zoom mode) | |
| 66 | - | zoomFactor := uc.defaultZoom | |
| 67 | - | if domain != "" { | |
| 68 | - | zoom, err := uc.zoomRepo.Get(ctx, domain) | |
| 69 | - | if err != nil { | |
| 70 | - | log.Warn().Err(err).Str("domain", domain).Msg("failed to get zoom level") | |
| 71 | - | } else if zoom != nil { | |
| 72 | - | zoomFactor = zoom.ZoomFactor | |
| 73 | - | } | |
| 74 | - | } | |
| 75 | - | ||
| 76 | - | // Apply zoom before loading (important for DOM zoom) | |
| 77 | - | if err := input.WebView.SetZoomLevel(ctx, zoomFactor); err != nil { | |
| 78 | - | log.Warn().Err(err).Float64("zoom", zoomFactor).Msg("failed to set zoom level") | |
| 79 | - | } | |
| 80 | - | ||
| 81 | - | // Navigate to URL | |
| 59 | + | // Navigate to URL (zoom will be applied on LoadCommitted to avoid shift) | |
| 82 | 60 | if err := input.WebView.LoadURI(ctx, input.URL); err != nil { | |
| 83 | 61 | return nil, fmt.Errorf("failed to load URL: %w", err) | |
| 84 | 62 | } | |
| @@ -88,11 +66,10 @@ func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*N | |||
| 88 | 66 | ||
| 89 | 67 | log.Info(). | |
| 90 | 68 | Str("url", input.URL). | |
| 91 | - | Float64("zoom", zoomFactor). | |
| 92 | 69 | Msg("navigation initiated") | |
| 93 | 70 | ||
| 94 | 71 | return &NavigateOutput{ | |
| 95 | - | AppliedZoom: zoomFactor, | |
| 72 | + | AppliedZoom: uc.defaultZoom, | |
| 96 | 73 | }, nil | |
| 97 | 74 | } | |
| 98 | 75 | ||
| @@ -47,7 +47,8 @@ type NavigateOutput struct { | |||||
| 47 | AppliedZoom float64 | 47 | AppliedZoom float64 | ||
| 48 | } | 48 | } | ||
| 49 | 49 | ||||
| 50 | - | // Execute navigates to a URL, applies zoom, and records history. | 50 | + | // Execute navigates to a URL and records history. |
| 51 | + | // Zoom is applied later via LoadCommitted event in ContentCoordinator. | |||
| 51 | func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*NavigateOutput, error) { | 52 | func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*NavigateOutput, error) { | ||
| 52 | log := logging.FromContext(ctx) | 53 | log := logging.FromContext(ctx) | ||
| 53 | log.Debug(). | 54 | log.Debug(). | ||
| @@ -55,30 +56,7 @@ func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*N | |||||
| 55 | Str("pane_id", input.PaneID). | 56 | Str("pane_id", input.PaneID). | ||
| 56 | Msg("navigating to URL") | 57 | Msg("navigating to URL") | ||
| 57 | 58 | ||||
| 58 | - | // Extract domain for zoom lookup | 59 | + | // Navigate to URL (zoom will be applied on LoadCommitted to avoid shift) |
| 59 | - | domain, err := ExtractDomain(input.URL) | |||
| 60 | - | if err != nil { | |||
| 61 | - | log.Debug().Str("url", input.URL).Msg("could not extract domain, using default zoom") | |||
| 62 | - | domain = "" | |||
| 63 | - | } | |||
| 64 | - | ||||
| 65 | - | // Load and apply zoom level before navigation (for DOM zoom mode) | |||
| 66 | - | zoomFactor := uc.defaultZoom | |||
| 67 | - | if domain != "" { | |||
| 68 | - | zoom, err := uc.zoomRepo.Get(ctx, domain) | |||
| 69 | - | if err != nil { | |||
| 70 | - | log.Warn().Err(err).Str("domain", domain).Msg("failed to get zoom level") | |||
| 71 | - | } else if zoom != nil { | |||
| 72 | - | zoomFactor = zoom.ZoomFactor | |||
| 73 | - | } | |||
| 74 | - | } | |||
| 75 | - | ||||
| 76 | - | // Apply zoom before loading (important for DOM zoom) | |||
| 77 | - | if err := input.WebView.SetZoomLevel(ctx, zoomFactor); err != nil { | |||
| 78 | - | log.Warn().Err(err).Float64("zoom", zoomFactor).Msg("failed to set zoom level") | |||
| 79 | - | } | |||
| 80 | - | ||||
| 81 | - | // Navigate to URL | |||
| 82 | if err := input.WebView.LoadURI(ctx, input.URL); err != nil { | 60 | if err := input.WebView.LoadURI(ctx, input.URL); err != nil { | ||
| 83 | return nil, fmt.Errorf("failed to load URL: %w", err) | 61 | return nil, fmt.Errorf("failed to load URL: %w", err) | ||
| 84 | } | 62 | } | ||
| @@ -88,11 +66,10 @@ func (uc *NavigateUseCase) Execute(ctx context.Context, input NavigateInput) (*N | |||||
| 88 | 66 | ||||
| 89 | log.Info(). | 67 | log.Info(). | ||
| 90 | Str("url", input.URL). | 68 | Str("url", input.URL). | ||
| 91 | - | Float64("zoom", zoomFactor). | |||
| 92 | Msg("navigation initiated") | 69 | Msg("navigation initiated") | ||
| 93 | 70 | ||||
| 94 | return &NavigateOutput{ | 71 | return &NavigateOutput{ | ||
| 95 | - | AppliedZoom: zoomFactor, | 72 | + | AppliedZoom: uc.defaultZoom, |
| 96 | }, nil | 73 | }, nil | ||
| 97 | } | 74 | } | ||
| 98 | 75 | ||||
M internal/ui/app.go +1 -0
| @@ -269,6 +269,7 @@ func (a *App) initCoordinators(ctx context.Context) { | |||
| 269 | 269 | a.widgetFactory, | |
| 270 | 270 | a.faviconCache, | |
| 271 | 271 | getActiveWS, | |
| 272 | + | a.deps.ZoomUC, | |
| 272 | 273 | ) | |
| 273 | 274 | ||
| 274 | 275 | // 2. Tab Coordinator | |
| @@ -269,6 +269,7 @@ func (a *App) initCoordinators(ctx context.Context) { | |||||
| 269 | a.widgetFactory, | 269 | a.widgetFactory, | ||
| 270 | a.faviconCache, | 270 | a.faviconCache, | ||
| 271 | getActiveWS, | 271 | getActiveWS, | ||
| 272 | + | a.deps.ZoomUC, | |||
| 272 | ) | 273 | ) | ||
| 273 | 274 | ||||
| 274 | // 2. Tab Coordinator | 275 | // 2. Tab Coordinator | ||
M internal/ui/coordinator/content.go +31 -0
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "fmt" | |
| 6 | 6 | "sync" | |
| 7 | 7 | ||
| 8 | + | "github.com/bnema/dumber/internal/application/usecase" | |
| 8 | 9 | "github.com/bnema/dumber/internal/domain/entity" | |
| 9 | 10 | "github.com/bnema/dumber/internal/infrastructure/webkit" | |
| 10 | 11 | "github.com/bnema/dumber/internal/logging" | |
| @@ -19,6 +20,7 @@ type ContentCoordinator struct { | |||
| 19 | 20 | pool *webkit.WebViewPool | |
| 20 | 21 | widgetFactory layout.WidgetFactory | |
| 21 | 22 | faviconCache *cache.FaviconCache | |
| 23 | + | zoomUC *usecase.ManageZoomUseCase | |
| 22 | 24 | ||
| 23 | 25 | webViews map[entity.PaneID]*webkit.WebView | |
| 24 | 26 | paneTitles map[entity.PaneID]string | |
| @@ -43,6 +45,7 @@ func NewContentCoordinator( | |||
| 43 | 45 | widgetFactory layout.WidgetFactory, | |
| 44 | 46 | faviconCache *cache.FaviconCache, | |
| 45 | 47 | getActiveWS func() (*entity.Workspace, *component.WorkspaceView), | |
| 48 | + | zoomUC *usecase.ManageZoomUseCase, | |
| 46 | 49 | ) *ContentCoordinator { | |
| 47 | 50 | log := logging.FromContext(ctx) | |
| 48 | 51 | log.Debug().Msg("creating content coordinator") | |
| @@ -51,6 +54,7 @@ func NewContentCoordinator( | |||
| 51 | 54 | pool: pool, | |
| 52 | 55 | widgetFactory: widgetFactory, | |
| 53 | 56 | faviconCache: faviconCache, | |
| 57 | + | zoomUC: zoomUC, | |
| 54 | 58 | webViews: make(map[entity.PaneID]*webkit.WebView), | |
| 55 | 59 | paneTitles: make(map[entity.PaneID]string), | |
| 56 | 60 | navOrigins: make(map[entity.PaneID]string), | |
| @@ -92,6 +96,13 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||
| 92 | 96 | c.onFaviconChanged(ctx, paneID, favicon) | |
| 93 | 97 | } | |
| 94 | 98 | ||
| 99 | + | // Set up load change callback to re-apply zoom on navigation | |
| 100 | + | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |
| 101 | + | if event == webkit.LoadCommitted { | |
| 102 | + | c.onLoadCommitted(ctx, paneID, wv) | |
| 103 | + | } | |
| 104 | + | } | |
| 105 | + | ||
| 95 | 106 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | |
| 96 | 107 | return wv, nil | |
| 97 | 108 | } | |
| @@ -391,3 +402,23 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||
| 391 | 402 | } | |
| 392 | 403 | } | |
| 393 | 404 | } | |
| 405 | + | ||
| 406 | + | // onLoadCommitted re-applies zoom when page content starts loading. | |
| 407 | + | // WebKit may reset zoom during document transitions, so we reapply after LoadCommitted. | |
| 408 | + | func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |
| 409 | + | if c.zoomUC == nil { | |
| 410 | + | return | |
| 411 | + | } | |
| 412 | + | ||
| 413 | + | url := wv.URI() | |
| 414 | + | if url == "" { | |
| 415 | + | return | |
| 416 | + | } | |
| 417 | + | ||
| 418 | + | domain, err := usecase.ExtractDomain(url) | |
| 419 | + | if err != nil { | |
| 420 | + | return | |
| 421 | + | } | |
| 422 | + | ||
| 423 | + | _ = c.zoomUC.ApplyToWebView(ctx, wv, domain) | |
| 424 | + | } | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "sync" | 6 | "sync" | ||
| 7 | 7 | ||||
| 8 | + | "github.com/bnema/dumber/internal/application/usecase" | |||
| 8 | "github.com/bnema/dumber/internal/domain/entity" | 9 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 9 | "github.com/bnema/dumber/internal/infrastructure/webkit" | 10 | "github.com/bnema/dumber/internal/infrastructure/webkit" | ||
| 10 | "github.com/bnema/dumber/internal/logging" | 11 | "github.com/bnema/dumber/internal/logging" | ||
| @@ -19,6 +20,7 @@ type ContentCoordinator struct { | |||||
| 19 | pool *webkit.WebViewPool | 20 | pool *webkit.WebViewPool | ||
| 20 | widgetFactory layout.WidgetFactory | 21 | widgetFactory layout.WidgetFactory | ||
| 21 | faviconCache *cache.FaviconCache | 22 | faviconCache *cache.FaviconCache | ||
| 23 | + | zoomUC *usecase.ManageZoomUseCase | |||
| 22 | 24 | ||||
| 23 | webViews map[entity.PaneID]*webkit.WebView | 25 | webViews map[entity.PaneID]*webkit.WebView | ||
| 24 | paneTitles map[entity.PaneID]string | 26 | paneTitles map[entity.PaneID]string | ||
| @@ -43,6 +45,7 @@ func NewContentCoordinator( | |||||
| 43 | widgetFactory layout.WidgetFactory, | 45 | widgetFactory layout.WidgetFactory, | ||
| 44 | faviconCache *cache.FaviconCache, | 46 | faviconCache *cache.FaviconCache, | ||
| 45 | getActiveWS func() (*entity.Workspace, *component.WorkspaceView), | 47 | getActiveWS func() (*entity.Workspace, *component.WorkspaceView), | ||
| 48 | + | zoomUC *usecase.ManageZoomUseCase, | |||
| 46 | ) *ContentCoordinator { | 49 | ) *ContentCoordinator { | ||
| 47 | log := logging.FromContext(ctx) | 50 | log := logging.FromContext(ctx) | ||
| 48 | log.Debug().Msg("creating content coordinator") | 51 | log.Debug().Msg("creating content coordinator") | ||
| @@ -51,6 +54,7 @@ func NewContentCoordinator( | |||||
| 51 | pool: pool, | 54 | pool: pool, | ||
| 52 | widgetFactory: widgetFactory, | 55 | widgetFactory: widgetFactory, | ||
| 53 | faviconCache: faviconCache, | 56 | faviconCache: faviconCache, | ||
| 57 | + | zoomUC: zoomUC, | |||
| 54 | webViews: make(map[entity.PaneID]*webkit.WebView), | 58 | webViews: make(map[entity.PaneID]*webkit.WebView), | ||
| 55 | paneTitles: make(map[entity.PaneID]string), | 59 | paneTitles: make(map[entity.PaneID]string), | ||
| 56 | navOrigins: make(map[entity.PaneID]string), | 60 | navOrigins: make(map[entity.PaneID]string), | ||
| @@ -92,6 +96,13 @@ func (c *ContentCoordinator) EnsureWebView(ctx context.Context, paneID entity.Pa | |||||
| 92 | c.onFaviconChanged(ctx, paneID, favicon) | 96 | c.onFaviconChanged(ctx, paneID, favicon) | ||
| 93 | } | 97 | } | ||
| 94 | 98 | ||||
| 99 | + | // Set up load change callback to re-apply zoom on navigation | |||
| 100 | + | wv.OnLoadChanged = func(event webkit.LoadEvent) { | |||
| 101 | + | if event == webkit.LoadCommitted { | |||
| 102 | + | c.onLoadCommitted(ctx, paneID, wv) | |||
| 103 | + | } | |||
| 104 | + | } | |||
| 105 | + | ||||
| 95 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | 106 | log.Debug().Str("pane_id", string(paneID)).Msg("webview acquired for pane") | ||
| 96 | return wv, nil | 107 | return wv, nil | ||
| 97 | } | 108 | } | ||
| @@ -391,3 +402,23 @@ func (c *ContentCoordinator) PreloadCachedFavicon(ctx context.Context, paneID en | |||||
| 391 | } | 402 | } | ||
| 392 | } | 403 | } | ||
| 393 | } | 404 | } | ||
| 405 | + | ||||
| 406 | + | // onLoadCommitted re-applies zoom when page content starts loading. | |||
| 407 | + | // WebKit may reset zoom during document transitions, so we reapply after LoadCommitted. | |||
| 408 | + | func (c *ContentCoordinator) onLoadCommitted(ctx context.Context, paneID entity.PaneID, wv *webkit.WebView) { | |||
| 409 | + | if c.zoomUC == nil { | |||
| 410 | + | return | |||
| 411 | + | } | |||
| 412 | + | ||||
| 413 | + | url := wv.URI() | |||
| 414 | + | if url == "" { | |||
| 415 | + | return | |||
| 416 | + | } | |||
| 417 | + | ||||
| 418 | + | domain, err := usecase.ExtractDomain(url) | |||
| 419 | + | if err != nil { | |||
| 420 | + | return | |||
| 421 | + | } | |||
| 422 | + | ||||
| 423 | + | _ = c.zoomUC.ApplyToWebView(ctx, wv, domain) | |||
| 424 | + | } | |||