fix(runtime): harden navigation and browser lifecycle
- commit
- 55df320207c8d69b9efece1d2728e6ac1cc343b9
- author
- brice · · 2026-08-16 00:40 UTC
- parent
- 6744d8a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
41 files · +454 -275
M README.md +3 -3
| @@ -43,7 +43,7 @@ The browser chrome stays out of the way by default. Open the omnibox when you wa | |||
| 43 | 43 | ||
| 44 | 44 | ### Keyboard workflow | |
| 45 | 45 | ||
| 46 | - | - Pane, tab, page, resize, and session modes | |
| 46 | + | - Pane, tab, Vim, resize, and session modes | |
| 47 | 47 | - Vim/Zellij-style navigation patterns | |
| 48 | 48 | - Search bangs such as `!g`, `!gi`, and `!ddg` | |
| 49 | 49 | - Launcher integration for `rofi`, `fuzzel`, and `dmenu` | |
| @@ -67,7 +67,7 @@ The browser chrome stays out of the way by default. Open the omnibox when you wa | |||
| 67 | 67 | ### Install script | |
| 68 | 68 | ||
| 69 | 69 | ```bash | |
| 70 | - | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | sh | |
| 70 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/v0.31.0/install.sh | DUMBER_VERSION=v0.31.0 bash | |
| 71 | 71 | dumber browse | |
| 72 | 72 | ``` | |
| 73 | 73 | ||
| @@ -96,7 +96,7 @@ Dumber uses modal keybindings for browser management. | |||
| 96 | 96 | |------|-------------|----------| | |
| 97 | 97 | | Pane | `Ctrl+P` | Split, stack, close, and move panes | | |
| 98 | 98 | | Tab | `Ctrl+T` | Create, close, switch, and rename tabs | | |
| 99 | - | | Page | `Ctrl+Y` | Scroll the active webpage with `h/j/k/l` and `Shift+J/K`; arrow keys stay native and other app shortcuts wait until exit | | |
| 99 | + | | Vim Mode | `Ctrl+Y` | Navigate the active webpage with configurable Vim-style sequences; arrow keys stay native and other app shortcuts wait until exit | | |
| 100 | 100 | | Resize | `Ctrl+N` | Resize panes with `hjkl` or arrow keys | | |
| 101 | 101 | | Session | `Ctrl+O` | Snapshot, restore, and browse sessions | | |
| 102 | 102 | ||
| @@ -43,7 +43,7 @@ The browser chrome stays out of the way by default. Open the omnibox when you wa | |||||
| 43 | 43 | ||||
| 44 | ### Keyboard workflow | 44 | ### Keyboard workflow | ||
| 45 | 45 | ||||
| 46 | - | - Pane, tab, page, resize, and session modes | 46 | + | - Pane, tab, Vim, resize, and session modes |
| 47 | - Vim/Zellij-style navigation patterns | 47 | - Vim/Zellij-style navigation patterns | ||
| 48 | - Search bangs such as `!g`, `!gi`, and `!ddg` | 48 | - Search bangs such as `!g`, `!gi`, and `!ddg` | ||
| 49 | - Launcher integration for `rofi`, `fuzzel`, and `dmenu` | 49 | - Launcher integration for `rofi`, `fuzzel`, and `dmenu` | ||
| @@ -67,7 +67,7 @@ The browser chrome stays out of the way by default. Open the omnibox when you wa | |||||
| 67 | ### Install script | 67 | ### Install script | ||
| 68 | 68 | ||||
| 69 | ```bash | 69 | ```bash | ||
| 70 | - | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | sh | 70 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/v0.31.0/install.sh | DUMBER_VERSION=v0.31.0 bash |
| 71 | dumber browse | 71 | dumber browse | ||
| 72 | ``` | 72 | ``` | ||
| 73 | 73 | ||||
| @@ -96,7 +96,7 @@ Dumber uses modal keybindings for browser management. | |||||
| 96 | |------|-------------|----------| | 96 | |------|-------------|----------| | ||
| 97 | | Pane | `Ctrl+P` | Split, stack, close, and move panes | | 97 | | Pane | `Ctrl+P` | Split, stack, close, and move panes | | ||
| 98 | | Tab | `Ctrl+T` | Create, close, switch, and rename tabs | | 98 | | Tab | `Ctrl+T` | Create, close, switch, and rename tabs | | ||
| 99 | - | | Page | `Ctrl+Y` | Scroll the active webpage with `h/j/k/l` and `Shift+J/K`; arrow keys stay native and other app shortcuts wait until exit | | 99 | + | | Vim Mode | `Ctrl+Y` | Navigate the active webpage with configurable Vim-style sequences; arrow keys stay native and other app shortcuts wait until exit | |
| 100 | | Resize | `Ctrl+N` | Resize panes with `hjkl` or arrow keys | | 100 | | Resize | `Ctrl+N` | Resize panes with `hjkl` or arrow keys | | ||
| 101 | | Session | `Ctrl+O` | Snapshot, restore, and browse sessions | | 101 | | Session | `Ctrl+O` | Snapshot, restore, and browse sessions | | ||
| 102 | 102 | ||||
M cmd/dumber/main.go +4 -4
| @@ -118,13 +118,13 @@ func tryForwardBrowserURLToRunningInstance( | |||
| 118 | 118 | } | |
| 119 | 119 | ||
| 120 | 120 | delivered, err := deliver(ctx, browseURL) | |
| 121 | + | if delivered { | |
| 122 | + | return true, nil | |
| 123 | + | } | |
| 121 | 124 | if err != nil { | |
| 122 | - | if delivered && errors.Is(err, desktop.ErrBrowserLaunchRelayUnconfirmed) { | |
| 123 | - | return true, nil | |
| 124 | - | } | |
| 125 | 125 | return false, err | |
| 126 | 126 | } | |
| 127 | - | return delivered, nil | |
| 127 | + | return false, nil | |
| 128 | 128 | } | |
| 129 | 129 | ||
| 130 | 130 | func launchStandaloneBrowserURL(ctx context.Context, launch func(context.Context, string) error, uri string) error { | |
| @@ -118,13 +118,13 @@ func tryForwardBrowserURLToRunningInstance( | |||||
| 118 | } | 118 | } | ||
| 119 | 119 | ||||
| 120 | delivered, err := deliver(ctx, browseURL) | 120 | delivered, err := deliver(ctx, browseURL) | ||
| 121 | + | if delivered { | |||
| 122 | + | return true, nil | |||
| 123 | + | } | |||
| 121 | if err != nil { | 124 | if err != nil { | ||
| 122 | - | if delivered && errors.Is(err, desktop.ErrBrowserLaunchRelayUnconfirmed) { | |||
| 123 | - | return true, nil | |||
| 124 | - | } | |||
| 125 | return false, err | 125 | return false, err | ||
| 126 | } | 126 | } | ||
| 127 | - | return delivered, nil | 127 | + | return false, nil |
| 128 | } | 128 | } | ||
| 129 | 129 | ||||
| 130 | func launchStandaloneBrowserURL(ctx context.Context, launch func(context.Context, string) error, uri string) error { | 130 | func launchStandaloneBrowserURL(ctx context.Context, launch func(context.Context, string) error, uri string) error { | ||
M docs/installation.md +3 -3
| @@ -13,19 +13,19 @@ Installs to `~/.local/bin` if available, otherwise `/usr/local/bin` (requires su | |||
| 13 | 13 | Install a specific version: | |
| 14 | 14 | ||
| 15 | 15 | ```bash | |
| 16 | - | DUMBER_VERSION=v0.26.2 curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | bash | |
| 16 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | DUMBER_VERSION=v0.26.2 bash | |
| 17 | 17 | ``` | |
| 18 | 18 | ||
| 19 | 19 | Install the latest prerelease: | |
| 20 | 20 | ||
| 21 | 21 | ```bash | |
| 22 | - | DUMBER_PRERELEASE=1 curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | bash | |
| 22 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | DUMBER_PRERELEASE=1 bash | |
| 23 | 23 | ``` | |
| 24 | 24 | ||
| 25 | 25 | Combine both to install a specific prerelease: | |
| 26 | 26 | ||
| 27 | 27 | ```bash | |
| 28 | - | DUMBER_VERSION=v0.27.0-rc.1 DUMBER_PRERELEASE=1 curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | bash | |
| 28 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | DUMBER_VERSION=v0.27.0-rc.1 DUMBER_PRERELEASE=1 bash | |
| 29 | 29 | ``` | |
| 30 | 30 | ||
| 31 | 31 | ## Flatpak | |
| @@ -13,19 +13,19 @@ Installs to `~/.local/bin` if available, otherwise `/usr/local/bin` (requires su | |||||
| 13 | Install a specific version: | 13 | Install a specific version: | ||
| 14 | 14 | ||||
| 15 | ```bash | 15 | ```bash | ||
| 16 | - | DUMBER_VERSION=v0.26.2 curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | bash | 16 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | DUMBER_VERSION=v0.26.2 bash |
| 17 | ``` | 17 | ``` | ||
| 18 | 18 | ||||
| 19 | Install the latest prerelease: | 19 | Install the latest prerelease: | ||
| 20 | 20 | ||||
| 21 | ```bash | 21 | ```bash | ||
| 22 | - | DUMBER_PRERELEASE=1 curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | bash | 22 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | DUMBER_PRERELEASE=1 bash |
| 23 | ``` | 23 | ``` | ||
| 24 | 24 | ||||
| 25 | Combine both to install a specific prerelease: | 25 | Combine both to install a specific prerelease: | ||
| 26 | 26 | ||||
| 27 | ```bash | 27 | ```bash | ||
| 28 | - | DUMBER_VERSION=v0.27.0-rc.1 DUMBER_PRERELEASE=1 curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | bash | 28 | + | curl -fsSL https://raw.githubusercontent.com/bnema/dumber/main/install.sh | DUMBER_VERSION=v0.27.0-rc.1 DUMBER_PRERELEASE=1 bash |
| 29 | ``` | 29 | ``` | ||
| 30 | 30 | ||||
| 31 | ## Flatpak | 31 | ## Flatpak | ||
M internal/application/port/favorites_sidebar.go +1 -0
| @@ -18,4 +18,5 @@ type FavoritesSidebarFavorites interface { | |||
| 18 | 18 | SetShortcut(ctx context.Context, id entity.FavoriteID, key *int) error | |
| 19 | 19 | TagFavorite(ctx context.Context, favID entity.FavoriteID, tagID entity.TagID) error | |
| 20 | 20 | UntagFavorite(ctx context.Context, favID entity.FavoriteID, tagID entity.TagID) error | |
| 21 | + | UpdateFavoriteTags(ctx context.Context, favID entity.FavoriteID, tagIDs []entity.TagID) error | |
| 21 | 22 | } | |
| @@ -18,4 +18,5 @@ type FavoritesSidebarFavorites interface { | |||||
| 18 | SetShortcut(ctx context.Context, id entity.FavoriteID, key *int) error | 18 | SetShortcut(ctx context.Context, id entity.FavoriteID, key *int) error | ||
| 19 | TagFavorite(ctx context.Context, favID entity.FavoriteID, tagID entity.TagID) error | 19 | TagFavorite(ctx context.Context, favID entity.FavoriteID, tagID entity.TagID) error | ||
| 20 | UntagFavorite(ctx context.Context, favID entity.FavoriteID, tagID entity.TagID) error | 20 | UntagFavorite(ctx context.Context, favID entity.FavoriteID, tagID entity.TagID) error | ||
| 21 | + | UpdateFavoriteTags(ctx context.Context, favID entity.FavoriteID, tagIDs []entity.TagID) error | |||
| 21 | } | 22 | } | ||
M internal/application/port/webview.go +14 -0
| @@ -5,6 +5,7 @@ package port | |||
| 5 | 5 | ||
| 6 | 6 | import ( | |
| 7 | 7 | "context" | |
| 8 | + | "fmt" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/dumber/internal/application/dto" | |
| 10 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| @@ -420,6 +421,19 @@ type PageScrollRequest struct { | |||
| 420 | 421 | Continuous bool | |
| 421 | 422 | } | |
| 422 | 423 | ||
| 424 | + | // IsValid reports whether c is a supported semantic page-scroll command. | |
| 425 | + | func (c PageScrollCommand) IsValid() bool { | |
| 426 | + | return c >= PageScrollCommandLeft && c <= PageScrollCommandDownFast | |
| 427 | + | } | |
| 428 | + | ||
| 429 | + | // Validate checks the command identity carried by a page-scroll request. | |
| 430 | + | func (r PageScrollRequest) Validate() error { | |
| 431 | + | if !r.Command.IsValid() { | |
| 432 | + | return fmt.Errorf("unsupported page scroll command %d", r.Command) | |
| 433 | + | } | |
| 434 | + | return nil | |
| 435 | + | } | |
| 436 | + | ||
| 423 | 437 | // PageScrollable is an optional capability for WebViews that support semantic | |
| 424 | 438 | // page scrolling. Page scrolling is used by keyboard-driven navigation modes | |
| 425 | 439 | // such as Vim mode. | |
| @@ -5,6 +5,7 @@ package port | |||||
| 5 | 5 | ||||
| 6 | import ( | 6 | import ( | ||
| 7 | "context" | 7 | "context" | ||
| 8 | + | "fmt" | |||
| 8 | 9 | ||||
| 9 | "github.com/bnema/dumber/internal/application/dto" | 10 | "github.com/bnema/dumber/internal/application/dto" | ||
| 10 | "github.com/bnema/dumber/internal/domain/entity" | 11 | "github.com/bnema/dumber/internal/domain/entity" | ||
| @@ -420,6 +421,19 @@ type PageScrollRequest struct { | |||||
| 420 | Continuous bool | 421 | Continuous bool | ||
| 421 | } | 422 | } | ||
| 422 | 423 | ||||
| 424 | + | // IsValid reports whether c is a supported semantic page-scroll command. | |||
| 425 | + | func (c PageScrollCommand) IsValid() bool { | |||
| 426 | + | return c >= PageScrollCommandLeft && c <= PageScrollCommandDownFast | |||
| 427 | + | } | |||
| 428 | + | ||||
| 429 | + | // Validate checks the command identity carried by a page-scroll request. | |||
| 430 | + | func (r PageScrollRequest) Validate() error { | |||
| 431 | + | if !r.Command.IsValid() { | |||
| 432 | + | return fmt.Errorf("unsupported page scroll command %d", r.Command) | |||
| 433 | + | } | |||
| 434 | + | return nil | |||
| 435 | + | } | |||
| 436 | + | ||||
| 423 | // PageScrollable is an optional capability for WebViews that support semantic | 437 | // PageScrollable is an optional capability for WebViews that support semantic | ||
| 424 | // page scrolling. Page scrolling is used by keyboard-driven navigation modes | 438 | // page scrolling. Page scrolling is used by keyboard-driven navigation modes | ||
| 425 | // such as Vim mode. | 439 | // such as Vim mode. | ||
M internal/application/usecase/manage_favorites.go +77 -0
| @@ -599,6 +599,83 @@ func (uc *ManageFavoritesUseCase) UntagFavorite(ctx context.Context, favID entit | |||
| 599 | 599 | return nil | |
| 600 | 600 | } | |
| 601 | 601 | ||
| 602 | + | // UpdateFavoriteTags reconciles a favorite's tag assignments with the requested state. | |
| 603 | + | // Failed mutations are rolled back in reverse order when the repository cannot | |
| 604 | + | // provide a transaction boundary. | |
| 605 | + | func (uc *ManageFavoritesUseCase) UpdateFavoriteTags(ctx context.Context, favID entity.FavoriteID, tagIDs []entity.TagID) error { | |
| 606 | + | currentTags, err := uc.tagRepo.GetForFavorite(ctx, favID) | |
| 607 | + | if err != nil { | |
| 608 | + | return fmt.Errorf("failed to load tags for favorite %d: %w", favID, err) | |
| 609 | + | } | |
| 610 | + | ||
| 611 | + | current := make(map[entity.TagID]struct{}, len(currentTags)) | |
| 612 | + | for _, tag := range currentTags { | |
| 613 | + | if tag != nil { | |
| 614 | + | current[tag.ID] = struct{}{} | |
| 615 | + | } | |
| 616 | + | } | |
| 617 | + | wanted := make(map[entity.TagID]struct{}, len(tagIDs)) | |
| 618 | + | for _, id := range tagIDs { | |
| 619 | + | wanted[id] = struct{}{} | |
| 620 | + | } | |
| 621 | + | ||
| 622 | + | toRemove := make([]entity.TagID, 0, len(current)) | |
| 623 | + | for id := range current { | |
| 624 | + | if _, ok := wanted[id]; !ok { | |
| 625 | + | toRemove = append(toRemove, id) | |
| 626 | + | } | |
| 627 | + | } | |
| 628 | + | toAdd := make([]entity.TagID, 0, len(wanted)) | |
| 629 | + | for id := range wanted { | |
| 630 | + | if _, ok := current[id]; !ok { | |
| 631 | + | toAdd = append(toAdd, id) | |
| 632 | + | } | |
| 633 | + | } | |
| 634 | + | sort.Slice(toRemove, func(i, j int) bool { return toRemove[i] < toRemove[j] }) | |
| 635 | + | sort.Slice(toAdd, func(i, j int) bool { return toAdd[i] < toAdd[j] }) | |
| 636 | + | ||
| 637 | + | type mutation struct { | |
| 638 | + | tagID entity.TagID | |
| 639 | + | added bool | |
| 640 | + | } | |
| 641 | + | applied := make([]mutation, 0, len(toRemove)+len(toAdd)) | |
| 642 | + | rollbackLog := logging.FromContext(ctx) | |
| 643 | + | rollback := func() { | |
| 644 | + | for i := len(applied) - 1; i >= 0; i-- { | |
| 645 | + | change := applied[i] | |
| 646 | + | var rollbackErr error | |
| 647 | + | if change.added { | |
| 648 | + | rollbackErr = uc.UntagFavorite(ctx, favID, change.tagID) | |
| 649 | + | } else { | |
| 650 | + | rollbackErr = uc.TagFavorite(ctx, favID, change.tagID) | |
| 651 | + | } | |
| 652 | + | if rollbackErr != nil { | |
| 653 | + | rollbackLog.Warn(). | |
| 654 | + | Err(rollbackErr). | |
| 655 | + | Int64("favorite_id", int64(favID)). | |
| 656 | + | Int64("tag_id", int64(change.tagID)). | |
| 657 | + | Msg("failed to roll back favorite tag mutation") | |
| 658 | + | } | |
| 659 | + | } | |
| 660 | + | } | |
| 661 | + | ||
| 662 | + | for _, id := range toRemove { | |
| 663 | + | if err := uc.UntagFavorite(ctx, favID, id); err != nil { | |
| 664 | + | rollback() | |
| 665 | + | return fmt.Errorf("failed to remove tag %d from favorite %d: %w", id, favID, err) | |
| 666 | + | } | |
| 667 | + | applied = append(applied, mutation{tagID: id}) | |
| 668 | + | } | |
| 669 | + | for _, id := range toAdd { | |
| 670 | + | if err := uc.TagFavorite(ctx, favID, id); err != nil { | |
| 671 | + | rollback() | |
| 672 | + | return fmt.Errorf("failed to add tag %d to favorite %d: %w", id, favID, err) | |
| 673 | + | } | |
| 674 | + | applied = append(applied, mutation{tagID: id, added: true}) | |
| 675 | + | } | |
| 676 | + | return nil | |
| 677 | + | } | |
| 678 | + | ||
| 602 | 679 | // GetTagsForFavorite retrieves all tags for a favorite. | |
| 603 | 680 | func (uc *ManageFavoritesUseCase) GetTagsForFavorite(ctx context.Context, favID entity.FavoriteID) ([]*entity.Tag, error) { | |
| 604 | 681 | log := logging.FromContext(ctx) | |
| @@ -599,6 +599,83 @@ func (uc *ManageFavoritesUseCase) UntagFavorite(ctx context.Context, favID entit | |||||
| 599 | return nil | 599 | return nil | ||
| 600 | } | 600 | } | ||
| 601 | 601 | ||||
| 602 | + | // UpdateFavoriteTags reconciles a favorite's tag assignments with the requested state. | |||
| 603 | + | // Failed mutations are rolled back in reverse order when the repository cannot | |||
| 604 | + | // provide a transaction boundary. | |||
| 605 | + | func (uc *ManageFavoritesUseCase) UpdateFavoriteTags(ctx context.Context, favID entity.FavoriteID, tagIDs []entity.TagID) error { | |||
| 606 | + | currentTags, err := uc.tagRepo.GetForFavorite(ctx, favID) | |||
| 607 | + | if err != nil { | |||
| 608 | + | return fmt.Errorf("failed to load tags for favorite %d: %w", favID, err) | |||
| 609 | + | } | |||
| 610 | + | ||||
| 611 | + | current := make(map[entity.TagID]struct{}, len(currentTags)) | |||
| 612 | + | for _, tag := range currentTags { | |||
| 613 | + | if tag != nil { | |||
| 614 | + | current[tag.ID] = struct{}{} | |||
| 615 | + | } | |||
| 616 | + | } | |||
| 617 | + | wanted := make(map[entity.TagID]struct{}, len(tagIDs)) | |||
| 618 | + | for _, id := range tagIDs { | |||
| 619 | + | wanted[id] = struct{}{} | |||
| 620 | + | } | |||
| 621 | + | ||||
| 622 | + | toRemove := make([]entity.TagID, 0, len(current)) | |||
| 623 | + | for id := range current { | |||
| 624 | + | if _, ok := wanted[id]; !ok { | |||
| 625 | + | toRemove = append(toRemove, id) | |||
| 626 | + | } | |||
| 627 | + | } | |||
| 628 | + | toAdd := make([]entity.TagID, 0, len(wanted)) | |||
| 629 | + | for id := range wanted { | |||
| 630 | + | if _, ok := current[id]; !ok { | |||
| 631 | + | toAdd = append(toAdd, id) | |||
| 632 | + | } | |||
| 633 | + | } | |||
| 634 | + | sort.Slice(toRemove, func(i, j int) bool { return toRemove[i] < toRemove[j] }) | |||
| 635 | + | sort.Slice(toAdd, func(i, j int) bool { return toAdd[i] < toAdd[j] }) | |||
| 636 | + | ||||
| 637 | + | type mutation struct { | |||
| 638 | + | tagID entity.TagID | |||
| 639 | + | added bool | |||
| 640 | + | } | |||
| 641 | + | applied := make([]mutation, 0, len(toRemove)+len(toAdd)) | |||
| 642 | + | rollbackLog := logging.FromContext(ctx) | |||
| 643 | + | rollback := func() { | |||
| 644 | + | for i := len(applied) - 1; i >= 0; i-- { | |||
| 645 | + | change := applied[i] | |||
| 646 | + | var rollbackErr error | |||
| 647 | + | if change.added { | |||
| 648 | + | rollbackErr = uc.UntagFavorite(ctx, favID, change.tagID) | |||
| 649 | + | } else { | |||
| 650 | + | rollbackErr = uc.TagFavorite(ctx, favID, change.tagID) | |||
| 651 | + | } | |||
| 652 | + | if rollbackErr != nil { | |||
| 653 | + | rollbackLog.Warn(). | |||
| 654 | + | Err(rollbackErr). | |||
| 655 | + | Int64("favorite_id", int64(favID)). | |||
| 656 | + | Int64("tag_id", int64(change.tagID)). | |||
| 657 | + | Msg("failed to roll back favorite tag mutation") | |||
| 658 | + | } | |||
| 659 | + | } | |||
| 660 | + | } | |||
| 661 | + | ||||
| 662 | + | for _, id := range toRemove { | |||
| 663 | + | if err := uc.UntagFavorite(ctx, favID, id); err != nil { | |||
| 664 | + | rollback() | |||
| 665 | + | return fmt.Errorf("failed to remove tag %d from favorite %d: %w", id, favID, err) | |||
| 666 | + | } | |||
| 667 | + | applied = append(applied, mutation{tagID: id}) | |||
| 668 | + | } | |||
| 669 | + | for _, id := range toAdd { | |||
| 670 | + | if err := uc.TagFavorite(ctx, favID, id); err != nil { | |||
| 671 | + | rollback() | |||
| 672 | + | return fmt.Errorf("failed to add tag %d to favorite %d: %w", id, favID, err) | |||
| 673 | + | } | |||
| 674 | + | applied = append(applied, mutation{tagID: id, added: true}) | |||
| 675 | + | } | |||
| 676 | + | return nil | |||
| 677 | + | } | |||
| 678 | + | ||||
| 602 | // GetTagsForFavorite retrieves all tags for a favorite. | 679 | // GetTagsForFavorite retrieves all tags for a favorite. | ||
| 603 | func (uc *ManageFavoritesUseCase) GetTagsForFavorite(ctx context.Context, favID entity.FavoriteID) ([]*entity.Tag, error) { | 680 | func (uc *ManageFavoritesUseCase) GetTagsForFavorite(ctx context.Context, favID entity.FavoriteID) ([]*entity.Tag, error) { | ||
| 604 | log := logging.FromContext(ctx) | 681 | log := logging.FromContext(ctx) | ||
M internal/application/usecase/vim_mode_policy.go +2 -3
| @@ -5,9 +5,8 @@ package usecase | |||
| 5 | 5 | type VimModePolicyTransition string | |
| 6 | 6 | ||
| 7 | 7 | const ( | |
| 8 | - | VimModePolicyTransitionStay VimModePolicyTransition = "stay" | |
| 9 | - | VimModePolicyTransitionExit VimModePolicyTransition = "exit" | |
| 10 | - | VimModePolicyTransitionBlockActivation VimModePolicyTransition = "block_activation" | |
| 8 | + | VimModePolicyTransitionStay VimModePolicyTransition = "stay" | |
| 9 | + | VimModePolicyTransitionExit VimModePolicyTransition = "exit" | |
| 11 | 10 | ) | |
| 12 | 11 | ||
| 13 | 12 | // VimModePolicyTrigger identifies the focus or activation event being | |
| @@ -5,9 +5,8 @@ package usecase | |||||
| 5 | type VimModePolicyTransition string | 5 | type VimModePolicyTransition string | ||
| 6 | 6 | ||||
| 7 | const ( | 7 | const ( | ||
| 8 | - | VimModePolicyTransitionStay VimModePolicyTransition = "stay" | 8 | + | VimModePolicyTransitionStay VimModePolicyTransition = "stay" |
| 9 | - | VimModePolicyTransitionExit VimModePolicyTransition = "exit" | 9 | + | VimModePolicyTransitionExit VimModePolicyTransition = "exit" |
| 10 | - | VimModePolicyTransitionBlockActivation VimModePolicyTransition = "block_activation" | |||
| 11 | ) | 10 | ) | ||
| 12 | 11 | ||||
| 13 | // VimModePolicyTrigger identifies the focus or activation event being | 12 | // VimModePolicyTrigger identifies the focus or activation event being | ||
M internal/application/usecase/vim_navigation.go +6 -3
| @@ -8,7 +8,10 @@ import ( | |||
| 8 | 8 | "github.com/bnema/dumber/internal/application/port" | |
| 9 | 9 | ) | |
| 10 | 10 | ||
| 11 | - | var errUnsupportedVimNavigationAction = errors.New("vim navigation: unsupported action") | |
| 11 | + | var ( | |
| 12 | + | errUnsupportedVimNavigationAction = errors.New("vim navigation: unsupported action") | |
| 13 | + | errUnsupportedVimNavigationEngine = errors.New("vim navigation: unsupported webview capability") | |
| 14 | + | ) | |
| 12 | 15 | ||
| 13 | 16 | // VimNavigationUseCase maps configured Vim Mode navigation actions to optional | |
| 14 | 17 | // WebView capabilities. The UI layer supplies the active WebView and remains | |
| @@ -29,7 +32,7 @@ func (*VimNavigationUseCase) Execute(ctx context.Context, wv port.WebView, actio | |||
| 29 | 32 | if action == "focus-input" { | |
| 30 | 33 | focuser, ok := wv.(port.PageInputFocuser) | |
| 31 | 34 | if !ok { | |
| 32 | - | return nil | |
| 35 | + | return errUnsupportedVimNavigationEngine | |
| 33 | 36 | } | |
| 34 | 37 | focuser.FocusNextInput() | |
| 35 | 38 | return nil | |
| @@ -57,7 +60,7 @@ func (*VimNavigationUseCase) Execute(ctx context.Context, wv port.WebView, actio | |||
| 57 | 60 | ||
| 58 | 61 | navigator, ok := wv.(port.SemanticNavigable) | |
| 59 | 62 | if !ok { | |
| 60 | - | return nil | |
| 63 | + | return errUnsupportedVimNavigationEngine | |
| 61 | 64 | } | |
| 62 | 65 | return navigator.NavigateSemantic(ctx, request) | |
| 63 | 66 | } | |
| @@ -8,7 +8,10 @@ import ( | |||||
| 8 | "github.com/bnema/dumber/internal/application/port" | 8 | "github.com/bnema/dumber/internal/application/port" | ||
| 9 | ) | 9 | ) | ||
| 10 | 10 | ||||
| 11 | - | var errUnsupportedVimNavigationAction = errors.New("vim navigation: unsupported action") | 11 | + | var ( |
| 12 | + | errUnsupportedVimNavigationAction = errors.New("vim navigation: unsupported action") | |||
| 13 | + | errUnsupportedVimNavigationEngine = errors.New("vim navigation: unsupported webview capability") | |||
| 14 | + | ) | |||
| 12 | 15 | ||||
| 13 | // VimNavigationUseCase maps configured Vim Mode navigation actions to optional | 16 | // VimNavigationUseCase maps configured Vim Mode navigation actions to optional | ||
| 14 | // WebView capabilities. The UI layer supplies the active WebView and remains | 17 | // WebView capabilities. The UI layer supplies the active WebView and remains | ||
| @@ -29,7 +32,7 @@ func (*VimNavigationUseCase) Execute(ctx context.Context, wv port.WebView, actio | |||||
| 29 | if action == "focus-input" { | 32 | if action == "focus-input" { | ||
| 30 | focuser, ok := wv.(port.PageInputFocuser) | 33 | focuser, ok := wv.(port.PageInputFocuser) | ||
| 31 | if !ok { | 34 | if !ok { | ||
| 32 | - | return nil | 35 | + | return errUnsupportedVimNavigationEngine |
| 33 | } | 36 | } | ||
| 34 | focuser.FocusNextInput() | 37 | focuser.FocusNextInput() | ||
| 35 | return nil | 38 | return nil | ||
| @@ -57,7 +60,7 @@ func (*VimNavigationUseCase) Execute(ctx context.Context, wv port.WebView, actio | |||||
| 57 | 60 | ||||
| 58 | navigator, ok := wv.(port.SemanticNavigable) | 61 | navigator, ok := wv.(port.SemanticNavigable) | ||
| 59 | if !ok { | 62 | if !ok { | ||
| 60 | - | return nil | 63 | + | return errUnsupportedVimNavigationEngine |
| 61 | } | 64 | } | ||
| 62 | return navigator.NavigateSemantic(ctx, request) | 65 | return navigator.NavigateSemantic(ctx, request) | ||
| 63 | } | 66 | } | ||
M internal/application/usecase/vim_navigation_test.go +1 -1
| @@ -59,6 +59,6 @@ func TestVimNavigationUseCaseNavigatePageFocus(t *testing.T) { | |||
| 59 | 59 | func TestVimNavigationUseCaseIgnoresUnsupportedCapability(t *testing.T) { | |
| 60 | 60 | wv := portmocks.NewMockWebView(t) | |
| 61 | 61 | ||
| 62 | - | require.NoError(t, NewVimNavigationUseCase().Execute(context.Background(), wv, "focus-input", 1, "")) | |
| 62 | + | require.ErrorIs(t, NewVimNavigationUseCase().Execute(context.Background(), wv, "focus-input", 1, ""), errUnsupportedVimNavigationEngine) | |
| 63 | 63 | assert.False(t, NewVimNavigationUseCase().NavigatePageFocus(wv, true)) | |
| 64 | 64 | } | |
| @@ -59,6 +59,6 @@ func TestVimNavigationUseCaseNavigatePageFocus(t *testing.T) { | |||||
| 59 | func TestVimNavigationUseCaseIgnoresUnsupportedCapability(t *testing.T) { | 59 | func TestVimNavigationUseCaseIgnoresUnsupportedCapability(t *testing.T) { | ||
| 60 | wv := portmocks.NewMockWebView(t) | 60 | wv := portmocks.NewMockWebView(t) | ||
| 61 | 61 | ||||
| 62 | - | require.NoError(t, NewVimNavigationUseCase().Execute(context.Background(), wv, "focus-input", 1, "")) | 62 | + | require.ErrorIs(t, NewVimNavigationUseCase().Execute(context.Background(), wv, "focus-input", 1, ""), errUnsupportedVimNavigationEngine) |
| 63 | assert.False(t, NewVimNavigationUseCase().NavigatePageFocus(wv, true)) | 63 | assert.False(t, NewVimNavigationUseCase().NavigatePageFocus(wv, true)) | ||
| 64 | } | 64 | } | ||
M internal/domain/vimkeys/key_test.go +2 -0
| @@ -59,6 +59,8 @@ func TestSequenceString(t *testing.T) { | |||
| 59 | 59 | {name: "yah", seq: Sequence{{Sym: "y"}, {Sym: "a"}, {Sym: "h"}}, want: "yah"}, | |
| 60 | 60 | {name: "double bracket", seq: Sequence{{Sym: "]"}, {Sym: "]"}}, want: "]]"}, | |
| 61 | 61 | {name: "gO", seq: Sequence{{Sym: "g"}, {Sym: "o", Mods: ModShift}}, want: "gO"}, | |
| 62 | + | {name: "esc collision", seq: Sequence{{Sym: "e"}, {Sym: "s"}, {Sym: "c"}}, want: "<e>sc"}, | |
| 63 | + | {name: "enter collision", seq: Sequence{{Sym: "e"}, {Sym: "n"}, {Sym: "t"}, {Sym: "e"}, {Sym: "r"}}, want: "<e>nter"}, | |
| 62 | 64 | } | |
| 63 | 65 | for _, tt := range tests { | |
| 64 | 66 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -59,6 +59,8 @@ func TestSequenceString(t *testing.T) { | |||||
| 59 | {name: "yah", seq: Sequence{{Sym: "y"}, {Sym: "a"}, {Sym: "h"}}, want: "yah"}, | 59 | {name: "yah", seq: Sequence{{Sym: "y"}, {Sym: "a"}, {Sym: "h"}}, want: "yah"}, | ||
| 60 | {name: "double bracket", seq: Sequence{{Sym: "]"}, {Sym: "]"}}, want: "]]"}, | 60 | {name: "double bracket", seq: Sequence{{Sym: "]"}, {Sym: "]"}}, want: "]]"}, | ||
| 61 | {name: "gO", seq: Sequence{{Sym: "g"}, {Sym: "o", Mods: ModShift}}, want: "gO"}, | 61 | {name: "gO", seq: Sequence{{Sym: "g"}, {Sym: "o", Mods: ModShift}}, want: "gO"}, | ||
| 62 | + | {name: "esc collision", seq: Sequence{{Sym: "e"}, {Sym: "s"}, {Sym: "c"}}, want: "<e>sc"}, | |||
| 63 | + | {name: "enter collision", seq: Sequence{{Sym: "e"}, {Sym: "n"}, {Sym: "t"}, {Sym: "e"}, {Sym: "r"}}, want: "<e>nter"}, | |||
| 62 | } | 64 | } | ||
| 63 | for _, tt := range tests { | 65 | for _, tt := range tests { | ||
| 64 | t.Run(tt.name, func(t *testing.T) { | 66 | t.Run(tt.name, func(t *testing.T) { | ||
M internal/domain/vimkeys/matcher_test.go +19 -6
| @@ -137,12 +137,6 @@ func TestMatcher_Counts(t *testing.T) { | |||
| 137 | 137 | count: 10, | |
| 138 | 138 | action: "down", | |
| 139 | 139 | }, | |
| 140 | - | { | |
| 141 | - | name: "03j", | |
| 142 | - | keys: []Key{{Sym: "0"}, {Sym: "3"}, {Sym: "j"}}, | |
| 143 | - | count: 3, | |
| 144 | - | action: "down", | |
| 145 | - | }, | |
| 146 | 140 | { | |
| 147 | 141 | name: "0 binding", | |
| 148 | 142 | keys: []Key{{Sym: "0"}}, | |
| @@ -170,6 +164,25 @@ func TestMatcher_Counts(t *testing.T) { | |||
| 170 | 164 | } | |
| 171 | 165 | } | |
| 172 | 166 | ||
| 167 | + | func TestMatcher_ZeroThenCount(t *testing.T) { | |
| 168 | + | m := NewMatcher(testTrie(t)) | |
| 169 | + | ||
| 170 | + | zero := m.Feed(Key{Sym: "0"}) | |
| 171 | + | if zero.Kind != ResultComplete || zero.Action != "zero" || zero.Count != 0 { | |
| 172 | + | t.Fatalf("first zero result = %#v, want complete zero binding", zero) | |
| 173 | + | } | |
| 174 | + | ||
| 175 | + | count := m.Feed(Key{Sym: "3"}) | |
| 176 | + | if count.Kind != ResultPending || count.Pending != "3" || count.Count != 3 { | |
| 177 | + | t.Fatalf("count prefix result = %#v, want pending count 3", count) | |
| 178 | + | } | |
| 179 | + | ||
| 180 | + | complete := m.Feed(Key{Sym: "j"}) | |
| 181 | + | if complete.Kind != ResultComplete || complete.Action != "down" || complete.Count != 3 { | |
| 182 | + | t.Fatalf("counted action result = %#v, want complete down count 3", complete) | |
| 183 | + | } | |
| 184 | + | } | |
| 185 | + | ||
| 173 | 186 | func TestMatcher_CountPendingString(t *testing.T) { | |
| 174 | 187 | m := NewMatcher(testTrie(t)) | |
| 175 | 188 | m.Reset() | |
| @@ -137,12 +137,6 @@ func TestMatcher_Counts(t *testing.T) { | |||||
| 137 | count: 10, | 137 | count: 10, | ||
| 138 | action: "down", | 138 | action: "down", | ||
| 139 | }, | 139 | }, | ||
| 140 | - | { | |||
| 141 | - | name: "03j", | |||
| 142 | - | keys: []Key{{Sym: "0"}, {Sym: "3"}, {Sym: "j"}}, | |||
| 143 | - | count: 3, | |||
| 144 | - | action: "down", | |||
| 145 | - | }, | |||
| 146 | { | 140 | { | ||
| 147 | name: "0 binding", | 141 | name: "0 binding", | ||
| 148 | keys: []Key{{Sym: "0"}}, | 142 | keys: []Key{{Sym: "0"}}, | ||
| @@ -170,6 +164,25 @@ func TestMatcher_Counts(t *testing.T) { | |||||
| 170 | } | 164 | } | ||
| 171 | } | 165 | } | ||
| 172 | 166 | ||||
| 167 | + | func TestMatcher_ZeroThenCount(t *testing.T) { | |||
| 168 | + | m := NewMatcher(testTrie(t)) | |||
| 169 | + | ||||
| 170 | + | zero := m.Feed(Key{Sym: "0"}) | |||
| 171 | + | if zero.Kind != ResultComplete || zero.Action != "zero" || zero.Count != 0 { | |||
| 172 | + | t.Fatalf("first zero result = %#v, want complete zero binding", zero) | |||
| 173 | + | } | |||
| 174 | + | ||||
| 175 | + | count := m.Feed(Key{Sym: "3"}) | |||
| 176 | + | if count.Kind != ResultPending || count.Pending != "3" || count.Count != 3 { | |||
| 177 | + | t.Fatalf("count prefix result = %#v, want pending count 3", count) | |||
| 178 | + | } | |||
| 179 | + | ||||
| 180 | + | complete := m.Feed(Key{Sym: "j"}) | |||
| 181 | + | if complete.Kind != ResultComplete || complete.Action != "down" || complete.Count != 3 { | |||
| 182 | + | t.Fatalf("counted action result = %#v, want complete down count 3", complete) | |||
| 183 | + | } | |||
| 184 | + | } | |||
| 185 | + | ||||
| 173 | func TestMatcher_CountPendingString(t *testing.T) { | 186 | func TestMatcher_CountPendingString(t *testing.T) { | ||
| 174 | m := NewMatcher(testTrie(t)) | 187 | m := NewMatcher(testTrie(t)) | ||
| 175 | m.Reset() | 188 | m.Reset() | ||
M internal/domain/vimkeys/parse.go +3 -0
| @@ -7,6 +7,9 @@ import ( | |||
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | 9 | func ParseBinding(s string) (Sequence, error) { | |
| 10 | + | if !utf8.ValidString(s) { | |
| 11 | + | return nil, ErrBadBinding | |
| 12 | + | } | |
| 10 | 13 | s = strings.TrimSpace(s) | |
| 11 | 14 | if s == "" { | |
| 12 | 15 | return nil, ErrEmptyBinding | |
| @@ -7,6 +7,9 @@ import ( | |||||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | func ParseBinding(s string) (Sequence, error) { | 9 | func ParseBinding(s string) (Sequence, error) { | ||
| 10 | + | if !utf8.ValidString(s) { | |||
| 11 | + | return nil, ErrBadBinding | |||
| 12 | + | } | |||
| 10 | s = strings.TrimSpace(s) | 13 | s = strings.TrimSpace(s) | ||
| 11 | if s == "" { | 14 | if s == "" { | ||
| 12 | return nil, ErrEmptyBinding | 15 | return nil, ErrEmptyBinding | ||
M internal/infrastructure/cef/accessibility_capture.go +2 -2
| @@ -326,8 +326,8 @@ func (c *accessibilityCapture) Write(p accessibilityPayload) error { | |||
| 326 | 326 | } | |
| 327 | 327 | c.seq = next | |
| 328 | 328 | c.fileCount++ | |
| 329 | - | c.totalBytes += payloadBytes | |
| 330 | - | _, writeErr := file.WriteString(p.JSON) | |
| 329 | + | written, writeErr := file.WriteString(p.JSON) | |
| 330 | + | c.totalBytes += int64(written) | |
| 331 | 331 | chmodErr := file.Chmod(accessibilityCaptureFilePerm) | |
| 332 | 332 | closeErr := file.Close() | |
| 333 | 333 | if writeErr != nil { | |
| @@ -326,8 +326,8 @@ func (c *accessibilityCapture) Write(p accessibilityPayload) error { | |||||
| 326 | } | 326 | } | ||
| 327 | c.seq = next | 327 | c.seq = next | ||
| 328 | c.fileCount++ | 328 | c.fileCount++ | ||
| 329 | - | c.totalBytes += payloadBytes | 329 | + | written, writeErr := file.WriteString(p.JSON) |
| 330 | - | _, writeErr := file.WriteString(p.JSON) | 330 | + | c.totalBytes += int64(written) |
| 331 | chmodErr := file.Chmod(accessibilityCaptureFilePerm) | 331 | chmodErr := file.Chmod(accessibilityCaptureFilePerm) | ||
| 332 | closeErr := file.Close() | 332 | closeErr := file.Close() | ||
| 333 | if writeErr != nil { | 333 | if writeErr != nil { | ||
M internal/infrastructure/cef/input_focus.go +6 -6
| @@ -46,10 +46,10 @@ func focusNextInputScript() string { | |||
| 46 | 46 | const isEligible = (element) => { | |
| 47 | 47 | if (!isVisible(element)) return false; | |
| 48 | 48 | if (element instanceof HTMLInputElement) { | |
| 49 | - | return !element.disabled && !element.readOnly && !hiddenInputTypes.has(element.type.toLowerCase()); | |
| 49 | + | return !element.matches(":disabled") && !element.readOnly && !hiddenInputTypes.has(element.type.toLowerCase()); | |
| 50 | 50 | } | |
| 51 | 51 | if (element instanceof HTMLTextAreaElement) { | |
| 52 | - | return !element.disabled && !element.readOnly; | |
| 52 | + | return !element.matches(":disabled") && !element.readOnly; | |
| 53 | 53 | } | |
| 54 | 54 | return element.isContentEditable && element.getAttribute("contenteditable") !== "false"; | |
| 55 | 55 | }; | |
| @@ -70,9 +70,9 @@ func pageFocusNavigationScript(backward bool) string { | |||
| 70 | 70 | return `(() => { | |
| 71 | 71 | const direction = ` + direction + `; | |
| 72 | 72 | const selector = [ | |
| 73 | - | "a[href]", "area[href]", "button:not([disabled])", | |
| 74 | - | "input:not([disabled]):not([type=\"hidden\"])", "select:not([disabled])", | |
| 75 | - | "textarea:not([disabled])", "[contenteditable]:not([contenteditable=\"false\"])", | |
| 73 | + | "a[href]", "area[href]", "button:not(:disabled)", | |
| 74 | + | "input:not(:disabled):not([type=\"hidden\"])", "select:not(:disabled)", | |
| 75 | + | "textarea:not(:disabled)", "[contenteditable]:not([contenteditable=\"false\"])", | |
| 76 | 76 | "[tabindex]:not([tabindex=\"-1\"])" | |
| 77 | 77 | ].join(","); | |
| 78 | 78 | const isVisible = (element) => { | |
| @@ -83,7 +83,7 @@ func pageFocusNavigationScript(backward bool) string { | |||
| 83 | 83 | return style.display !== "none" && style.visibility !== "hidden" && style.visibility !== "collapse"; | |
| 84 | 84 | }; | |
| 85 | 85 | const candidates = Array.from(document.querySelectorAll(selector)).filter((element) => { | |
| 86 | - | return isVisible(element) && !element.disabled && element.tabIndex >= 0; | |
| 86 | + | return isVisible(element) && !element.matches(":disabled") && element.tabIndex >= 0; | |
| 87 | 87 | }); | |
| 88 | 88 | if (candidates.length === 0) return; | |
| 89 | 89 | const active = document.activeElement; | |
| @@ -46,10 +46,10 @@ func focusNextInputScript() string { | |||||
| 46 | const isEligible = (element) => { | 46 | const isEligible = (element) => { | ||
| 47 | if (!isVisible(element)) return false; | 47 | if (!isVisible(element)) return false; | ||
| 48 | if (element instanceof HTMLInputElement) { | 48 | if (element instanceof HTMLInputElement) { | ||
| 49 | - | return !element.disabled && !element.readOnly && !hiddenInputTypes.has(element.type.toLowerCase()); | 49 | + | return !element.matches(":disabled") && !element.readOnly && !hiddenInputTypes.has(element.type.toLowerCase()); |
| 50 | } | 50 | } | ||
| 51 | if (element instanceof HTMLTextAreaElement) { | 51 | if (element instanceof HTMLTextAreaElement) { | ||
| 52 | - | return !element.disabled && !element.readOnly; | 52 | + | return !element.matches(":disabled") && !element.readOnly; |
| 53 | } | 53 | } | ||
| 54 | return element.isContentEditable && element.getAttribute("contenteditable") !== "false"; | 54 | return element.isContentEditable && element.getAttribute("contenteditable") !== "false"; | ||
| 55 | }; | 55 | }; | ||
| @@ -70,9 +70,9 @@ func pageFocusNavigationScript(backward bool) string { | |||||
| 70 | return `(() => { | 70 | return `(() => { | ||
| 71 | const direction = ` + direction + `; | 71 | const direction = ` + direction + `; | ||
| 72 | const selector = [ | 72 | const selector = [ | ||
| 73 | - | "a[href]", "area[href]", "button:not([disabled])", | 73 | + | "a[href]", "area[href]", "button:not(:disabled)", |
| 74 | - | "input:not([disabled]):not([type=\"hidden\"])", "select:not([disabled])", | 74 | + | "input:not(:disabled):not([type=\"hidden\"])", "select:not(:disabled)", |
| 75 | - | "textarea:not([disabled])", "[contenteditable]:not([contenteditable=\"false\"])", | 75 | + | "textarea:not(:disabled)", "[contenteditable]:not([contenteditable=\"false\"])", |
| 76 | "[tabindex]:not([tabindex=\"-1\"])" | 76 | "[tabindex]:not([tabindex=\"-1\"])" | ||
| 77 | ].join(","); | 77 | ].join(","); | ||
| 78 | const isVisible = (element) => { | 78 | const isVisible = (element) => { | ||
| @@ -83,7 +83,7 @@ func pageFocusNavigationScript(backward bool) string { | |||||
| 83 | return style.display !== "none" && style.visibility !== "hidden" && style.visibility !== "collapse"; | 83 | return style.display !== "none" && style.visibility !== "hidden" && style.visibility !== "collapse"; | ||
| 84 | }; | 84 | }; | ||
| 85 | const candidates = Array.from(document.querySelectorAll(selector)).filter((element) => { | 85 | const candidates = Array.from(document.querySelectorAll(selector)).filter((element) => { | ||
| 86 | - | return isVisible(element) && !element.disabled && element.tabIndex >= 0; | 86 | + | return isVisible(element) && !element.matches(":disabled") && element.tabIndex >= 0; |
| 87 | }); | 87 | }); | ||
| 88 | if (candidates.length === 0) return; | 88 | if (candidates.length === 0) return; | ||
| 89 | const active = document.activeElement; | 89 | const active = document.activeElement; | ||
M internal/infrastructure/cef/input_focus_test.go +2 -2
| @@ -32,7 +32,7 @@ func TestWebViewNavigatePageFocusExecutesDirectionalScript(t *testing.T) { | |||
| 32 | 32 | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(mockStringContaining(t, | |
| 33 | 33 | "const direction = -1;", | |
| 34 | 34 | "a[href]", | |
| 35 | - | "textarea:not([disabled])", | |
| 35 | + | "textarea:not(:disabled)", | |
| 36 | 36 | "document.activeElement", | |
| 37 | 37 | "targetIndex", | |
| 38 | 38 | "candidates[targetIndex].focus()", | |
| @@ -47,7 +47,7 @@ func TestFocusNextInputScriptWrapsAndNoOpsWithoutEligibleTarget(t *testing.T) { | |||
| 47 | 47 | require.Contains(t, script, "const eligible = candidates.filter(isEligible);") | |
| 48 | 48 | require.Contains(t, script, "if (eligible.length === 0) return;") | |
| 49 | 49 | require.Contains(t, script, "const target = eligible[(activeIndex + 1 + eligible.length) % eligible.length];") | |
| 50 | - | require.Contains(t, script, "!element.disabled && !element.readOnly") | |
| 50 | + | require.Contains(t, script, "!element.matches(\":disabled\") && !element.readOnly") | |
| 51 | 51 | require.Contains(t, script, `hiddenInputTypes.has(element.type.toLowerCase())`) | |
| 52 | 52 | } | |
| 53 | 53 | ||
| @@ -32,7 +32,7 @@ func TestWebViewNavigatePageFocusExecutesDirectionalScript(t *testing.T) { | |||||
| 32 | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(mockStringContaining(t, | 32 | frame.EXPECT().ExecuteJavaScript(mock.MatchedBy(mockStringContaining(t, | ||
| 33 | "const direction = -1;", | 33 | "const direction = -1;", | ||
| 34 | "a[href]", | 34 | "a[href]", | ||
| 35 | - | "textarea:not([disabled])", | 35 | + | "textarea:not(:disabled)", |
| 36 | "document.activeElement", | 36 | "document.activeElement", | ||
| 37 | "targetIndex", | 37 | "targetIndex", | ||
| 38 | "candidates[targetIndex].focus()", | 38 | "candidates[targetIndex].focus()", | ||
| @@ -47,7 +47,7 @@ func TestFocusNextInputScriptWrapsAndNoOpsWithoutEligibleTarget(t *testing.T) { | |||||
| 47 | require.Contains(t, script, "const eligible = candidates.filter(isEligible);") | 47 | require.Contains(t, script, "const eligible = candidates.filter(isEligible);") | ||
| 48 | require.Contains(t, script, "if (eligible.length === 0) return;") | 48 | require.Contains(t, script, "if (eligible.length === 0) return;") | ||
| 49 | require.Contains(t, script, "const target = eligible[(activeIndex + 1 + eligible.length) % eligible.length];") | 49 | require.Contains(t, script, "const target = eligible[(activeIndex + 1 + eligible.length) % eligible.length];") | ||
| 50 | - | require.Contains(t, script, "!element.disabled && !element.readOnly") | 50 | + | require.Contains(t, script, "!element.matches(\":disabled\") && !element.readOnly") |
| 51 | require.Contains(t, script, `hiddenInputTypes.has(element.type.toLowerCase())`) | 51 | require.Contains(t, script, `hiddenInputTypes.has(element.type.toLowerCase())`) | ||
| 52 | } | 52 | } | ||
| 53 | 53 | ||||
M internal/infrastructure/cef/semantic_navigation.go +1 -0
| @@ -55,6 +55,7 @@ func headingNavigationScript(direction, count int, highlightColor string) string | |||
| 55 | 55 | ); | |
| 56 | 56 | if (` + strconv.Itoa(direction) + ` > 0) { | |
| 57 | 57 | index = firstAtOrAfterViewportTop - 1; | |
| 58 | + | if (index < -1) index = -1; | |
| 58 | 59 | } else { | |
| 59 | 60 | index = firstAtOrAfterViewportTop >= 0 ? firstAtOrAfterViewportTop : headings.length; | |
| 60 | 61 | } | |
| @@ -55,6 +55,7 @@ func headingNavigationScript(direction, count int, highlightColor string) string | |||||
| 55 | ); | 55 | ); | ||
| 56 | if (` + strconv.Itoa(direction) + ` > 0) { | 56 | if (` + strconv.Itoa(direction) + ` > 0) { | ||
| 57 | index = firstAtOrAfterViewportTop - 1; | 57 | index = firstAtOrAfterViewportTop - 1; | ||
| 58 | + | if (index < -1) index = -1; | |||
| 58 | } else { | 59 | } else { | ||
| 59 | index = firstAtOrAfterViewportTop >= 0 ? firstAtOrAfterViewportTop : headings.length; | 60 | index = firstAtOrAfterViewportTop >= 0 ? firstAtOrAfterViewportTop : headings.length; | ||
| 60 | } | 61 | } | ||
M internal/infrastructure/cef/semantic_navigation_test.go +1 -0
| @@ -45,6 +45,7 @@ func TestWebViewNavigateSemantic_NormalizesCountAndRejectsUnsupportedRequests(t | |||
| 45 | 45 | forward := headingNavigationScript(1, 1, "") | |
| 46 | 46 | backward := headingNavigationScript(-1, 1, "#4ade80") | |
| 47 | 47 | assert.Contains(t, forward, "index = firstAtOrAfterViewportTop - 1") | |
| 48 | + | assert.Contains(t, forward, "if (index < -1) index = -1;") | |
| 48 | 49 | assert.Contains(t, backward, "index = firstAtOrAfterViewportTop >= 0 ? firstAtOrAfterViewportTop : headings.length") | |
| 49 | 50 | assert.Contains(t, backward, "-1 * 1") | |
| 50 | 51 | assert.NotContains(t, backward, "window.innerHeight") | |
| @@ -45,6 +45,7 @@ func TestWebViewNavigateSemantic_NormalizesCountAndRejectsUnsupportedRequests(t | |||||
| 45 | forward := headingNavigationScript(1, 1, "") | 45 | forward := headingNavigationScript(1, 1, "") | ||
| 46 | backward := headingNavigationScript(-1, 1, "#4ade80") | 46 | backward := headingNavigationScript(-1, 1, "#4ade80") | ||
| 47 | assert.Contains(t, forward, "index = firstAtOrAfterViewportTop - 1") | 47 | assert.Contains(t, forward, "index = firstAtOrAfterViewportTop - 1") | ||
| 48 | + | assert.Contains(t, forward, "if (index < -1) index = -1;") | |||
| 48 | assert.Contains(t, backward, "index = firstAtOrAfterViewportTop >= 0 ? firstAtOrAfterViewportTop : headings.length") | 49 | assert.Contains(t, backward, "index = firstAtOrAfterViewportTop >= 0 ? firstAtOrAfterViewportTop : headings.length") | ||
| 49 | assert.Contains(t, backward, "-1 * 1") | 50 | assert.Contains(t, backward, "-1 * 1") | ||
| 50 | assert.NotContains(t, backward, "window.innerHeight") | 51 | assert.NotContains(t, backward, "window.innerHeight") | ||
M internal/infrastructure/cef/webview.go +38 -7
| @@ -2435,6 +2435,15 @@ func pageScrollWheelDeltas(request port.PageScrollRequest) (deltaX, deltaY int32 | |||
| 2435 | 2435 | // exhausted nested scroller can hand off automatically to its ancestor or the | |
| 2436 | 2436 | // document. Native precision-wheel input remains the pre-frame fallback. | |
| 2437 | 2437 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | |
| 2438 | + | if wv == nil { | |
| 2439 | + | return errors.New("cef: webview is nil") | |
| 2440 | + | } | |
| 2441 | + | if err := ctx.Err(); err != nil { | |
| 2442 | + | return err | |
| 2443 | + | } | |
| 2444 | + | if err := request.Validate(); err != nil { | |
| 2445 | + | return err | |
| 2446 | + | } | |
| 2438 | 2447 | if wv.destroyed.Load() { | |
| 2439 | 2448 | return errDestroyed | |
| 2440 | 2449 | } | |
| @@ -2485,7 +2494,7 @@ func (wv *WebView) ensurePageScrollFlush() { | |||
| 2485 | 2494 | q.mu.Unlock() | |
| 2486 | 2495 | ||
| 2487 | 2496 | if wv.engine == nil { | |
| 2488 | - | wv.flushPageScroll() | |
| 2497 | + | wv.drainPageScrollSynchronously() | |
| 2489 | 2498 | return | |
| 2490 | 2499 | } | |
| 2491 | 2500 | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | |
| @@ -2496,17 +2505,43 @@ func (wv *WebView) ensurePageScrollFlush() { | |||
| 2496 | 2505 | q.heldDX = 0 | |
| 2497 | 2506 | q.heldDY = 0 | |
| 2498 | 2507 | q.mu.Unlock() | |
| 2499 | - | wv.flushPageScroll() | |
| 2508 | + | wv.drainPageScrollSynchronously() | |
| 2509 | + | } | |
| 2510 | + | } | |
| 2511 | + | ||
| 2512 | + | func (wv *WebView) drainPageScrollSynchronously() { | |
| 2513 | + | for wv.drainPageScrollOnce() { | |
| 2500 | 2514 | } | |
| 2515 | + | q := &wv.pageScrollQueue | |
| 2516 | + | q.mu.Lock() | |
| 2517 | + | q.flushPending = false | |
| 2518 | + | q.mu.Unlock() | |
| 2501 | 2519 | } | |
| 2502 | 2520 | ||
| 2503 | 2521 | func (wv *WebView) flushPageScroll() { | |
| 2522 | + | wv.drainPageScrollOnce() | |
| 2523 | + | ||
| 2524 | + | q := &wv.pageScrollQueue | |
| 2525 | + | q.mu.Lock() | |
| 2526 | + | q.flushPending = false | |
| 2527 | + | hasWork := q.tapDX != 0 || q.tapDY != 0 || q.heldDX != 0 || q.heldDY != 0 | |
| 2528 | + | q.mu.Unlock() | |
| 2529 | + | if hasWork { | |
| 2530 | + | wv.ensurePageScrollFlush() | |
| 2531 | + | } | |
| 2532 | + | } | |
| 2533 | + | ||
| 2534 | + | func (wv *WebView) drainPageScrollOnce() bool { | |
| 2504 | 2535 | q := &wv.pageScrollQueue | |
| 2505 | 2536 | q.mu.Lock() | |
| 2506 | 2537 | tapDX, tapDY := q.tapDX, q.tapDY | |
| 2507 | 2538 | heldDX, heldDY := q.heldDX, q.heldDY | |
| 2508 | 2539 | heldGeneration := q.generation | |
| 2509 | 2540 | beforeExecute := q.beforeExecute | |
| 2541 | + | if tapDX == 0 && tapDY == 0 && heldDX == 0 && heldDY == 0 { | |
| 2542 | + | q.mu.Unlock() | |
| 2543 | + | return false | |
| 2544 | + | } | |
| 2510 | 2545 | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | |
| 2511 | 2546 | q.mu.Unlock() | |
| 2512 | 2547 | ||
| @@ -2528,11 +2563,7 @@ func (wv *WebView) flushPageScroll() { | |||
| 2528 | 2563 | wv.executeJavaScriptNow(webutil.BuildPageScrollByJS(dx, dy)) | |
| 2529 | 2564 | } | |
| 2530 | 2565 | q.commitMu.Unlock() | |
| 2531 | - | ||
| 2532 | - | q.mu.Lock() | |
| 2533 | - | q.flushPending = false | |
| 2534 | - | q.mu.Unlock() | |
| 2535 | - | wv.ensurePageScrollFlush() | |
| 2566 | + | return true | |
| 2536 | 2567 | } | |
| 2537 | 2568 | ||
| 2538 | 2569 | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | |
| @@ -2435,6 +2435,15 @@ func pageScrollWheelDeltas(request port.PageScrollRequest) (deltaX, deltaY int32 | |||||
| 2435 | // exhausted nested scroller can hand off automatically to its ancestor or the | 2435 | // exhausted nested scroller can hand off automatically to its ancestor or the | ||
| 2436 | // document. Native precision-wheel input remains the pre-frame fallback. | 2436 | // document. Native precision-wheel input remains the pre-frame fallback. | ||
| 2437 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | 2437 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | ||
| 2438 | + | if wv == nil { | |||
| 2439 | + | return errors.New("cef: webview is nil") | |||
| 2440 | + | } | |||
| 2441 | + | if err := ctx.Err(); err != nil { | |||
| 2442 | + | return err | |||
| 2443 | + | } | |||
| 2444 | + | if err := request.Validate(); err != nil { | |||
| 2445 | + | return err | |||
| 2446 | + | } | |||
| 2438 | if wv.destroyed.Load() { | 2447 | if wv.destroyed.Load() { | ||
| 2439 | return errDestroyed | 2448 | return errDestroyed | ||
| 2440 | } | 2449 | } | ||
| @@ -2485,7 +2494,7 @@ func (wv *WebView) ensurePageScrollFlush() { | |||||
| 2485 | q.mu.Unlock() | 2494 | q.mu.Unlock() | ||
| 2486 | 2495 | ||||
| 2487 | if wv.engine == nil { | 2496 | if wv.engine == nil { | ||
| 2488 | - | wv.flushPageScroll() | 2497 | + | wv.drainPageScrollSynchronously() |
| 2489 | return | 2498 | return | ||
| 2490 | } | 2499 | } | ||
| 2491 | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | 2500 | task := cefNewTask(cefTaskFunc(wv.flushPageScroll)) | ||
| @@ -2496,17 +2505,43 @@ func (wv *WebView) ensurePageScrollFlush() { | |||||
| 2496 | q.heldDX = 0 | 2505 | q.heldDX = 0 | ||
| 2497 | q.heldDY = 0 | 2506 | q.heldDY = 0 | ||
| 2498 | q.mu.Unlock() | 2507 | q.mu.Unlock() | ||
| 2499 | - | wv.flushPageScroll() | 2508 | + | wv.drainPageScrollSynchronously() |
| 2509 | + | } | |||
| 2510 | + | } | |||
| 2511 | + | ||||
| 2512 | + | func (wv *WebView) drainPageScrollSynchronously() { | |||
| 2513 | + | for wv.drainPageScrollOnce() { | |||
| 2500 | } | 2514 | } | ||
| 2515 | + | q := &wv.pageScrollQueue | |||
| 2516 | + | q.mu.Lock() | |||
| 2517 | + | q.flushPending = false | |||
| 2518 | + | q.mu.Unlock() | |||
| 2501 | } | 2519 | } | ||
| 2502 | 2520 | ||||
| 2503 | func (wv *WebView) flushPageScroll() { | 2521 | func (wv *WebView) flushPageScroll() { | ||
| 2522 | + | wv.drainPageScrollOnce() | |||
| 2523 | + | ||||
| 2524 | + | q := &wv.pageScrollQueue | |||
| 2525 | + | q.mu.Lock() | |||
| 2526 | + | q.flushPending = false | |||
| 2527 | + | hasWork := q.tapDX != 0 || q.tapDY != 0 || q.heldDX != 0 || q.heldDY != 0 | |||
| 2528 | + | q.mu.Unlock() | |||
| 2529 | + | if hasWork { | |||
| 2530 | + | wv.ensurePageScrollFlush() | |||
| 2531 | + | } | |||
| 2532 | + | } | |||
| 2533 | + | ||||
| 2534 | + | func (wv *WebView) drainPageScrollOnce() bool { | |||
| 2504 | q := &wv.pageScrollQueue | 2535 | q := &wv.pageScrollQueue | ||
| 2505 | q.mu.Lock() | 2536 | q.mu.Lock() | ||
| 2506 | tapDX, tapDY := q.tapDX, q.tapDY | 2537 | tapDX, tapDY := q.tapDX, q.tapDY | ||
| 2507 | heldDX, heldDY := q.heldDX, q.heldDY | 2538 | heldDX, heldDY := q.heldDX, q.heldDY | ||
| 2508 | heldGeneration := q.generation | 2539 | heldGeneration := q.generation | ||
| 2509 | beforeExecute := q.beforeExecute | 2540 | beforeExecute := q.beforeExecute | ||
| 2541 | + | if tapDX == 0 && tapDY == 0 && heldDX == 0 && heldDY == 0 { | |||
| 2542 | + | q.mu.Unlock() | |||
| 2543 | + | return false | |||
| 2544 | + | } | |||
| 2510 | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | 2545 | q.tapDX, q.tapDY, q.heldDX, q.heldDY = 0, 0, 0, 0 | ||
| 2511 | q.mu.Unlock() | 2546 | q.mu.Unlock() | ||
| 2512 | 2547 | ||||
| @@ -2528,11 +2563,7 @@ func (wv *WebView) flushPageScroll() { | |||||
| 2528 | wv.executeJavaScriptNow(webutil.BuildPageScrollByJS(dx, dy)) | 2563 | wv.executeJavaScriptNow(webutil.BuildPageScrollByJS(dx, dy)) | ||
| 2529 | } | 2564 | } | ||
| 2530 | q.commitMu.Unlock() | 2565 | q.commitMu.Unlock() | ||
| 2531 | - | 2566 | + | return true | |
| 2532 | - | q.mu.Lock() | |||
| 2533 | - | q.flushPending = false | |||
| 2534 | - | q.mu.Unlock() | |||
| 2535 | - | wv.ensurePageScrollFlush() | |||
| 2536 | } | 2567 | } | ||
| 2537 | 2568 | ||||
| 2538 | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | 2569 | // CancelPageScroll invalidates all accepted but unconsumed held-key deltas. | ||
M internal/infrastructure/config/keybindings_gateway_test.go +16 -24
| @@ -2,7 +2,6 @@ package config | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "os" | |
| 6 | 5 | "path/filepath" | |
| 7 | 6 | "sync" | |
| 8 | 7 | "testing" | |
| @@ -118,13 +117,11 @@ func TestKeybindingsGateway_VimModeSetKeybindingUpdatesFile(t *testing.T) { | |||
| 118 | 117 | }) | |
| 119 | 118 | require.NoError(t, err) | |
| 120 | 119 | ||
| 121 | - | // Verify the file was written with the updated keybinding. | |
| 122 | - | configFile := mgr.GetConfigFile() | |
| 123 | - | data, err := os.ReadFile(configFile) | |
| 124 | - | require.NoError(t, err) | |
| 125 | - | content := string(data) | |
| 126 | - | assert.Contains(t, content, `keys = ['a']`) | |
| 127 | - | assert.NotContains(t, content, `keys = ['h']`) | |
| 120 | + | // Reload and assert on the parsed Vim-mode configuration rather than raw TOML text. | |
| 121 | + | reloaded, reloadErr := NewManager() | |
| 122 | + | require.NoError(t, reloadErr) | |
| 123 | + | require.NoError(t, reloaded.Load()) | |
| 124 | + | assert.Equal(t, []string{"a"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-left"].Keys) | |
| 128 | 125 | } | |
| 129 | 126 | ||
| 130 | 127 | func TestKeybindingsGateway_VimModeResetKeybinding(t *testing.T) { | |
| @@ -145,13 +142,11 @@ func TestKeybindingsGateway_VimModeResetKeybinding(t *testing.T) { | |||
| 145 | 142 | }) | |
| 146 | 143 | require.NoError(t, err) | |
| 147 | 144 | ||
| 148 | - | // Verify the file was updated back to default. | |
| 149 | - | configFile := mgr.GetConfigFile() | |
| 150 | - | data, err := os.ReadFile(configFile) | |
| 151 | - | require.NoError(t, err) | |
| 152 | - | content := string(data) | |
| 153 | - | assert.Contains(t, content, `keys = ['h']`) | |
| 154 | - | assert.NotContains(t, content, `keys = ['a']`) | |
| 145 | + | // Reload and assert on the parsed default binding. | |
| 146 | + | reloaded, reloadErr := NewManager() | |
| 147 | + | require.NoError(t, reloadErr) | |
| 148 | + | require.NoError(t, reloaded.Load()) | |
| 149 | + | assert.Equal(t, []string{"h"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-left"].Keys) | |
| 155 | 150 | } | |
| 156 | 151 | ||
| 157 | 152 | func TestKeybindingsGateway_VimModeIsPartOfResetAll(t *testing.T) { | |
| @@ -176,15 +171,12 @@ func TestKeybindingsGateway_VimModeIsPartOfResetAll(t *testing.T) { | |||
| 176 | 171 | err := gw.ResetAllKeybindings(context.Background()) | |
| 177 | 172 | require.NoError(t, err) | |
| 178 | 173 | ||
| 179 | - | // Verify file has defaults. | |
| 180 | - | configFile := mgr.GetConfigFile() | |
| 181 | - | data, err := os.ReadFile(configFile) | |
| 182 | - | require.NoError(t, err) | |
| 183 | - | content := string(data) | |
| 184 | - | assert.Contains(t, content, `keys = ['h']`, "vim-scroll-left should be restored to default") | |
| 185 | - | assert.Contains(t, content, `keys = ['j']`, "vim-scroll-down should be restored to default") | |
| 186 | - | assert.NotContains(t, content, "keys = ['a']") | |
| 187 | - | assert.NotContains(t, content, "keys = ['b']") | |
| 174 | + | // Reload and assert on the parsed default bindings. | |
| 175 | + | reloaded, reloadErr := NewManager() | |
| 176 | + | require.NoError(t, reloadErr) | |
| 177 | + | require.NoError(t, reloaded.Load()) | |
| 178 | + | assert.Equal(t, []string{"h"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-left"].Keys, "vim-scroll-left should be restored to default") | |
| 179 | + | assert.Equal(t, []string{"j"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-down"].Keys, "vim-scroll-down should be restored to default") | |
| 188 | 180 | } | |
| 189 | 181 | ||
| 190 | 182 | func TestKeybindingsGateway_VimModeConflictsWithinSelf(t *testing.T) { | |
| @@ -2,7 +2,6 @@ package config | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | - | "os" | |||
| 6 | "path/filepath" | 5 | "path/filepath" | ||
| 7 | "sync" | 6 | "sync" | ||
| 8 | "testing" | 7 | "testing" | ||
| @@ -118,13 +117,11 @@ func TestKeybindingsGateway_VimModeSetKeybindingUpdatesFile(t *testing.T) { | |||||
| 118 | }) | 117 | }) | ||
| 119 | require.NoError(t, err) | 118 | require.NoError(t, err) | ||
| 120 | 119 | ||||
| 121 | - | // Verify the file was written with the updated keybinding. | 120 | + | // Reload and assert on the parsed Vim-mode configuration rather than raw TOML text. |
| 122 | - | configFile := mgr.GetConfigFile() | 121 | + | reloaded, reloadErr := NewManager() |
| 123 | - | data, err := os.ReadFile(configFile) | 122 | + | require.NoError(t, reloadErr) |
| 124 | - | require.NoError(t, err) | 123 | + | require.NoError(t, reloaded.Load()) |
| 125 | - | content := string(data) | 124 | + | assert.Equal(t, []string{"a"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-left"].Keys) |
| 126 | - | assert.Contains(t, content, `keys = ['a']`) | |||
| 127 | - | assert.NotContains(t, content, `keys = ['h']`) | |||
| 128 | } | 125 | } | ||
| 129 | 126 | ||||
| 130 | func TestKeybindingsGateway_VimModeResetKeybinding(t *testing.T) { | 127 | func TestKeybindingsGateway_VimModeResetKeybinding(t *testing.T) { | ||
| @@ -145,13 +142,11 @@ func TestKeybindingsGateway_VimModeResetKeybinding(t *testing.T) { | |||||
| 145 | }) | 142 | }) | ||
| 146 | require.NoError(t, err) | 143 | require.NoError(t, err) | ||
| 147 | 144 | ||||
| 148 | - | // Verify the file was updated back to default. | 145 | + | // Reload and assert on the parsed default binding. |
| 149 | - | configFile := mgr.GetConfigFile() | 146 | + | reloaded, reloadErr := NewManager() |
| 150 | - | data, err := os.ReadFile(configFile) | 147 | + | require.NoError(t, reloadErr) |
| 151 | - | require.NoError(t, err) | 148 | + | require.NoError(t, reloaded.Load()) |
| 152 | - | content := string(data) | 149 | + | assert.Equal(t, []string{"h"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-left"].Keys) |
| 153 | - | assert.Contains(t, content, `keys = ['h']`) | |||
| 154 | - | assert.NotContains(t, content, `keys = ['a']`) | |||
| 155 | } | 150 | } | ||
| 156 | 151 | ||||
| 157 | func TestKeybindingsGateway_VimModeIsPartOfResetAll(t *testing.T) { | 152 | func TestKeybindingsGateway_VimModeIsPartOfResetAll(t *testing.T) { | ||
| @@ -176,15 +171,12 @@ func TestKeybindingsGateway_VimModeIsPartOfResetAll(t *testing.T) { | |||||
| 176 | err := gw.ResetAllKeybindings(context.Background()) | 171 | err := gw.ResetAllKeybindings(context.Background()) | ||
| 177 | require.NoError(t, err) | 172 | require.NoError(t, err) | ||
| 178 | 173 | ||||
| 179 | - | // Verify file has defaults. | 174 | + | // Reload and assert on the parsed default bindings. |
| 180 | - | configFile := mgr.GetConfigFile() | 175 | + | reloaded, reloadErr := NewManager() |
| 181 | - | data, err := os.ReadFile(configFile) | 176 | + | require.NoError(t, reloadErr) |
| 182 | - | require.NoError(t, err) | 177 | + | require.NoError(t, reloaded.Load()) |
| 183 | - | content := string(data) | 178 | + | assert.Equal(t, []string{"h"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-left"].Keys, "vim-scroll-left should be restored to default") |
| 184 | - | assert.Contains(t, content, `keys = ['h']`, "vim-scroll-left should be restored to default") | 179 | + | assert.Equal(t, []string{"j"}, reloaded.Get().Workspace.VimMode.Actions["vim-scroll-down"].Keys, "vim-scroll-down should be restored to default") |
| 185 | - | assert.Contains(t, content, `keys = ['j']`, "vim-scroll-down should be restored to default") | |||
| 186 | - | assert.NotContains(t, content, "keys = ['a']") | |||
| 187 | - | assert.NotContains(t, content, "keys = ['b']") | |||
| 188 | } | 180 | } | ||
| 189 | 181 | ||||
| 190 | func TestKeybindingsGateway_VimModeConflictsWithinSelf(t *testing.T) { | 182 | func TestKeybindingsGateway_VimModeConflictsWithinSelf(t *testing.T) { | ||
M internal/infrastructure/config/migrate.go +16 -6
| @@ -811,19 +811,29 @@ func (m *Migrator) detectLegacyVimModeRenames(rawUserKeys map[string]any) []port | |||
| 811 | 811 | return changes | |
| 812 | 812 | } | |
| 813 | 813 | ||
| 814 | - | func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |
| 815 | - | workspace, ok := rawConfig["workspace"].(map[string]any) | |
| 814 | + | func (m *Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |
| 815 | + | workspace, ok := m.toStringAnyMap(rawConfig["workspace"]) | |
| 816 | 816 | if !ok { | |
| 817 | 817 | return | |
| 818 | 818 | } | |
| 819 | - | legacyMode, ok := workspace["page_mode"].(map[string]any) | |
| 819 | + | rawConfig["workspace"] = workspace | |
| 820 | + | legacyMode, ok := m.toStringAnyMap(workspace["page_mode"]) | |
| 820 | 821 | if !ok { | |
| 821 | 822 | return | |
| 822 | 823 | } | |
| 823 | - | vimMode, ok := workspace["vim_mode"].(map[string]any) | |
| 824 | + | workspace["page_mode"] = legacyMode | |
| 825 | + | vimMode, ok := m.toStringAnyMap(workspace["vim_mode"]) | |
| 824 | 826 | if !ok { | |
| 825 | 827 | vimMode = make(map[string]any) | |
| 826 | 828 | workspace["vim_mode"] = vimMode | |
| 829 | + | } else { | |
| 830 | + | workspace["vim_mode"] = vimMode | |
| 831 | + | } | |
| 832 | + | if legacyActions, ok := m.toStringAnyMap(legacyMode["actions"]); ok { | |
| 833 | + | legacyMode["actions"] = legacyActions | |
| 834 | + | } | |
| 835 | + | if vimActions, ok := m.toStringAnyMap(vimMode["actions"]); ok { | |
| 836 | + | vimMode["actions"] = vimActions | |
| 827 | 837 | } | |
| 828 | 838 | for key, value := range legacyMode { | |
| 829 | 839 | if key == "actions" { | |
| @@ -833,8 +843,8 @@ func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |||
| 833 | 843 | vimMode[key] = value | |
| 834 | 844 | } | |
| 835 | 845 | } | |
| 836 | - | legacyActions, _ := legacyMode["actions"].(map[string]any) | |
| 837 | - | vimActions, _ := vimMode["actions"].(map[string]any) | |
| 846 | + | legacyActions, _ := m.toStringAnyMap(legacyMode["actions"]) | |
| 847 | + | vimActions, _ := m.toStringAnyMap(vimMode["actions"]) | |
| 838 | 848 | if vimActions == nil { | |
| 839 | 849 | vimActions = make(map[string]any) | |
| 840 | 850 | vimMode["actions"] = vimActions | |
| @@ -811,19 +811,29 @@ func (m *Migrator) detectLegacyVimModeRenames(rawUserKeys map[string]any) []port | |||||
| 811 | return changes | 811 | return changes | ||
| 812 | } | 812 | } | ||
| 813 | 813 | ||||
| 814 | - | func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | 814 | + | func (m *Migrator) migrateLegacyVimMode(rawConfig map[string]any) { |
| 815 | - | workspace, ok := rawConfig["workspace"].(map[string]any) | 815 | + | workspace, ok := m.toStringAnyMap(rawConfig["workspace"]) |
| 816 | if !ok { | 816 | if !ok { | ||
| 817 | return | 817 | return | ||
| 818 | } | 818 | } | ||
| 819 | - | legacyMode, ok := workspace["page_mode"].(map[string]any) | 819 | + | rawConfig["workspace"] = workspace |
| 820 | + | legacyMode, ok := m.toStringAnyMap(workspace["page_mode"]) | |||
| 820 | if !ok { | 821 | if !ok { | ||
| 821 | return | 822 | return | ||
| 822 | } | 823 | } | ||
| 823 | - | vimMode, ok := workspace["vim_mode"].(map[string]any) | 824 | + | workspace["page_mode"] = legacyMode |
| 825 | + | vimMode, ok := m.toStringAnyMap(workspace["vim_mode"]) | |||
| 824 | if !ok { | 826 | if !ok { | ||
| 825 | vimMode = make(map[string]any) | 827 | vimMode = make(map[string]any) | ||
| 826 | workspace["vim_mode"] = vimMode | 828 | workspace["vim_mode"] = vimMode | ||
| 829 | + | } else { | |||
| 830 | + | workspace["vim_mode"] = vimMode | |||
| 831 | + | } | |||
| 832 | + | if legacyActions, ok := m.toStringAnyMap(legacyMode["actions"]); ok { | |||
| 833 | + | legacyMode["actions"] = legacyActions | |||
| 834 | + | } | |||
| 835 | + | if vimActions, ok := m.toStringAnyMap(vimMode["actions"]); ok { | |||
| 836 | + | vimMode["actions"] = vimActions | |||
| 827 | } | 837 | } | ||
| 828 | for key, value := range legacyMode { | 838 | for key, value := range legacyMode { | ||
| 829 | if key == "actions" { | 839 | if key == "actions" { | ||
| @@ -833,8 +843,8 @@ func (*Migrator) migrateLegacyVimMode(rawConfig map[string]any) { | |||||
| 833 | vimMode[key] = value | 843 | vimMode[key] = value | ||
| 834 | } | 844 | } | ||
| 835 | } | 845 | } | ||
| 836 | - | legacyActions, _ := legacyMode["actions"].(map[string]any) | 846 | + | legacyActions, _ := m.toStringAnyMap(legacyMode["actions"]) |
| 837 | - | vimActions, _ := vimMode["actions"].(map[string]any) | 847 | + | vimActions, _ := m.toStringAnyMap(vimMode["actions"]) |
| 838 | if vimActions == nil { | 848 | if vimActions == nil { | ||
| 839 | vimActions = make(map[string]any) | 849 | vimActions = make(map[string]any) | ||
| 840 | vimMode["actions"] = vimActions | 850 | vimMode["actions"] = vimActions | ||
M internal/infrastructure/config/validation.go +31 -15
| @@ -278,21 +278,7 @@ func validateModalModeActionsWithKeyCanon( | |||
| 278 | 278 | } | |
| 279 | 279 | ||
| 280 | 280 | if keyCanon == nil { | |
| 281 | - | seenKeys := make(map[string]string) | |
| 282 | - | for action, binding := range actions { | |
| 283 | - | if len(binding.Keys) == 0 { | |
| 284 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.actions.%s must have at least one key binding", modePath, action)) | |
| 285 | - | } | |
| 286 | - | for _, key := range binding.Keys { | |
| 287 | - | if existingAction, exists := seenKeys[key]; exists { | |
| 288 | - | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 289 | - | "duplicate key binding '%s' found in %s actions '%s' and '%s'", key, modePath, existingAction, action, | |
| 290 | - | )) | |
| 291 | - | } | |
| 292 | - | seenKeys[key] = action | |
| 293 | - | } | |
| 294 | - | } | |
| 295 | - | return validationErrors | |
| 281 | + | keyCanon = func(key string) (string, error) { return key, nil } | |
| 296 | 282 | } | |
| 297 | 283 | ||
| 298 | 284 | actionNames := make([]string, 0, len(actions)) | |
| @@ -322,6 +308,32 @@ func validateModalModeActionsWithKeyCanon( | |||
| 322 | 308 | )) | |
| 323 | 309 | continue | |
| 324 | 310 | } | |
| 311 | + | if modePath == "vim_mode" { | |
| 312 | + | sequence, parseErr := vimkeys.ParseBinding(canonical) | |
| 313 | + | if parseErr == nil { | |
| 314 | + | if len(sequence) > 0 && isVimCountStartKey(sequence[0]) { | |
| 315 | + | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 316 | + | "workspace.%s.actions.%s cannot start with a count digit (1-9): '%s'", | |
| 317 | + | modePath, | |
| 318 | + | action, | |
| 319 | + | canonical, | |
| 320 | + | )) | |
| 321 | + | } | |
| 322 | + | legacyExit := action == "confirm" || action == "cancel" | |
| 323 | + | if !legacyExit { | |
| 324 | + | for _, key := range sequence { | |
| 325 | + | if key.Mods == 0 && (key.Sym == "CR" || key.Sym == "Esc") { | |
| 326 | + | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 327 | + | "workspace.%s.actions.%s cannot bind Enter or Escape outside the modal fallback actions", | |
| 328 | + | modePath, | |
| 329 | + | action, | |
| 330 | + | )) | |
| 331 | + | break | |
| 332 | + | } | |
| 333 | + | } | |
| 334 | + | } | |
| 335 | + | } | |
| 336 | + | } | |
| 325 | 337 | if existingAction, exists := seenSequence[canonical]; exists { | |
| 326 | 338 | validationErrors = append(validationErrors, fmt.Sprintf( | |
| 327 | 339 | "duplicate key binding '%s' found in %s actions '%s' and '%s'", | |
| @@ -337,6 +349,10 @@ func validateModalModeActionsWithKeyCanon( | |||
| 337 | 349 | return validationErrors | |
| 338 | 350 | } | |
| 339 | 351 | ||
| 352 | + | func isVimCountStartKey(key vimkeys.Key) bool { | |
| 353 | + | return key.Mods == 0 && len(key.Sym) == 1 && key.Sym[0] >= '1' && key.Sym[0] <= '9' | |
| 354 | + | } | |
| 355 | + | ||
| 340 | 356 | func canonicalizeVimModeBinding(key string) string { | |
| 341 | 357 | switch strings.ToLower(strings.TrimSpace(key)) { | |
| 342 | 358 | case "enter": | |
| @@ -278,21 +278,7 @@ func validateModalModeActionsWithKeyCanon( | |||||
| 278 | } | 278 | } | ||
| 279 | 279 | ||||
| 280 | if keyCanon == nil { | 280 | if keyCanon == nil { | ||
| 281 | - | seenKeys := make(map[string]string) | 281 | + | keyCanon = func(key string) (string, error) { return key, nil } |
| 282 | - | for action, binding := range actions { | |||
| 283 | - | if len(binding.Keys) == 0 { | |||
| 284 | - | validationErrors = append(validationErrors, fmt.Sprintf("workspace.%s.actions.%s must have at least one key binding", modePath, action)) | |||
| 285 | - | } | |||
| 286 | - | for _, key := range binding.Keys { | |||
| 287 | - | if existingAction, exists := seenKeys[key]; exists { | |||
| 288 | - | validationErrors = append(validationErrors, fmt.Sprintf( | |||
| 289 | - | "duplicate key binding '%s' found in %s actions '%s' and '%s'", key, modePath, existingAction, action, | |||
| 290 | - | )) | |||
| 291 | - | } | |||
| 292 | - | seenKeys[key] = action | |||
| 293 | - | } | |||
| 294 | - | } | |||
| 295 | - | return validationErrors | |||
| 296 | } | 282 | } | ||
| 297 | 283 | ||||
| 298 | actionNames := make([]string, 0, len(actions)) | 284 | actionNames := make([]string, 0, len(actions)) | ||
| @@ -322,6 +308,32 @@ func validateModalModeActionsWithKeyCanon( | |||||
| 322 | )) | 308 | )) | ||
| 323 | continue | 309 | continue | ||
| 324 | } | 310 | } | ||
| 311 | + | if modePath == "vim_mode" { | |||
| 312 | + | sequence, parseErr := vimkeys.ParseBinding(canonical) | |||
| 313 | + | if parseErr == nil { | |||
| 314 | + | if len(sequence) > 0 && isVimCountStartKey(sequence[0]) { | |||
| 315 | + | validationErrors = append(validationErrors, fmt.Sprintf( | |||
| 316 | + | "workspace.%s.actions.%s cannot start with a count digit (1-9): '%s'", | |||
| 317 | + | modePath, | |||
| 318 | + | action, | |||
| 319 | + | canonical, | |||
| 320 | + | )) | |||
| 321 | + | } | |||
| 322 | + | legacyExit := action == "confirm" || action == "cancel" | |||
| 323 | + | if !legacyExit { | |||
| 324 | + | for _, key := range sequence { | |||
| 325 | + | if key.Mods == 0 && (key.Sym == "CR" || key.Sym == "Esc") { | |||
| 326 | + | validationErrors = append(validationErrors, fmt.Sprintf( | |||
| 327 | + | "workspace.%s.actions.%s cannot bind Enter or Escape outside the modal fallback actions", | |||
| 328 | + | modePath, | |||
| 329 | + | action, | |||
| 330 | + | )) | |||
| 331 | + | break | |||
| 332 | + | } | |||
| 333 | + | } | |||
| 334 | + | } | |||
| 335 | + | } | |||
| 336 | + | } | |||
| 325 | if existingAction, exists := seenSequence[canonical]; exists { | 337 | if existingAction, exists := seenSequence[canonical]; exists { | ||
| 326 | validationErrors = append(validationErrors, fmt.Sprintf( | 338 | validationErrors = append(validationErrors, fmt.Sprintf( | ||
| 327 | "duplicate key binding '%s' found in %s actions '%s' and '%s'", | 339 | "duplicate key binding '%s' found in %s actions '%s' and '%s'", | ||
| @@ -337,6 +349,10 @@ func validateModalModeActionsWithKeyCanon( | |||||
| 337 | return validationErrors | 349 | return validationErrors | ||
| 338 | } | 350 | } | ||
| 339 | 351 | ||||
| 352 | + | func isVimCountStartKey(key vimkeys.Key) bool { | |||
| 353 | + | return key.Mods == 0 && len(key.Sym) == 1 && key.Sym[0] >= '1' && key.Sym[0] <= '9' | |||
| 354 | + | } | |||
| 355 | + | ||||
| 340 | func canonicalizeVimModeBinding(key string) string { | 356 | func canonicalizeVimModeBinding(key string) string { | ||
| 341 | switch strings.ToLower(strings.TrimSpace(key)) { | 357 | switch strings.ToLower(strings.TrimSpace(key)) { | ||
| 342 | case "enter": | 358 | case "enter": | ||
M internal/infrastructure/desktop/browser_launch_relay.go +12 -2
| @@ -46,7 +46,10 @@ type browserLaunchRequest struct { | |||
| 46 | 46 | ||
| 47 | 47 | type browserLaunchAction string | |
| 48 | 48 | ||
| 49 | - | const browserLaunchActionOpenFreshWindow browserLaunchAction = "open-fresh-window" | |
| 49 | + | const ( | |
| 50 | + | browserLaunchActionOpenExternalURL browserLaunchAction = "open-external-url" | |
| 51 | + | browserLaunchActionOpenFreshWindow browserLaunchAction = "open-fresh-window" | |
| 52 | + | ) | |
| 50 | 53 | ||
| 51 | 54 | type browserLaunchResponse struct { | |
| 52 | 55 | RequestID string `json:"request_id,omitempty"` | |
| @@ -369,10 +372,17 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||
| 369 | 372 | Msg("browser launch relay calling browser URL opener") | |
| 370 | 373 | var err error | |
| 371 | 374 | switch request.Action { | |
| 375 | + | case "", browserLaunchActionOpenExternalURL: | |
| 376 | + | err = opener.OpenExternalURL(ctx, request.URL) | |
| 372 | 377 | case browserLaunchActionOpenFreshWindow: | |
| 373 | 378 | err = opener.OpenFreshWindow(ctx, request.URL) | |
| 374 | 379 | default: | |
| 375 | - | err = opener.OpenExternalURL(ctx, request.URL) | |
| 380 | + | log.Warn(). | |
| 381 | + | Str("request_id", requestID). | |
| 382 | + | Str("url_host", safeURLHost(request.URL)). | |
| 383 | + | Str("action", string(request.Action)). | |
| 384 | + | Msg("browser launch relay rejected unknown action") | |
| 385 | + | return | |
| 376 | 386 | } | |
| 377 | 387 | if err != nil { | |
| 378 | 388 | log.Warn().Err(err). | |
| @@ -46,7 +46,10 @@ type browserLaunchRequest struct { | |||||
| 46 | 46 | ||||
| 47 | type browserLaunchAction string | 47 | type browserLaunchAction string | ||
| 48 | 48 | ||||
| 49 | - | const browserLaunchActionOpenFreshWindow browserLaunchAction = "open-fresh-window" | 49 | + | const ( |
| 50 | + | browserLaunchActionOpenExternalURL browserLaunchAction = "open-external-url" | |||
| 51 | + | browserLaunchActionOpenFreshWindow browserLaunchAction = "open-fresh-window" | |||
| 52 | + | ) | |||
| 50 | 53 | ||||
| 51 | type browserLaunchResponse struct { | 54 | type browserLaunchResponse struct { | ||
| 52 | RequestID string `json:"request_id,omitempty"` | 55 | RequestID string `json:"request_id,omitempty"` | ||
| @@ -369,10 +372,17 @@ func (*browserLaunchRelayListener) handleConnection(ctx context.Context, conn *n | |||||
| 369 | Msg("browser launch relay calling browser URL opener") | 372 | Msg("browser launch relay calling browser URL opener") | ||
| 370 | var err error | 373 | var err error | ||
| 371 | switch request.Action { | 374 | switch request.Action { | ||
| 375 | + | case "", browserLaunchActionOpenExternalURL: | |||
| 376 | + | err = opener.OpenExternalURL(ctx, request.URL) | |||
| 372 | case browserLaunchActionOpenFreshWindow: | 377 | case browserLaunchActionOpenFreshWindow: | ||
| 373 | err = opener.OpenFreshWindow(ctx, request.URL) | 378 | err = opener.OpenFreshWindow(ctx, request.URL) | ||
| 374 | default: | 379 | default: | ||
| 375 | - | err = opener.OpenExternalURL(ctx, request.URL) | 380 | + | log.Warn(). |
| 381 | + | Str("request_id", requestID). | |||
| 382 | + | Str("url_host", safeURLHost(request.URL)). | |||
| 383 | + | Str("action", string(request.Action)). | |||
| 384 | + | Msg("browser launch relay rejected unknown action") | |||
| 385 | + | return | |||
| 376 | } | 386 | } | ||
| 377 | if err != nil { | 387 | if err != nil { | ||
| 378 | log.Warn().Err(err). | 388 | log.Warn().Err(err). | ||
M internal/infrastructure/webkit/injector.go +25 -14
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "sort" | |
| 7 | + | "strconv" | |
| 7 | 8 | "strings" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/dumber/internal/application/port" | |
| @@ -179,9 +180,8 @@ const explicitCopyScript = `(function() { | |||
| 179 | 180 | } | |
| 180 | 181 | })();` | |
| 181 | 182 | ||
| 182 | - | // accentDetectionScript is built at init from entity.AccentMap so the JS | |
| 183 | + | // accentDetectionScriptKeys is built at init from entity.AccentMap so the JS | |
| 183 | 184 | // filter stays in sync with the Go-side accent table. | |
| 184 | - | var accentDetectionScript string | |
| 185 | 185 | var accentDetectionScriptKeys string | |
| 186 | 186 | ||
| 187 | 187 | func buildExplicitCopyScript() string { | |
| @@ -198,9 +198,9 @@ func buildAccentDetectionScript(token string) string { | |||
| 198 | 198 | ||
| 199 | 199 | function postEditableFocus(editable) { | |
| 200 | 200 | if (lastEditableFocusState === editable) return; | |
| 201 | - | lastEditableFocusState = editable; | |
| 202 | 201 | if (!editableFocusToken) return; | |
| 203 | 202 | if (!(window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.dumber)) return; | |
| 203 | + | lastEditableFocusState = editable; | |
| 204 | 204 | window.webkit.messageHandlers.dumber.postMessage({ | |
| 205 | 205 | type: 'editable_focus_changed', | |
| 206 | 206 | payload: { editable: editable, token: editableFocusToken } | |
| @@ -213,10 +213,13 @@ func buildAccentDetectionScript(token string) string { | |||
| 213 | 213 | if (el.isContentEditable) return true; | |
| 214 | 214 | const editableAncestor = el.closest('[contenteditable]'); | |
| 215 | 215 | if (editableAncestor && editableAncestor.isContentEditable) return true; | |
| 216 | - | if (el.closest('textarea')) return true; | |
| 216 | + | const textarea = el.closest('textarea'); | |
| 217 | + | if (textarea) return !textarea.matches(':disabled') && !textarea.readOnly; | |
| 217 | 218 | const input = el.closest('input'); | |
| 218 | 219 | if (!input) return false; | |
| 219 | - | return !/^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | |
| 220 | + | const disabled = input.matches(':disabled'); | |
| 221 | + | const unsupported = /^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | |
| 222 | + | return !disabled && !input.readOnly && !unsupported; | |
| 220 | 223 | } | |
| 221 | 224 | ||
| 222 | 225 | document.addEventListener('keydown', function(e) { | |
| @@ -243,6 +246,11 @@ func buildAccentDetectionScript(token string) string { | |||
| 243 | 246 | } | |
| 244 | 247 | }, true); | |
| 245 | 248 | ||
| 249 | + | function canReportNegativeFocus(target) { | |
| 250 | + | if (window.top === window) return true; | |
| 251 | + | return lastEditableFocusState === true && window.__dumber_lastEditableEl === target; | |
| 252 | + | } | |
| 253 | + | ||
| 246 | 254 | document.addEventListener('focusin', function(e) { | |
| 247 | 255 | if (e && e.isTrusted === false) return; | |
| 248 | 256 | if (isEditableTarget(e.target)) { | |
| @@ -250,14 +258,16 @@ func buildAccentDetectionScript(token string) string { | |||
| 250 | 258 | postEditableFocus(true); | |
| 251 | 259 | return; | |
| 252 | 260 | } | |
| 253 | - | postEditableFocus(false); | |
| 261 | + | if (canReportNegativeFocus(window.__dumber_lastEditableEl)) { | |
| 262 | + | postEditableFocus(false); | |
| 263 | + | } | |
| 254 | 264 | }, true); | |
| 255 | 265 | ||
| 256 | 266 | document.addEventListener('focusout', function(e) { | |
| 257 | 267 | if (e && e.isTrusted === false) return; | |
| 258 | 268 | if (!isEditableTarget(e.target)) return; | |
| 259 | 269 | setTimeout(function() { | |
| 260 | - | if (!isEditableTarget(document.activeElement)) { | |
| 270 | + | if (!isEditableTarget(document.activeElement) && canReportNegativeFocus(e.target)) { | |
| 261 | 271 | postEditableFocus(false); | |
| 262 | 272 | } | |
| 263 | 273 | }, 0); | |
| @@ -266,7 +276,7 @@ func buildAccentDetectionScript(token string) string { | |||
| 266 | 276 | if (isEditableTarget(document.activeElement)) { | |
| 267 | 277 | window.__dumber_lastEditableEl = document.activeElement; | |
| 268 | 278 | postEditableFocus(true); | |
| 269 | - | } else { | |
| 279 | + | } else if (window.top === window) { | |
| 270 | 280 | postEditableFocus(false); | |
| 271 | 281 | } | |
| 272 | 282 | })();`, accentDetectionScriptKeys, token) | |
| @@ -280,7 +290,6 @@ func init() { | |||
| 280 | 290 | } | |
| 281 | 291 | sort.Strings(keys) | |
| 282 | 292 | accentDetectionScriptKeys = strings.Join(keys, ",") | |
| 283 | - | accentDetectionScript = buildAccentDetectionScript("") | |
| 284 | 293 | } | |
| 285 | 294 | ||
| 286 | 295 | // accentDetectionInjectionMode controls which frames receive the accent detection script. | |
| @@ -490,12 +499,14 @@ func (ci *ContentInjector) InjectScripts(ctx context.Context, ucm *webkit.UserCo | |||
| 490 | 499 | ||
| 491 | 500 | // 8. Inject accent key detection for all pages and all frames (unconditional). | |
| 492 | 501 | // JS only reports keydown/keyup events; Go handles timing and picker display. | |
| 493 | - | accentScript := accentDetectionScript | |
| 494 | - | if wv := LookupWebView(webviewID); wv != nil { | |
| 495 | - | if token := wv.EditableFocusBridgeToken(); token != "" { | |
| 496 | - | accentScript = buildAccentDetectionScript(token) | |
| 497 | - | } | |
| 502 | + | wv := LookupWebView(webviewID) | |
| 503 | + | if wv == nil || wv.EditableFocusBridgeToken() == "" { | |
| 504 | + | log.Error(). | |
| 505 | + | Str("webview_id", strconv.FormatUint(uint64(webviewID), 10)). | |
| 506 | + | Msg("skipping accent detection script without editable-focus bridge token") | |
| 507 | + | return | |
| 498 | 508 | } | |
| 509 | + | accentScript := buildAccentDetectionScript(wv.EditableFocusBridgeToken()) | |
| 499 | 510 | addScript( | |
| 500 | 511 | webkit.NewUserScript( | |
| 501 | 512 | accentScript, | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "sort" | 6 | "sort" | ||
| 7 | + | "strconv" | |||
| 7 | "strings" | 8 | "strings" | ||
| 8 | 9 | ||||
| 9 | "github.com/bnema/dumber/internal/application/port" | 10 | "github.com/bnema/dumber/internal/application/port" | ||
| @@ -179,9 +180,8 @@ const explicitCopyScript = `(function() { | |||||
| 179 | } | 180 | } | ||
| 180 | })();` | 181 | })();` | ||
| 181 | 182 | ||||
| 182 | - | // accentDetectionScript is built at init from entity.AccentMap so the JS | 183 | + | // accentDetectionScriptKeys is built at init from entity.AccentMap so the JS |
| 183 | // filter stays in sync with the Go-side accent table. | 184 | // filter stays in sync with the Go-side accent table. | ||
| 184 | - | var accentDetectionScript string | |||
| 185 | var accentDetectionScriptKeys string | 185 | var accentDetectionScriptKeys string | ||
| 186 | 186 | ||||
| 187 | func buildExplicitCopyScript() string { | 187 | func buildExplicitCopyScript() string { | ||
| @@ -198,9 +198,9 @@ func buildAccentDetectionScript(token string) string { | |||||
| 198 | 198 | ||||
| 199 | function postEditableFocus(editable) { | 199 | function postEditableFocus(editable) { | ||
| 200 | if (lastEditableFocusState === editable) return; | 200 | if (lastEditableFocusState === editable) return; | ||
| 201 | - | lastEditableFocusState = editable; | |||
| 202 | if (!editableFocusToken) return; | 201 | if (!editableFocusToken) return; | ||
| 203 | if (!(window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.dumber)) return; | 202 | if (!(window.webkit && window.webkit.messageHandlers && window.webkit.messageHandlers.dumber)) return; | ||
| 203 | + | lastEditableFocusState = editable; | |||
| 204 | window.webkit.messageHandlers.dumber.postMessage({ | 204 | window.webkit.messageHandlers.dumber.postMessage({ | ||
| 205 | type: 'editable_focus_changed', | 205 | type: 'editable_focus_changed', | ||
| 206 | payload: { editable: editable, token: editableFocusToken } | 206 | payload: { editable: editable, token: editableFocusToken } | ||
| @@ -213,10 +213,13 @@ func buildAccentDetectionScript(token string) string { | |||||
| 213 | if (el.isContentEditable) return true; | 213 | if (el.isContentEditable) return true; | ||
| 214 | const editableAncestor = el.closest('[contenteditable]'); | 214 | const editableAncestor = el.closest('[contenteditable]'); | ||
| 215 | if (editableAncestor && editableAncestor.isContentEditable) return true; | 215 | if (editableAncestor && editableAncestor.isContentEditable) return true; | ||
| 216 | - | if (el.closest('textarea')) return true; | 216 | + | const textarea = el.closest('textarea'); |
| 217 | + | if (textarea) return !textarea.matches(':disabled') && !textarea.readOnly; | |||
| 217 | const input = el.closest('input'); | 218 | const input = el.closest('input'); | ||
| 218 | if (!input) return false; | 219 | if (!input) return false; | ||
| 219 | - | return !/^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | 220 | + | const disabled = input.matches(':disabled'); |
| 221 | + | const unsupported = /^(button|checkbox|color|file|hidden|image|radio|range|reset|submit)$/i.test(input.type); | |||
| 222 | + | return !disabled && !input.readOnly && !unsupported; | |||
| 220 | } | 223 | } | ||
| 221 | 224 | ||||
| 222 | document.addEventListener('keydown', function(e) { | 225 | document.addEventListener('keydown', function(e) { | ||
| @@ -243,6 +246,11 @@ func buildAccentDetectionScript(token string) string { | |||||
| 243 | } | 246 | } | ||
| 244 | }, true); | 247 | }, true); | ||
| 245 | 248 | ||||
| 249 | + | function canReportNegativeFocus(target) { | |||
| 250 | + | if (window.top === window) return true; | |||
| 251 | + | return lastEditableFocusState === true && window.__dumber_lastEditableEl === target; | |||
| 252 | + | } | |||
| 253 | + | ||||
| 246 | document.addEventListener('focusin', function(e) { | 254 | document.addEventListener('focusin', function(e) { | ||
| 247 | if (e && e.isTrusted === false) return; | 255 | if (e && e.isTrusted === false) return; | ||
| 248 | if (isEditableTarget(e.target)) { | 256 | if (isEditableTarget(e.target)) { | ||
| @@ -250,14 +258,16 @@ func buildAccentDetectionScript(token string) string { | |||||
| 250 | postEditableFocus(true); | 258 | postEditableFocus(true); | ||
| 251 | return; | 259 | return; | ||
| 252 | } | 260 | } | ||
| 253 | - | postEditableFocus(false); | 261 | + | if (canReportNegativeFocus(window.__dumber_lastEditableEl)) { |
| 262 | + | postEditableFocus(false); | |||
| 263 | + | } | |||
| 254 | }, true); | 264 | }, true); | ||
| 255 | 265 | ||||
| 256 | document.addEventListener('focusout', function(e) { | 266 | document.addEventListener('focusout', function(e) { | ||
| 257 | if (e && e.isTrusted === false) return; | 267 | if (e && e.isTrusted === false) return; | ||
| 258 | if (!isEditableTarget(e.target)) return; | 268 | if (!isEditableTarget(e.target)) return; | ||
| 259 | setTimeout(function() { | 269 | setTimeout(function() { | ||
| 260 | - | if (!isEditableTarget(document.activeElement)) { | 270 | + | if (!isEditableTarget(document.activeElement) && canReportNegativeFocus(e.target)) { |
| 261 | postEditableFocus(false); | 271 | postEditableFocus(false); | ||
| 262 | } | 272 | } | ||
| 263 | }, 0); | 273 | }, 0); | ||
| @@ -266,7 +276,7 @@ func buildAccentDetectionScript(token string) string { | |||||
| 266 | if (isEditableTarget(document.activeElement)) { | 276 | if (isEditableTarget(document.activeElement)) { | ||
| 267 | window.__dumber_lastEditableEl = document.activeElement; | 277 | window.__dumber_lastEditableEl = document.activeElement; | ||
| 268 | postEditableFocus(true); | 278 | postEditableFocus(true); | ||
| 269 | - | } else { | 279 | + | } else if (window.top === window) { |
| 270 | postEditableFocus(false); | 280 | postEditableFocus(false); | ||
| 271 | } | 281 | } | ||
| 272 | })();`, accentDetectionScriptKeys, token) | 282 | })();`, accentDetectionScriptKeys, token) | ||
| @@ -280,7 +290,6 @@ func init() { | |||||
| 280 | } | 290 | } | ||
| 281 | sort.Strings(keys) | 291 | sort.Strings(keys) | ||
| 282 | accentDetectionScriptKeys = strings.Join(keys, ",") | 292 | accentDetectionScriptKeys = strings.Join(keys, ",") | ||
| 283 | - | accentDetectionScript = buildAccentDetectionScript("") | |||
| 284 | } | 293 | } | ||
| 285 | 294 | ||||
| 286 | // accentDetectionInjectionMode controls which frames receive the accent detection script. | 295 | // accentDetectionInjectionMode controls which frames receive the accent detection script. | ||
| @@ -490,12 +499,14 @@ func (ci *ContentInjector) InjectScripts(ctx context.Context, ucm *webkit.UserCo | |||||
| 490 | 499 | ||||
| 491 | // 8. Inject accent key detection for all pages and all frames (unconditional). | 500 | // 8. Inject accent key detection for all pages and all frames (unconditional). | ||
| 492 | // JS only reports keydown/keyup events; Go handles timing and picker display. | 501 | // JS only reports keydown/keyup events; Go handles timing and picker display. | ||
| 493 | - | accentScript := accentDetectionScript | 502 | + | wv := LookupWebView(webviewID) |
| 494 | - | if wv := LookupWebView(webviewID); wv != nil { | 503 | + | if wv == nil || wv.EditableFocusBridgeToken() == "" { |
| 495 | - | if token := wv.EditableFocusBridgeToken(); token != "" { | 504 | + | log.Error(). |
| 496 | - | accentScript = buildAccentDetectionScript(token) | 505 | + | Str("webview_id", strconv.FormatUint(uint64(webviewID), 10)). |
| 497 | - | } | 506 | + | Msg("skipping accent detection script without editable-focus bridge token") |
| 507 | + | return | |||
| 498 | } | 508 | } | ||
| 509 | + | accentScript := buildAccentDetectionScript(wv.EditableFocusBridgeToken()) | |||
| 499 | addScript( | 510 | addScript( | ||
| 500 | webkit.NewUserScript( | 511 | webkit.NewUserScript( | ||
| 501 | accentScript, | 512 | accentScript, | ||
M internal/infrastructure/webkit/injector_accent_test.go +17 -15
| @@ -19,32 +19,34 @@ func TestAccentDetectionScriptInjectionMode(t *testing.T) { | |||
| 19 | 19 | } | |
| 20 | 20 | ||
| 21 | 21 | func TestAccentDetectionScriptTracksLastFocusedEditableElement(t *testing.T) { | |
| 22 | + | script := buildAccentDetectionScript("") | |
| 22 | 23 | assert.Contains(t, | |
| 23 | - | accentDetectionScript, | |
| 24 | + | script, | |
| 24 | 25 | "window.__dumber_lastEditableEl = e.target", | |
| 25 | 26 | "accent detection script must track the last focused editable element", | |
| 26 | 27 | ) | |
| 27 | 28 | assert.Contains(t, | |
| 28 | - | accentDetectionScript, | |
| 29 | + | script, | |
| 29 | 30 | "document.addEventListener('focusin'", | |
| 30 | 31 | "accent detection script must register a focusin listener", | |
| 31 | 32 | ) | |
| 32 | - | assert.Contains(t, accentDetectionScript, "el.isContentEditable") | |
| 33 | - | assert.Contains(t, accentDetectionScript, "closest('[contenteditable]')") | |
| 33 | + | assert.Contains(t, script, "el.isContentEditable") | |
| 34 | + | assert.Contains(t, script, "closest('[contenteditable]')") | |
| 34 | 35 | } | |
| 35 | 36 | ||
| 36 | 37 | func TestAccentDetectionScriptPostsEditableFocusChangedMessages(t *testing.T) { | |
| 37 | - | assert.Contains(t, accentDetectionScript, "editable_focus_changed") | |
| 38 | - | assert.Contains(t, accentDetectionScript, "document.addEventListener('focusout'") | |
| 39 | - | assert.Contains(t, accentDetectionScript, `const editableFocusToken = "";`) | |
| 40 | - | assert.Contains(t, accentDetectionScript, `payload: { editable: editable, token: editableFocusToken }`) | |
| 41 | - | assert.Contains(t, accentDetectionScript, "let lastEditableFocusState = null;") | |
| 42 | - | assert.Contains(t, accentDetectionScript, "if (lastEditableFocusState === editable) return;") | |
| 43 | - | assert.Contains(t, accentDetectionScript, `postEditableFocus(true)`) | |
| 44 | - | assert.Contains(t, accentDetectionScript, `postEditableFocus(false)`) | |
| 45 | - | assert.Contains(t, accentDetectionScript, `e && e.isTrusted === false`) | |
| 46 | - | assert.Contains(t, accentDetectionScript, `document.activeElement`) | |
| 47 | - | assert.Contains(t, accentDetectionScript, `window.__dumber_lastEditableEl = document.activeElement`) | |
| 38 | + | script := buildAccentDetectionScript("") | |
| 39 | + | assert.Contains(t, script, "editable_focus_changed") | |
| 40 | + | assert.Contains(t, script, "document.addEventListener('focusout'") | |
| 41 | + | assert.Contains(t, script, `const editableFocusToken = "";`) | |
| 42 | + | assert.Contains(t, script, `payload: { editable: editable, token: editableFocusToken }`) | |
| 43 | + | assert.Contains(t, script, "let lastEditableFocusState = null;") | |
| 44 | + | assert.Contains(t, script, "if (lastEditableFocusState === editable) return;") | |
| 45 | + | assert.Contains(t, script, `postEditableFocus(true)`) | |
| 46 | + | assert.Contains(t, script, `postEditableFocus(false)`) | |
| 47 | + | assert.Contains(t, script, `e && e.isTrusted === false`) | |
| 48 | + | assert.Contains(t, script, `document.activeElement`) | |
| 49 | + | assert.Contains(t, script, `window.__dumber_lastEditableEl = document.activeElement`) | |
| 48 | 50 | } | |
| 49 | 51 | ||
| 50 | 52 | func TestBuildAccentDetectionScript_EmptyTokenReturnsBeforeHandlerLookup(t *testing.T) { | |
| @@ -19,32 +19,34 @@ func TestAccentDetectionScriptInjectionMode(t *testing.T) { | |||||
| 19 | } | 19 | } | ||
| 20 | 20 | ||||
| 21 | func TestAccentDetectionScriptTracksLastFocusedEditableElement(t *testing.T) { | 21 | func TestAccentDetectionScriptTracksLastFocusedEditableElement(t *testing.T) { | ||
| 22 | + | script := buildAccentDetectionScript("") | |||
| 22 | assert.Contains(t, | 23 | assert.Contains(t, | ||
| 23 | - | accentDetectionScript, | 24 | + | script, |
| 24 | "window.__dumber_lastEditableEl = e.target", | 25 | "window.__dumber_lastEditableEl = e.target", | ||
| 25 | "accent detection script must track the last focused editable element", | 26 | "accent detection script must track the last focused editable element", | ||
| 26 | ) | 27 | ) | ||
| 27 | assert.Contains(t, | 28 | assert.Contains(t, | ||
| 28 | - | accentDetectionScript, | 29 | + | script, |
| 29 | "document.addEventListener('focusin'", | 30 | "document.addEventListener('focusin'", | ||
| 30 | "accent detection script must register a focusin listener", | 31 | "accent detection script must register a focusin listener", | ||
| 31 | ) | 32 | ) | ||
| 32 | - | assert.Contains(t, accentDetectionScript, "el.isContentEditable") | 33 | + | assert.Contains(t, script, "el.isContentEditable") |
| 33 | - | assert.Contains(t, accentDetectionScript, "closest('[contenteditable]')") | 34 | + | assert.Contains(t, script, "closest('[contenteditable]')") |
| 34 | } | 35 | } | ||
| 35 | 36 | ||||
| 36 | func TestAccentDetectionScriptPostsEditableFocusChangedMessages(t *testing.T) { | 37 | func TestAccentDetectionScriptPostsEditableFocusChangedMessages(t *testing.T) { | ||
| 37 | - | assert.Contains(t, accentDetectionScript, "editable_focus_changed") | 38 | + | script := buildAccentDetectionScript("") |
| 38 | - | assert.Contains(t, accentDetectionScript, "document.addEventListener('focusout'") | 39 | + | assert.Contains(t, script, "editable_focus_changed") |
| 39 | - | assert.Contains(t, accentDetectionScript, `const editableFocusToken = "";`) | 40 | + | assert.Contains(t, script, "document.addEventListener('focusout'") |
| 40 | - | assert.Contains(t, accentDetectionScript, `payload: { editable: editable, token: editableFocusToken }`) | 41 | + | assert.Contains(t, script, `const editableFocusToken = "";`) |
| 41 | - | assert.Contains(t, accentDetectionScript, "let lastEditableFocusState = null;") | 42 | + | assert.Contains(t, script, `payload: { editable: editable, token: editableFocusToken }`) |
| 42 | - | assert.Contains(t, accentDetectionScript, "if (lastEditableFocusState === editable) return;") | 43 | + | assert.Contains(t, script, "let lastEditableFocusState = null;") |
| 43 | - | assert.Contains(t, accentDetectionScript, `postEditableFocus(true)`) | 44 | + | assert.Contains(t, script, "if (lastEditableFocusState === editable) return;") |
| 44 | - | assert.Contains(t, accentDetectionScript, `postEditableFocus(false)`) | 45 | + | assert.Contains(t, script, `postEditableFocus(true)`) |
| 45 | - | assert.Contains(t, accentDetectionScript, `e && e.isTrusted === false`) | 46 | + | assert.Contains(t, script, `postEditableFocus(false)`) |
| 46 | - | assert.Contains(t, accentDetectionScript, `document.activeElement`) | 47 | + | assert.Contains(t, script, `e && e.isTrusted === false`) |
| 47 | - | assert.Contains(t, accentDetectionScript, `window.__dumber_lastEditableEl = document.activeElement`) | 48 | + | assert.Contains(t, script, `document.activeElement`) |
| 49 | + | assert.Contains(t, script, `window.__dumber_lastEditableEl = document.activeElement`) | |||
| 48 | } | 50 | } | ||
| 49 | 51 | ||||
| 50 | func TestBuildAccentDetectionScript_EmptyTokenReturnsBeforeHandlerLookup(t *testing.T) { | 52 | func TestBuildAccentDetectionScript_EmptyTokenReturnsBeforeHandlerLookup(t *testing.T) { | ||
M internal/infrastructure/webkit/messaging.go +2 -1
| @@ -298,14 +298,15 @@ func (r *MessageRouter) handleAllowlistedBridgeMessage(senderWV *WebView, msg Me | |||
| 298 | 298 | if msg.Type != "editable_focus_changed" { | |
| 299 | 299 | return false | |
| 300 | 300 | } | |
| 301 | + | log := logging.FromContext(r.baseContext()) | |
| 301 | 302 | if senderWV == nil { | |
| 303 | + | log.Warn().Str("type", msg.Type).Msg("dropping allowlisted bridge message without sender WebView") | |
| 302 | 304 | return true | |
| 303 | 305 | } | |
| 304 | 306 | var payload struct { | |
| 305 | 307 | Editable bool `json:"editable"` | |
| 306 | 308 | Token string `json:"token"` | |
| 307 | 309 | } | |
| 308 | - | log := logging.FromContext(r.baseContext()) | |
| 309 | 310 | if len(msg.Payload) != 0 { | |
| 310 | 311 | if err := json.Unmarshal(msg.Payload, &payload); err != nil { | |
| 311 | 312 | log.Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") | |
| @@ -298,14 +298,15 @@ func (r *MessageRouter) handleAllowlistedBridgeMessage(senderWV *WebView, msg Me | |||||
| 298 | if msg.Type != "editable_focus_changed" { | 298 | if msg.Type != "editable_focus_changed" { | ||
| 299 | return false | 299 | return false | ||
| 300 | } | 300 | } | ||
| 301 | + | log := logging.FromContext(r.baseContext()) | |||
| 301 | if senderWV == nil { | 302 | if senderWV == nil { | ||
| 303 | + | log.Warn().Str("type", msg.Type).Msg("dropping allowlisted bridge message without sender WebView") | |||
| 302 | return true | 304 | return true | ||
| 303 | } | 305 | } | ||
| 304 | var payload struct { | 306 | var payload struct { | ||
| 305 | Editable bool `json:"editable"` | 307 | Editable bool `json:"editable"` | ||
| 306 | Token string `json:"token"` | 308 | Token string `json:"token"` | ||
| 307 | } | 309 | } | ||
| 308 | - | log := logging.FromContext(r.baseContext()) | |||
| 309 | if len(msg.Payload) != 0 { | 310 | if len(msg.Payload) != 0 { | ||
| 310 | if err := json.Unmarshal(msg.Payload, &payload); err != nil { | 311 | if err := json.Unmarshal(msg.Payload, &payload); err != nil { | ||
| 311 | log.Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") | 312 | log.Warn().Err(err).Str("type", msg.Type).Msg("failed to decode allowlisted bridge payload") | ||
M internal/infrastructure/webkit/webview.go +9 -0
| @@ -2436,6 +2436,15 @@ func (wv *WebView) AttachFrontend(ctx context.Context, injector *ContentInjector | |||
| 2436 | 2436 | // shared directional DOM-target resolution once its browser is ready. | |
| 2437 | 2437 | // Implements port.PageScrollable. | |
| 2438 | 2438 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | |
| 2439 | + | if err := ctx.Err(); err != nil { | |
| 2440 | + | return err | |
| 2441 | + | } | |
| 2442 | + | if wv == nil { | |
| 2443 | + | return fmt.Errorf("webkit: webview is nil") | |
| 2444 | + | } | |
| 2445 | + | if err := request.Validate(); err != nil { | |
| 2446 | + | return err | |
| 2447 | + | } | |
| 2439 | 2448 | if wv.destroyed.Load() { | |
| 2440 | 2449 | return fmt.Errorf("webview %d is destroyed", wv.id) | |
| 2441 | 2450 | } | |
| @@ -2436,6 +2436,15 @@ func (wv *WebView) AttachFrontend(ctx context.Context, injector *ContentInjector | |||||
| 2436 | // shared directional DOM-target resolution once its browser is ready. | 2436 | // shared directional DOM-target resolution once its browser is ready. | ||
| 2437 | // Implements port.PageScrollable. | 2437 | // Implements port.PageScrollable. | ||
| 2438 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | 2438 | func (wv *WebView) ScrollPage(ctx context.Context, request port.PageScrollRequest) error { | ||
| 2439 | + | if err := ctx.Err(); err != nil { | |||
| 2440 | + | return err | |||
| 2441 | + | } | |||
| 2442 | + | if wv == nil { | |||
| 2443 | + | return fmt.Errorf("webkit: webview is nil") | |||
| 2444 | + | } | |||
| 2445 | + | if err := request.Validate(); err != nil { | |||
| 2446 | + | return err | |||
| 2447 | + | } | |||
| 2439 | if wv.destroyed.Load() { | 2448 | if wv.destroyed.Load() { | ||
| 2440 | return fmt.Errorf("webview %d is destroyed", wv.id) | 2449 | return fmt.Errorf("webview %d is destroyed", wv.id) | ||
| 2441 | } | 2450 | } | ||
M internal/infrastructure/webkit/webview_runjavascript_test.go +6 -1
| @@ -132,7 +132,7 @@ func TestScrollPage_UsesFallbackDeltasAndRunsJavaScript(t *testing.T) { | |||
| 132 | 132 | } | |
| 133 | 133 | ||
| 134 | 134 | err := wv.ScrollPage(context.Background(), port.PageScrollRequest{ | |
| 135 | - | Command: port.PageScrollCommand(99), | |
| 135 | + | Command: port.PageScrollCommandLeft, | |
| 136 | 136 | FallbackDX: -12, | |
| 137 | 137 | FallbackDY: 80, | |
| 138 | 138 | }) | |
| @@ -168,11 +168,14 @@ func TestScrollPage_VariousRequestsForwardFallbackDeltas(t *testing.T) { | |||
| 168 | 168 | for _, tt := range tests { | |
| 169 | 169 | t.Run(tt.name, func(t *testing.T) { | |
| 170 | 170 | var gotDX, gotDY int | |
| 171 | + | buildCalls, runCalls := 0, 0 | |
| 171 | 172 | buildPageScrollFallbackJS = func(dx, dy int) string { | |
| 173 | + | buildCalls++ | |
| 172 | 174 | gotDX, gotDY = dx, dy | |
| 173 | 175 | return "ok" | |
| 174 | 176 | } | |
| 175 | 177 | runPageScrollFallbackJS = func(_ context.Context, _ *WebView, script string) { | |
| 178 | + | runCalls++ | |
| 176 | 179 | assert.Equal(t, "ok", script) | |
| 177 | 180 | } | |
| 178 | 181 | ||
| @@ -182,6 +185,8 @@ func TestScrollPage_VariousRequestsForwardFallbackDeltas(t *testing.T) { | |||
| 182 | 185 | } | |
| 183 | 186 | err := wv.ScrollPage(context.Background(), tt.req) | |
| 184 | 187 | require.NoError(t, err) | |
| 188 | + | assert.Equal(t, 1, buildCalls) | |
| 189 | + | assert.Equal(t, 1, runCalls) | |
| 185 | 190 | assert.Equal(t, tt.req.FallbackDX, gotDX) | |
| 186 | 191 | assert.Equal(t, tt.req.FallbackDY, gotDY) | |
| 187 | 192 | }) | |
| @@ -132,7 +132,7 @@ func TestScrollPage_UsesFallbackDeltasAndRunsJavaScript(t *testing.T) { | |||||
| 132 | } | 132 | } | ||
| 133 | 133 | ||||
| 134 | err := wv.ScrollPage(context.Background(), port.PageScrollRequest{ | 134 | err := wv.ScrollPage(context.Background(), port.PageScrollRequest{ | ||
| 135 | - | Command: port.PageScrollCommand(99), | 135 | + | Command: port.PageScrollCommandLeft, |
| 136 | FallbackDX: -12, | 136 | FallbackDX: -12, | ||
| 137 | FallbackDY: 80, | 137 | FallbackDY: 80, | ||
| 138 | }) | 138 | }) | ||
| @@ -168,11 +168,14 @@ func TestScrollPage_VariousRequestsForwardFallbackDeltas(t *testing.T) { | |||||
| 168 | for _, tt := range tests { | 168 | for _, tt := range tests { | ||
| 169 | t.Run(tt.name, func(t *testing.T) { | 169 | t.Run(tt.name, func(t *testing.T) { | ||
| 170 | var gotDX, gotDY int | 170 | var gotDX, gotDY int | ||
| 171 | + | buildCalls, runCalls := 0, 0 | |||
| 171 | buildPageScrollFallbackJS = func(dx, dy int) string { | 172 | buildPageScrollFallbackJS = func(dx, dy int) string { | ||
| 173 | + | buildCalls++ | |||
| 172 | gotDX, gotDY = dx, dy | 174 | gotDX, gotDY = dx, dy | ||
| 173 | return "ok" | 175 | return "ok" | ||
| 174 | } | 176 | } | ||
| 175 | runPageScrollFallbackJS = func(_ context.Context, _ *WebView, script string) { | 177 | runPageScrollFallbackJS = func(_ context.Context, _ *WebView, script string) { | ||
| 178 | + | runCalls++ | |||
| 176 | assert.Equal(t, "ok", script) | 179 | assert.Equal(t, "ok", script) | ||
| 177 | } | 180 | } | ||
| 178 | 181 | ||||
| @@ -182,6 +185,8 @@ func TestScrollPage_VariousRequestsForwardFallbackDeltas(t *testing.T) { | |||||
| 182 | } | 185 | } | ||
| 183 | err := wv.ScrollPage(context.Background(), tt.req) | 186 | err := wv.ScrollPage(context.Background(), tt.req) | ||
| 184 | require.NoError(t, err) | 187 | require.NoError(t, err) | ||
| 188 | + | assert.Equal(t, 1, buildCalls) | |||
| 189 | + | assert.Equal(t, 1, runCalls) | |||
| 185 | assert.Equal(t, tt.req.FallbackDX, gotDX) | 190 | assert.Equal(t, tt.req.FallbackDX, gotDX) | ||
| 186 | assert.Equal(t, tt.req.FallbackDY, gotDY) | 191 | assert.Equal(t, tt.req.FallbackDY, gotDY) | ||
| 187 | }) | 192 | }) | ||
M internal/ui/app.go +2 -10
| @@ -3920,16 +3920,8 @@ func (a *App) handlePageEditableFocusChanged(ctx context.Context, paneID entity. | |||
| 3920 | 3920 | a.applyVimModePolicyTransition(ctx, bw, transition) | |
| 3921 | 3921 | } | |
| 3922 | 3922 | ||
| 3923 | - | func (a *App) shouldBypassVimModeActivation(bw *browserWindow) bool { | |
| 3924 | - | ws := a.activeWorkspaceForBrowserWindow(bw) | |
| 3925 | - | if ws == nil { | |
| 3926 | - | return false | |
| 3927 | - | } | |
| 3928 | - | transition := a.vimModePolicy().Evaluate(usecase.VimModePolicyInput{ | |
| 3929 | - | Trigger: usecase.VimModePolicyTriggerActivationAttempt, | |
| 3930 | - | PageEditableFocused: a.pageEditableFocused(ws.ActivePaneID), | |
| 3931 | - | }) | |
| 3932 | - | return transition == usecase.VimModePolicyTransitionBlockActivation | |
| 3923 | + | func (*App) shouldBypassVimModeActivation(_ *browserWindow) bool { | |
| 3924 | + | return false | |
| 3933 | 3925 | } | |
| 3934 | 3926 | ||
| 3935 | 3927 | // handleModeChange is called when the input mode changes for a specific browser window. | |
| @@ -3920,16 +3920,8 @@ func (a *App) handlePageEditableFocusChanged(ctx context.Context, paneID entity. | |||||
| 3920 | a.applyVimModePolicyTransition(ctx, bw, transition) | 3920 | a.applyVimModePolicyTransition(ctx, bw, transition) | ||
| 3921 | } | 3921 | } | ||
| 3922 | 3922 | ||||
| 3923 | - | func (a *App) shouldBypassVimModeActivation(bw *browserWindow) bool { | 3923 | + | func (*App) shouldBypassVimModeActivation(_ *browserWindow) bool { |
| 3924 | - | ws := a.activeWorkspaceForBrowserWindow(bw) | 3924 | + | return false |
| 3925 | - | if ws == nil { | |||
| 3926 | - | return false | |||
| 3927 | - | } | |||
| 3928 | - | transition := a.vimModePolicy().Evaluate(usecase.VimModePolicyInput{ | |||
| 3929 | - | Trigger: usecase.VimModePolicyTriggerActivationAttempt, | |||
| 3930 | - | PageEditableFocused: a.pageEditableFocused(ws.ActivePaneID), | |||
| 3931 | - | }) | |||
| 3932 | - | return transition == usecase.VimModePolicyTransitionBlockActivation | |||
| 3933 | } | 3925 | } | ||
| 3934 | 3926 | ||||
| 3935 | // handleModeChange is called when the input mode changes for a specific browser window. | 3927 | // handleModeChange is called when the input mode changes for a specific browser window. | ||
M internal/ui/browser_window.go +30 -78
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "github.com/bnema/dumber/internal/application/port" | |
| 10 | 10 | "github.com/bnema/dumber/internal/application/usecase" | |
| 11 | 11 | "github.com/bnema/dumber/internal/domain/entity" | |
| 12 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |
| 12 | 13 | "github.com/bnema/dumber/internal/logging" | |
| 13 | 14 | "github.com/bnema/dumber/internal/shared/syncdispatch" | |
| 14 | 15 | "github.com/bnema/dumber/internal/ui/component" | |
| @@ -548,11 +549,9 @@ func (a *App) deterministicBrowserWindowFallback() *browserWindow { | |||
| 548 | 549 | return a.browserWindows[ids[0]] | |
| 549 | 550 | } | |
| 550 | 551 | ||
| 551 | - | func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |
| 552 | + | func (a *App) dispatchWindowURLWork(ctx context.Context, label, url string, work func(context.Context) error) error { | |
| 552 | 553 | log := logging.FromContext(ctx) | |
| 553 | - | log.Debug(). | |
| 554 | - | Str("url_host", logging.SafeURLHost(url)). | |
| 555 | - | Msg("ui: open external URL dispatch requested") | |
| 554 | + | log.Debug().Str("url_host", logging.SafeURLHost(url)).Str("dispatch_label", label).Msg("ui: window URL dispatch requested") | |
| 556 | 555 | ||
| 557 | 556 | dispatch := a.dispatchOnMainThread | |
| 558 | 557 | if dispatch == nil { | |
| @@ -564,111 +563,64 @@ func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |||
| 564 | 563 | } | |
| 565 | 564 | } | |
| 566 | 565 | ||
| 567 | - | var openErr error | |
| 568 | - | var windowCountBefore int | |
| 569 | - | var windowCountAfter int | |
| 570 | - | var hasTabCoord bool | |
| 571 | - | var hasTabsUC bool | |
| 572 | - | result := dispatch("ui.open_external_url", func() { | |
| 566 | + | var workErr error | |
| 567 | + | var windowCountBefore, windowCountAfter int | |
| 568 | + | var hasTabCoord, hasTabsUC bool | |
| 569 | + | result := dispatch(label, func() { | |
| 573 | 570 | windowCountBefore = len(a.browserWindows) | |
| 574 | 571 | hasTabCoord = a.tabCoord != nil | |
| 575 | 572 | hasTabsUC = a.tabsUC != nil | |
| 576 | - | cfg := a.runtimeConfigSnapshot() | |
| 577 | 573 | log.Debug(). | |
| 578 | 574 | Str("url_host", logging.SafeURLHost(url)). | |
| 575 | + | Str("dispatch_label", label). | |
| 579 | 576 | Int("window_count_before", windowCountBefore). | |
| 580 | 577 | Bool("has_tab_coord", hasTabCoord). | |
| 581 | 578 | Bool("has_tabs_uc", hasTabsUC). | |
| 582 | - | Msg("ui: open external URL main-thread work started") | |
| 583 | - | openErr = a.openExternalURLOnMainThread(ctx, url, cfg) | |
| 579 | + | Msg("ui: window URL main-thread work started") | |
| 580 | + | workErr = work(ctx) | |
| 584 | 581 | windowCountAfter = len(a.browserWindows) | |
| 585 | 582 | }) | |
| 586 | 583 | if !result.Completed() { | |
| 587 | 584 | log.Warn(). | |
| 588 | 585 | Str("url_host", logging.SafeURLHost(url)). | |
| 586 | + | Str("dispatch_label", label). | |
| 589 | 587 | Dur("elapsed", result.Elapsed). | |
| 590 | 588 | Str("dispatch_status", string(result.Status)). | |
| 591 | - | Msg("ui: open external URL skipped after main-thread dispatch did not complete") | |
| 589 | + | Msg("ui: window URL skipped after main-thread dispatch did not complete") | |
| 592 | 590 | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |
| 593 | 591 | } | |
| 594 | - | if openErr != nil { | |
| 595 | - | log.Warn().Err(openErr). | |
| 592 | + | if workErr != nil { | |
| 593 | + | log.Warn().Err(workErr). | |
| 596 | 594 | Str("url_host", logging.SafeURLHost(url)). | |
| 595 | + | Str("dispatch_label", label). | |
| 597 | 596 | Dur("elapsed", result.Elapsed). | |
| 598 | 597 | Str("dispatch_status", string(result.Status)). | |
| 599 | 598 | Int("window_count_after", windowCountAfter). | |
| 600 | - | Msg("ui: open external URL failed") | |
| 601 | - | return openErr | |
| 599 | + | Msg("ui: window URL operation failed") | |
| 600 | + | return workErr | |
| 602 | 601 | } | |
| 603 | - | ||
| 604 | 602 | log.Debug(). | |
| 605 | 603 | Str("url_host", logging.SafeURLHost(url)). | |
| 604 | + | Str("dispatch_label", label). | |
| 606 | 605 | Dur("elapsed", result.Elapsed). | |
| 607 | 606 | Str("dispatch_status", string(result.Status)). | |
| 608 | 607 | Int("window_count_after", windowCountAfter). | |
| 609 | - | Msg("ui: open external URL completed") | |
| 608 | + | Msg("ui: window URL operation completed") | |
| 610 | 609 | return nil | |
| 611 | 610 | } | |
| 612 | 611 | ||
| 613 | - | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | |
| 614 | - | log := logging.FromContext(ctx) | |
| 615 | - | log.Debug(). | |
| 616 | - | Str("url_host", logging.SafeURLHost(url)). | |
| 617 | - | Msg("ui: open fresh window dispatch requested") | |
| 618 | - | ||
| 619 | - | dispatch := a.dispatchOnMainThread | |
| 620 | - | if dispatch == nil { | |
| 621 | - | dispatch = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |
| 622 | - | if fn != nil { | |
| 623 | - | fn() | |
| 624 | - | } | |
| 625 | - | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |
| 626 | - | } | |
| 627 | - | } | |
| 628 | - | ||
| 629 | - | var openErr error | |
| 630 | - | var windowCountBefore int | |
| 631 | - | var windowCountAfter int | |
| 632 | - | var hasTabCoord bool | |
| 633 | - | var hasTabsUC bool | |
| 634 | - | result := dispatch("ui.open_fresh_window", func() { | |
| 635 | - | windowCountBefore = len(a.browserWindows) | |
| 636 | - | hasTabCoord = a.tabCoord != nil | |
| 637 | - | hasTabsUC = a.tabsUC != nil | |
| 638 | - | log.Debug(). | |
| 639 | - | Str("url_host", logging.SafeURLHost(url)). | |
| 640 | - | Int("window_count_before", windowCountBefore). | |
| 641 | - | Bool("has_tab_coord", hasTabCoord). | |
| 642 | - | Bool("has_tabs_uc", hasTabsUC). | |
| 643 | - | Msg("ui: open fresh window main-thread work started") | |
| 644 | - | openErr = a.openFreshWindow(ctx, url) | |
| 645 | - | windowCountAfter = len(a.browserWindows) | |
| 612 | + | func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |
| 613 | + | url = domainurl.Normalize(url) | |
| 614 | + | return a.dispatchWindowURLWork(ctx, "ui.open_external_url", url, func(ctx context.Context) error { | |
| 615 | + | return a.openExternalURLOnMainThread(ctx, url, a.runtimeConfigSnapshot()) | |
| 646 | 616 | }) | |
| 647 | - | if !result.Completed() { | |
| 648 | - | log.Warn(). | |
| 649 | - | Str("url_host", logging.SafeURLHost(url)). | |
| 650 | - | Dur("elapsed", result.Elapsed). | |
| 651 | - | Str("dispatch_status", string(result.Status)). | |
| 652 | - | Msg("ui: open fresh window skipped after main-thread dispatch did not complete") | |
| 653 | - | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |
| 654 | - | } | |
| 655 | - | if openErr != nil { | |
| 656 | - | log.Warn().Err(openErr). | |
| 657 | - | Str("url_host", logging.SafeURLHost(url)). | |
| 658 | - | Dur("elapsed", result.Elapsed). | |
| 659 | - | Str("dispatch_status", string(result.Status)). | |
| 660 | - | Int("window_count_after", windowCountAfter). | |
| 661 | - | Msg("ui: open fresh window failed") | |
| 662 | - | return openErr | |
| 663 | - | } | |
| 617 | + | } | |
| 664 | 618 | ||
| 665 | - | log.Debug(). | |
| 666 | - | Str("url_host", logging.SafeURLHost(url)). | |
| 667 | - | Dur("elapsed", result.Elapsed). | |
| 668 | - | Str("dispatch_status", string(result.Status)). | |
| 669 | - | Int("window_count_after", windowCountAfter). | |
| 670 | - | Msg("ui: open fresh window completed") | |
| 671 | - | return nil | |
| 619 | + | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | |
| 620 | + | url = domainurl.Normalize(url) | |
| 621 | + | return a.dispatchWindowURLWork(ctx, "ui.open_fresh_window", url, func(ctx context.Context) error { | |
| 622 | + | return a.openFreshWindow(ctx, url) | |
| 623 | + | }) | |
| 672 | 624 | } | |
| 673 | 625 | ||
| 674 | 626 | func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | |
| @@ -837,7 +789,7 @@ func verifyExternalPaneCreation(ws *entity.Workspace, previous entity.PaneID, ur | |||
| 837 | 789 | return fmt.Errorf("external URL pane was not activated") | |
| 838 | 790 | } | |
| 839 | 791 | active := ws.ActivePane() | |
| 840 | - | if active == nil || active.Pane == nil || active.Pane.URI != url { | |
| 792 | + | if active == nil || active.Pane == nil || domainurl.Normalize(active.Pane.URI) != domainurl.Normalize(url) { | |
| 841 | 793 | return fmt.Errorf("external URL pane was not created with the requested URL") | |
| 842 | 794 | } | |
| 843 | 795 | return nil | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "github.com/bnema/dumber/internal/application/port" | 9 | "github.com/bnema/dumber/internal/application/port" | ||
| 10 | "github.com/bnema/dumber/internal/application/usecase" | 10 | "github.com/bnema/dumber/internal/application/usecase" | ||
| 11 | "github.com/bnema/dumber/internal/domain/entity" | 11 | "github.com/bnema/dumber/internal/domain/entity" | ||
| 12 | + | domainurl "github.com/bnema/dumber/internal/domain/url" | |||
| 12 | "github.com/bnema/dumber/internal/logging" | 13 | "github.com/bnema/dumber/internal/logging" | ||
| 13 | "github.com/bnema/dumber/internal/shared/syncdispatch" | 14 | "github.com/bnema/dumber/internal/shared/syncdispatch" | ||
| 14 | "github.com/bnema/dumber/internal/ui/component" | 15 | "github.com/bnema/dumber/internal/ui/component" | ||
| @@ -548,11 +549,9 @@ func (a *App) deterministicBrowserWindowFallback() *browserWindow { | |||||
| 548 | return a.browserWindows[ids[0]] | 549 | return a.browserWindows[ids[0]] | ||
| 549 | } | 550 | } | ||
| 550 | 551 | ||||
| 551 | - | func (a *App) OpenExternalURL(ctx context.Context, url string) error { | 552 | + | func (a *App) dispatchWindowURLWork(ctx context.Context, label, url string, work func(context.Context) error) error { |
| 552 | log := logging.FromContext(ctx) | 553 | log := logging.FromContext(ctx) | ||
| 553 | - | log.Debug(). | 554 | + | log.Debug().Str("url_host", logging.SafeURLHost(url)).Str("dispatch_label", label).Msg("ui: window URL dispatch requested") |
| 554 | - | Str("url_host", logging.SafeURLHost(url)). | |||
| 555 | - | Msg("ui: open external URL dispatch requested") | |||
| 556 | 555 | ||||
| 557 | dispatch := a.dispatchOnMainThread | 556 | dispatch := a.dispatchOnMainThread | ||
| 558 | if dispatch == nil { | 557 | if dispatch == nil { | ||
| @@ -564,111 +563,64 @@ func (a *App) OpenExternalURL(ctx context.Context, url string) error { | |||||
| 564 | } | 563 | } | ||
| 565 | } | 564 | } | ||
| 566 | 565 | ||||
| 567 | - | var openErr error | 566 | + | var workErr error |
| 568 | - | var windowCountBefore int | 567 | + | var windowCountBefore, windowCountAfter int |
| 569 | - | var windowCountAfter int | 568 | + | var hasTabCoord, hasTabsUC bool |
| 570 | - | var hasTabCoord bool | 569 | + | result := dispatch(label, func() { |
| 571 | - | var hasTabsUC bool | |||
| 572 | - | result := dispatch("ui.open_external_url", func() { | |||
| 573 | windowCountBefore = len(a.browserWindows) | 570 | windowCountBefore = len(a.browserWindows) | ||
| 574 | hasTabCoord = a.tabCoord != nil | 571 | hasTabCoord = a.tabCoord != nil | ||
| 575 | hasTabsUC = a.tabsUC != nil | 572 | hasTabsUC = a.tabsUC != nil | ||
| 576 | - | cfg := a.runtimeConfigSnapshot() | |||
| 577 | log.Debug(). | 573 | log.Debug(). | ||
| 578 | Str("url_host", logging.SafeURLHost(url)). | 574 | Str("url_host", logging.SafeURLHost(url)). | ||
| 575 | + | Str("dispatch_label", label). | |||
| 579 | Int("window_count_before", windowCountBefore). | 576 | Int("window_count_before", windowCountBefore). | ||
| 580 | Bool("has_tab_coord", hasTabCoord). | 577 | Bool("has_tab_coord", hasTabCoord). | ||
| 581 | Bool("has_tabs_uc", hasTabsUC). | 578 | Bool("has_tabs_uc", hasTabsUC). | ||
| 582 | - | Msg("ui: open external URL main-thread work started") | 579 | + | Msg("ui: window URL main-thread work started") |
| 583 | - | openErr = a.openExternalURLOnMainThread(ctx, url, cfg) | 580 | + | workErr = work(ctx) |
| 584 | windowCountAfter = len(a.browserWindows) | 581 | windowCountAfter = len(a.browserWindows) | ||
| 585 | }) | 582 | }) | ||
| 586 | if !result.Completed() { | 583 | if !result.Completed() { | ||
| 587 | log.Warn(). | 584 | log.Warn(). | ||
| 588 | Str("url_host", logging.SafeURLHost(url)). | 585 | Str("url_host", logging.SafeURLHost(url)). | ||
| 586 | + | Str("dispatch_label", label). | |||
| 589 | Dur("elapsed", result.Elapsed). | 587 | Dur("elapsed", result.Elapsed). | ||
| 590 | Str("dispatch_status", string(result.Status)). | 588 | Str("dispatch_status", string(result.Status)). | ||
| 591 | - | Msg("ui: open external URL skipped after main-thread dispatch did not complete") | 589 | + | Msg("ui: window URL skipped after main-thread dispatch did not complete") |
| 592 | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | 590 | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | ||
| 593 | } | 591 | } | ||
| 594 | - | if openErr != nil { | 592 | + | if workErr != nil { |
| 595 | - | log.Warn().Err(openErr). | 593 | + | log.Warn().Err(workErr). |
| 596 | Str("url_host", logging.SafeURLHost(url)). | 594 | Str("url_host", logging.SafeURLHost(url)). | ||
| 595 | + | Str("dispatch_label", label). | |||
| 597 | Dur("elapsed", result.Elapsed). | 596 | Dur("elapsed", result.Elapsed). | ||
| 598 | Str("dispatch_status", string(result.Status)). | 597 | Str("dispatch_status", string(result.Status)). | ||
| 599 | Int("window_count_after", windowCountAfter). | 598 | Int("window_count_after", windowCountAfter). | ||
| 600 | - | Msg("ui: open external URL failed") | 599 | + | Msg("ui: window URL operation failed") |
| 601 | - | return openErr | 600 | + | return workErr |
| 602 | } | 601 | } | ||
| 603 | - | ||||
| 604 | log.Debug(). | 602 | log.Debug(). | ||
| 605 | Str("url_host", logging.SafeURLHost(url)). | 603 | Str("url_host", logging.SafeURLHost(url)). | ||
| 604 | + | Str("dispatch_label", label). | |||
| 606 | Dur("elapsed", result.Elapsed). | 605 | Dur("elapsed", result.Elapsed). | ||
| 607 | Str("dispatch_status", string(result.Status)). | 606 | Str("dispatch_status", string(result.Status)). | ||
| 608 | Int("window_count_after", windowCountAfter). | 607 | Int("window_count_after", windowCountAfter). | ||
| 609 | - | Msg("ui: open external URL completed") | 608 | + | Msg("ui: window URL operation completed") |
| 610 | return nil | 609 | return nil | ||
| 611 | } | 610 | } | ||
| 612 | 611 | ||||
| 613 | - | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { | 612 | + | func (a *App) OpenExternalURL(ctx context.Context, url string) error { |
| 614 | - | log := logging.FromContext(ctx) | 613 | + | url = domainurl.Normalize(url) |
| 615 | - | log.Debug(). | 614 | + | return a.dispatchWindowURLWork(ctx, "ui.open_external_url", url, func(ctx context.Context) error { |
| 616 | - | Str("url_host", logging.SafeURLHost(url)). | 615 | + | return a.openExternalURLOnMainThread(ctx, url, a.runtimeConfigSnapshot()) |
| 617 | - | Msg("ui: open fresh window dispatch requested") | |||
| 618 | - | ||||
| 619 | - | dispatch := a.dispatchOnMainThread | |||
| 620 | - | if dispatch == nil { | |||
| 621 | - | dispatch = func(label string, fn func()) syncdispatch.SyncDispatchResult { | |||
| 622 | - | if fn != nil { | |||
| 623 | - | fn() | |||
| 624 | - | } | |||
| 625 | - | return syncdispatch.SyncDispatchResult{Label: label, Status: syncdispatch.SyncDispatchInline} | |||
| 626 | - | } | |||
| 627 | - | } | |||
| 628 | - | ||||
| 629 | - | var openErr error | |||
| 630 | - | var windowCountBefore int | |||
| 631 | - | var windowCountAfter int | |||
| 632 | - | var hasTabCoord bool | |||
| 633 | - | var hasTabsUC bool | |||
| 634 | - | result := dispatch("ui.open_fresh_window", func() { | |||
| 635 | - | windowCountBefore = len(a.browserWindows) | |||
| 636 | - | hasTabCoord = a.tabCoord != nil | |||
| 637 | - | hasTabsUC = a.tabsUC != nil | |||
| 638 | - | log.Debug(). | |||
| 639 | - | Str("url_host", logging.SafeURLHost(url)). | |||
| 640 | - | Int("window_count_before", windowCountBefore). | |||
| 641 | - | Bool("has_tab_coord", hasTabCoord). | |||
| 642 | - | Bool("has_tabs_uc", hasTabsUC). | |||
| 643 | - | Msg("ui: open fresh window main-thread work started") | |||
| 644 | - | openErr = a.openFreshWindow(ctx, url) | |||
| 645 | - | windowCountAfter = len(a.browserWindows) | |||
| 646 | }) | 616 | }) | ||
| 647 | - | if !result.Completed() { | 617 | + | } |
| 648 | - | log.Warn(). | |||
| 649 | - | Str("url_host", logging.SafeURLHost(url)). | |||
| 650 | - | Dur("elapsed", result.Elapsed). | |||
| 651 | - | Str("dispatch_status", string(result.Status)). | |||
| 652 | - | Msg("ui: open fresh window skipped after main-thread dispatch did not complete") | |||
| 653 | - | return fmt.Errorf("main thread dispatch did not complete: %s", result.Status) | |||
| 654 | - | } | |||
| 655 | - | if openErr != nil { | |||
| 656 | - | log.Warn().Err(openErr). | |||
| 657 | - | Str("url_host", logging.SafeURLHost(url)). | |||
| 658 | - | Dur("elapsed", result.Elapsed). | |||
| 659 | - | Str("dispatch_status", string(result.Status)). | |||
| 660 | - | Int("window_count_after", windowCountAfter). | |||
| 661 | - | Msg("ui: open fresh window failed") | |||
| 662 | - | return openErr | |||
| 663 | - | } | |||
| 664 | 618 | ||||
| 665 | - | log.Debug(). | 619 | + | func (a *App) OpenFreshWindow(ctx context.Context, url string) error { |
| 666 | - | Str("url_host", logging.SafeURLHost(url)). | 620 | + | url = domainurl.Normalize(url) |
| 667 | - | Dur("elapsed", result.Elapsed). | 621 | + | return a.dispatchWindowURLWork(ctx, "ui.open_fresh_window", url, func(ctx context.Context) error { |
| 668 | - | Str("dispatch_status", string(result.Status)). | 622 | + | return a.openFreshWindow(ctx, url) |
| 669 | - | Int("window_count_after", windowCountAfter). | 623 | + | }) |
| 670 | - | Msg("ui: open fresh window completed") | |||
| 671 | - | return nil | |||
| 672 | } | 624 | } | ||
| 673 | 625 | ||||
| 674 | func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | 626 | func (a *App) openExternalURLOnMainThread(ctx context.Context, url string, cfg entity.RuntimeConfigSnapshot) error { | ||
| @@ -837,7 +789,7 @@ func verifyExternalPaneCreation(ws *entity.Workspace, previous entity.PaneID, ur | |||||
| 837 | return fmt.Errorf("external URL pane was not activated") | 789 | return fmt.Errorf("external URL pane was not activated") | ||
| 838 | } | 790 | } | ||
| 839 | active := ws.ActivePane() | 791 | active := ws.ActivePane() | ||
| 840 | - | if active == nil || active.Pane == nil || active.Pane.URI != url { | 792 | + | if active == nil || active.Pane == nil || domainurl.Normalize(active.Pane.URI) != domainurl.Normalize(url) { |
| 841 | return fmt.Errorf("external URL pane was not created with the requested URL") | 793 | return fmt.Errorf("external URL pane was not created with the requested URL") | ||
| 842 | } | 794 | } | ||
| 843 | return nil | 795 | return nil | ||
M internal/ui/component/favorites_sidebar_form_tags.go +4 -38
| @@ -133,48 +133,14 @@ func (fs *FavoritesSidebar) updateFavoriteTags(favoriteID entity.FavoriteID, wan | |||
| 133 | 133 | } | |
| 134 | 134 | fs.mu.RLock() | |
| 135 | 135 | uc, ctx := fs.favoritesUC, fs.ctx | |
| 136 | - | var current []entity.Tag | |
| 137 | - | for _, fav := range fs.allFavorites { | |
| 138 | - | if fav != nil && fav.ID == favoriteID { | |
| 139 | - | current = append(current, fav.Tags...) | |
| 140 | - | break | |
| 141 | - | } | |
| 142 | - | } | |
| 143 | 136 | fs.mu.RUnlock() | |
| 144 | 137 | if uc == nil { | |
| 145 | 138 | return false | |
| 146 | 139 | } | |
| 147 | - | wantedSet := make(map[entity.TagID]struct{}, len(wanted)) | |
| 148 | - | for _, id := range wanted { | |
| 149 | - | wantedSet[id] = struct{}{} | |
| 150 | - | } | |
| 151 | - | currentSet := tagIDSet(current) | |
| 152 | - | mutated := false | |
| 153 | - | for id := range currentSet { | |
| 154 | - | if _, ok := wantedSet[id]; ok { | |
| 155 | - | continue | |
| 156 | - | } | |
| 157 | - | if err := uc.UntagFavorite(ctx, favoriteID, id); err != nil { | |
| 158 | - | if mutated { | |
| 159 | - | fs.startLoad() | |
| 160 | - | } | |
| 161 | - | fs.setNotice(err.Error()) | |
| 162 | - | return false | |
| 163 | - | } | |
| 164 | - | mutated = true | |
| 165 | - | } | |
| 166 | - | for id := range wantedSet { | |
| 167 | - | if _, ok := currentSet[id]; ok { | |
| 168 | - | continue | |
| 169 | - | } | |
| 170 | - | if err := uc.TagFavorite(ctx, favoriteID, id); err != nil { | |
| 171 | - | if mutated { | |
| 172 | - | fs.startLoad() | |
| 173 | - | } | |
| 174 | - | fs.setNotice(err.Error()) | |
| 175 | - | return false | |
| 176 | - | } | |
| 177 | - | mutated = true | |
| 140 | + | if err := uc.UpdateFavoriteTags(ctx, favoriteID, wanted); err != nil { | |
| 141 | + | fs.startLoad() | |
| 142 | + | fs.setNotice(err.Error()) | |
| 143 | + | return false | |
| 178 | 144 | } | |
| 179 | 145 | return true | |
| 180 | 146 | } | |
| @@ -133,48 +133,14 @@ func (fs *FavoritesSidebar) updateFavoriteTags(favoriteID entity.FavoriteID, wan | |||||
| 133 | } | 133 | } | ||
| 134 | fs.mu.RLock() | 134 | fs.mu.RLock() | ||
| 135 | uc, ctx := fs.favoritesUC, fs.ctx | 135 | uc, ctx := fs.favoritesUC, fs.ctx | ||
| 136 | - | var current []entity.Tag | |||
| 137 | - | for _, fav := range fs.allFavorites { | |||
| 138 | - | if fav != nil && fav.ID == favoriteID { | |||
| 139 | - | current = append(current, fav.Tags...) | |||
| 140 | - | break | |||
| 141 | - | } | |||
| 142 | - | } | |||
| 143 | fs.mu.RUnlock() | 136 | fs.mu.RUnlock() | ||
| 144 | if uc == nil { | 137 | if uc == nil { | ||
| 145 | return false | 138 | return false | ||
| 146 | } | 139 | } | ||
| 147 | - | wantedSet := make(map[entity.TagID]struct{}, len(wanted)) | 140 | + | if err := uc.UpdateFavoriteTags(ctx, favoriteID, wanted); err != nil { |
| 148 | - | for _, id := range wanted { | 141 | + | fs.startLoad() |
| 149 | - | wantedSet[id] = struct{}{} | 142 | + | fs.setNotice(err.Error()) |
| 150 | - | } | 143 | + | return false |
| 151 | - | currentSet := tagIDSet(current) | |||
| 152 | - | mutated := false | |||
| 153 | - | for id := range currentSet { | |||
| 154 | - | if _, ok := wantedSet[id]; ok { | |||
| 155 | - | continue | |||
| 156 | - | } | |||
| 157 | - | if err := uc.UntagFavorite(ctx, favoriteID, id); err != nil { | |||
| 158 | - | if mutated { | |||
| 159 | - | fs.startLoad() | |||
| 160 | - | } | |||
| 161 | - | fs.setNotice(err.Error()) | |||
| 162 | - | return false | |||
| 163 | - | } | |||
| 164 | - | mutated = true | |||
| 165 | - | } | |||
| 166 | - | for id := range wantedSet { | |||
| 167 | - | if _, ok := currentSet[id]; ok { | |||
| 168 | - | continue | |||
| 169 | - | } | |||
| 170 | - | if err := uc.TagFavorite(ctx, favoriteID, id); err != nil { | |||
| 171 | - | if mutated { | |||
| 172 | - | fs.startLoad() | |||
| 173 | - | } | |||
| 174 | - | fs.setNotice(err.Error()) | |||
| 175 | - | return false | |||
| 176 | - | } | |||
| 177 | - | mutated = true | |||
| 178 | } | 144 | } | ||
| 179 | return true | 145 | return true | ||
| 180 | } | 146 | } | ||
M internal/ui/component/favorites_sidebar_test.go +31 -0
| @@ -120,6 +120,37 @@ func (f *fakeFavoritesSidebarUC) UntagFavorite(_ context.Context, favID entity.F | |||
| 120 | 120 | } | |
| 121 | 121 | return f.err | |
| 122 | 122 | } | |
| 123 | + | func (f *fakeFavoritesSidebarUC) UpdateFavoriteTags(ctx context.Context, favID entity.FavoriteID, wanted []entity.TagID) error { | |
| 124 | + | wantedSet := make(map[entity.TagID]struct{}, len(wanted)) | |
| 125 | + | for _, id := range wanted { | |
| 126 | + | wantedSet[id] = struct{}{} | |
| 127 | + | } | |
| 128 | + | var current []entity.Tag | |
| 129 | + | for _, favorite := range f.favorites { | |
| 130 | + | if favorite != nil && favorite.ID == favID { | |
| 131 | + | current = favorite.Tags | |
| 132 | + | break | |
| 133 | + | } | |
| 134 | + | } | |
| 135 | + | currentSet := make(map[entity.TagID]struct{}, len(current)) | |
| 136 | + | for _, tag := range current { | |
| 137 | + | currentSet[tag.ID] = struct{}{} | |
| 138 | + | if _, keep := wantedSet[tag.ID]; !keep { | |
| 139 | + | if err := f.UntagFavorite(ctx, favID, tag.ID); err != nil { | |
| 140 | + | return err | |
| 141 | + | } | |
| 142 | + | } | |
| 143 | + | } | |
| 144 | + | for _, id := range wanted { | |
| 145 | + | if _, exists := currentSet[id]; exists { | |
| 146 | + | continue | |
| 147 | + | } | |
| 148 | + | if err := f.TagFavorite(ctx, favID, id); err != nil { | |
| 149 | + | return err | |
| 150 | + | } | |
| 151 | + | } | |
| 152 | + | return nil | |
| 153 | + | } | |
| 123 | 154 | ||
| 124 | 155 | func newFavoritesSidebarHarness(favorites []*entity.Favorite, tags []*entity.Tag) *FavoritesSidebar { | |
| 125 | 156 | fs := &FavoritesSidebar{ | |
| @@ -120,6 +120,37 @@ func (f *fakeFavoritesSidebarUC) UntagFavorite(_ context.Context, favID entity.F | |||||
| 120 | } | 120 | } | ||
| 121 | return f.err | 121 | return f.err | ||
| 122 | } | 122 | } | ||
| 123 | + | func (f *fakeFavoritesSidebarUC) UpdateFavoriteTags(ctx context.Context, favID entity.FavoriteID, wanted []entity.TagID) error { | |||
| 124 | + | wantedSet := make(map[entity.TagID]struct{}, len(wanted)) | |||
| 125 | + | for _, id := range wanted { | |||
| 126 | + | wantedSet[id] = struct{}{} | |||
| 127 | + | } | |||
| 128 | + | var current []entity.Tag | |||
| 129 | + | for _, favorite := range f.favorites { | |||
| 130 | + | if favorite != nil && favorite.ID == favID { | |||
| 131 | + | current = favorite.Tags | |||
| 132 | + | break | |||
| 133 | + | } | |||
| 134 | + | } | |||
| 135 | + | currentSet := make(map[entity.TagID]struct{}, len(current)) | |||
| 136 | + | for _, tag := range current { | |||
| 137 | + | currentSet[tag.ID] = struct{}{} | |||
| 138 | + | if _, keep := wantedSet[tag.ID]; !keep { | |||
| 139 | + | if err := f.UntagFavorite(ctx, favID, tag.ID); err != nil { | |||
| 140 | + | return err | |||
| 141 | + | } | |||
| 142 | + | } | |||
| 143 | + | } | |||
| 144 | + | for _, id := range wanted { | |||
| 145 | + | if _, exists := currentSet[id]; exists { | |||
| 146 | + | continue | |||
| 147 | + | } | |||
| 148 | + | if err := f.TagFavorite(ctx, favID, id); err != nil { | |||
| 149 | + | return err | |||
| 150 | + | } | |||
| 151 | + | } | |||
| 152 | + | return nil | |||
| 153 | + | } | |||
| 123 | 154 | ||||
| 124 | func newFavoritesSidebarHarness(favorites []*entity.Favorite, tags []*entity.Tag) *FavoritesSidebar { | 155 | func newFavoritesSidebarHarness(favorites []*entity.Favorite, tags []*entity.Tag) *FavoritesSidebar { | ||
| 125 | fs := &FavoritesSidebar{ | 156 | fs := &FavoritesSidebar{ | ||
M internal/ui/component/omnibox_callback_pressure_stress_test.go +0 -2
| @@ -37,8 +37,6 @@ func runGTKCallbackLifecycleStress(t *testing.T) { | |||
| 37 | 37 | if display == nil { | |
| 38 | 38 | t.Skip("GTK native display prerequisite unavailable (no default GDK display)") | |
| 39 | 39 | } | |
| 40 | - | display.Unref() | |
| 41 | - | ||
| 42 | 40 | mainContext := glib.MainContextDefault() | |
| 43 | 41 | require.NotNil(t, mainContext) | |
| 44 | 42 | done := make(chan struct{}) | |
| @@ -37,8 +37,6 @@ func runGTKCallbackLifecycleStress(t *testing.T) { | |||||
| 37 | if display == nil { | 37 | if display == nil { | ||
| 38 | t.Skip("GTK native display prerequisite unavailable (no default GDK display)") | 38 | t.Skip("GTK native display prerequisite unavailable (no default GDK display)") | ||
| 39 | } | 39 | } | ||
| 40 | - | display.Unref() | |||
| 41 | - | ||||
| 42 | mainContext := glib.MainContextDefault() | 40 | mainContext := glib.MainContextDefault() | ||
| 43 | require.NotNil(t, mainContext) | 41 | require.NotNil(t, mainContext) | ||
| 44 | done := make(chan struct{}) | 42 | done := make(chan struct{}) | ||
M internal/ui/coordinator/workspace.go +24 -1
| @@ -212,6 +212,11 @@ func (c *WorkspaceCoordinator) splitWithInitialURL( | |||
| 212 | 212 | ctx, splitCtx.wsView, splitCtx.ws, output, direction, | |
| 213 | 213 | splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID, | |
| 214 | 214 | ); err != nil { | |
| 215 | + | if rollbackErr := c.ClosePaneByID(ctx, output.NewPaneNode.Pane.ID); rollbackErr != nil { | |
| 216 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to roll back split pane") | |
| 217 | + | } | |
| 218 | + | splitCtx.ws.ActivePaneID = oldActivePaneID | |
| 219 | + | c.notifyStateChanged() | |
| 215 | 220 | return err | |
| 216 | 221 | } | |
| 217 | 222 | if notifyNewPane { | |
| @@ -604,6 +609,13 @@ func (c *WorkspaceCoordinator) doIncrementalStackSplit( | |||
| 604 | 609 | wv, err := c.contentCoord.EnsureWebView(ctx, output.NewPaneNode.Pane.ID) | |
| 605 | 610 | if err != nil { | |
| 606 | 611 | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to ensure webview for new pane") | |
| 612 | + | if rollbackErr := c.ClosePaneByID(ctx, output.NewPaneNode.Pane.ID); rollbackErr != nil { | |
| 613 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to roll back stack split pane") | |
| 614 | + | } | |
| 615 | + | if ws := wsView.Workspace(); ws != nil { | |
| 616 | + | ws.ActivePaneID = oldActivePaneID | |
| 617 | + | } | |
| 618 | + | c.notifyStateChanged() | |
| 607 | 619 | return err | |
| 608 | 620 | } | |
| 609 | 621 | ||
| @@ -744,6 +756,11 @@ func (c *WorkspaceCoordinator) doIncrementalSplit( | |||
| 744 | 756 | wv, err := c.contentCoord.EnsureWebView(ctx, output.NewPaneNode.Pane.ID) | |
| 745 | 757 | if err != nil { | |
| 746 | 758 | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to ensure webview for new pane") | |
| 759 | + | if rollbackErr := c.ClosePaneByID(ctx, output.NewPaneNode.Pane.ID); rollbackErr != nil { | |
| 760 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to roll back incremental split pane") | |
| 761 | + | } | |
| 762 | + | ws.ActivePaneID = oldActivePaneID | |
| 763 | + | c.notifyStateChanged() | |
| 747 | 764 | return err | |
| 748 | 765 | } | |
| 749 | 766 | ||
| @@ -1523,7 +1540,7 @@ func (c *WorkspaceCoordinator) stackPaneWithURL(ctx context.Context, initialURL | |||
| 1523 | 1540 | // Get WebView and attach | |
| 1524 | 1541 | wv, err := c.contentCoord.EnsureWebView(ctx, newPaneID) | |
| 1525 | 1542 | if err != nil { | |
| 1526 | - | log.Warn().Err(err).Msg("failed to get webview for new pane") | |
| 1543 | + | c.rollbackStackPane(ctx, stackCtx, newPaneID) | |
| 1527 | 1544 | return err | |
| 1528 | 1545 | } | |
| 1529 | 1546 | widget := c.contentCoord.WrapWidget(ctx, wv) | |
| @@ -1999,6 +2016,11 @@ func (c *WorkspaceCoordinator) insertPopupSplit(ctx context.Context, input conte | |||
| 1999 | 2016 | // Update UI | |
| 2000 | 2017 | if wsView != nil { | |
| 2001 | 2018 | if err := c.applySplitToView(ctx, wsView, ws, output, direction, existingWidget, isStackSplit, input.ParentPaneID); err != nil { | |
| 2019 | + | if rollbackErr := c.ClosePaneByID(ctx, input.PopupPane.ID); rollbackErr != nil { | |
| 2020 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(input.PopupPane.ID)).Msg("failed to roll back popup split pane") | |
| 2021 | + | } | |
| 2022 | + | ws.ActivePaneID = input.ParentPaneID | |
| 2023 | + | c.notifyStateChanged() | |
| 2002 | 2024 | return err | |
| 2003 | 2025 | } | |
| 2004 | 2026 | c.attachPopupWebView(ctx, wsView, input) | |
| @@ -2115,6 +2137,7 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input con | |||
| 2115 | 2137 | ||
| 2116 | 2138 | // Revert stack conversion if we created a new stack | |
| 2117 | 2139 | conversionInfo.revert() | |
| 2140 | + | c.notifyStateChanged() | |
| 2118 | 2141 | ||
| 2119 | 2142 | return err | |
| 2120 | 2143 | } | |
| @@ -212,6 +212,11 @@ func (c *WorkspaceCoordinator) splitWithInitialURL( | |||||
| 212 | ctx, splitCtx.wsView, splitCtx.ws, output, direction, | 212 | ctx, splitCtx.wsView, splitCtx.ws, output, direction, | ||
| 213 | splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID, | 213 | splitCtx.existingWidget, splitCtx.isStackSplit, oldActivePaneID, | ||
| 214 | ); err != nil { | 214 | ); err != nil { | ||
| 215 | + | if rollbackErr := c.ClosePaneByID(ctx, output.NewPaneNode.Pane.ID); rollbackErr != nil { | |||
| 216 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to roll back split pane") | |||
| 217 | + | } | |||
| 218 | + | splitCtx.ws.ActivePaneID = oldActivePaneID | |||
| 219 | + | c.notifyStateChanged() | |||
| 215 | return err | 220 | return err | ||
| 216 | } | 221 | } | ||
| 217 | if notifyNewPane { | 222 | if notifyNewPane { | ||
| @@ -604,6 +609,13 @@ func (c *WorkspaceCoordinator) doIncrementalStackSplit( | |||||
| 604 | wv, err := c.contentCoord.EnsureWebView(ctx, output.NewPaneNode.Pane.ID) | 609 | wv, err := c.contentCoord.EnsureWebView(ctx, output.NewPaneNode.Pane.ID) | ||
| 605 | if err != nil { | 610 | if err != nil { | ||
| 606 | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to ensure webview for new pane") | 611 | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to ensure webview for new pane") | ||
| 612 | + | if rollbackErr := c.ClosePaneByID(ctx, output.NewPaneNode.Pane.ID); rollbackErr != nil { | |||
| 613 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to roll back stack split pane") | |||
| 614 | + | } | |||
| 615 | + | if ws := wsView.Workspace(); ws != nil { | |||
| 616 | + | ws.ActivePaneID = oldActivePaneID | |||
| 617 | + | } | |||
| 618 | + | c.notifyStateChanged() | |||
| 607 | return err | 619 | return err | ||
| 608 | } | 620 | } | ||
| 609 | 621 | ||||
| @@ -744,6 +756,11 @@ func (c *WorkspaceCoordinator) doIncrementalSplit( | |||||
| 744 | wv, err := c.contentCoord.EnsureWebView(ctx, output.NewPaneNode.Pane.ID) | 756 | wv, err := c.contentCoord.EnsureWebView(ctx, output.NewPaneNode.Pane.ID) | ||
| 745 | if err != nil { | 757 | if err != nil { | ||
| 746 | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to ensure webview for new pane") | 758 | log.Warn().Err(err).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to ensure webview for new pane") | ||
| 759 | + | if rollbackErr := c.ClosePaneByID(ctx, output.NewPaneNode.Pane.ID); rollbackErr != nil { | |||
| 760 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(output.NewPaneNode.Pane.ID)).Msg("failed to roll back incremental split pane") | |||
| 761 | + | } | |||
| 762 | + | ws.ActivePaneID = oldActivePaneID | |||
| 763 | + | c.notifyStateChanged() | |||
| 747 | return err | 764 | return err | ||
| 748 | } | 765 | } | ||
| 749 | 766 | ||||
| @@ -1523,7 +1540,7 @@ func (c *WorkspaceCoordinator) stackPaneWithURL(ctx context.Context, initialURL | |||||
| 1523 | // Get WebView and attach | 1540 | // Get WebView and attach | ||
| 1524 | wv, err := c.contentCoord.EnsureWebView(ctx, newPaneID) | 1541 | wv, err := c.contentCoord.EnsureWebView(ctx, newPaneID) | ||
| 1525 | if err != nil { | 1542 | if err != nil { | ||
| 1526 | - | log.Warn().Err(err).Msg("failed to get webview for new pane") | 1543 | + | c.rollbackStackPane(ctx, stackCtx, newPaneID) |
| 1527 | return err | 1544 | return err | ||
| 1528 | } | 1545 | } | ||
| 1529 | widget := c.contentCoord.WrapWidget(ctx, wv) | 1546 | widget := c.contentCoord.WrapWidget(ctx, wv) | ||
| @@ -1999,6 +2016,11 @@ func (c *WorkspaceCoordinator) insertPopupSplit(ctx context.Context, input conte | |||||
| 1999 | // Update UI | 2016 | // Update UI | ||
| 2000 | if wsView != nil { | 2017 | if wsView != nil { | ||
| 2001 | if err := c.applySplitToView(ctx, wsView, ws, output, direction, existingWidget, isStackSplit, input.ParentPaneID); err != nil { | 2018 | if err := c.applySplitToView(ctx, wsView, ws, output, direction, existingWidget, isStackSplit, input.ParentPaneID); err != nil { | ||
| 2019 | + | if rollbackErr := c.ClosePaneByID(ctx, input.PopupPane.ID); rollbackErr != nil { | |||
| 2020 | + | log.Warn().Err(rollbackErr).Str("pane_id", string(input.PopupPane.ID)).Msg("failed to roll back popup split pane") | |||
| 2021 | + | } | |||
| 2022 | + | ws.ActivePaneID = input.ParentPaneID | |||
| 2023 | + | c.notifyStateChanged() | |||
| 2002 | return err | 2024 | return err | ||
| 2003 | } | 2025 | } | ||
| 2004 | c.attachPopupWebView(ctx, wsView, input) | 2026 | c.attachPopupWebView(ctx, wsView, input) | ||
| @@ -2115,6 +2137,7 @@ func (c *WorkspaceCoordinator) insertPopupStacked(ctx context.Context, input con | |||||
| 2115 | 2137 | ||||
| 2116 | // Revert stack conversion if we created a new stack | 2138 | // Revert stack conversion if we created a new stack | ||
| 2117 | conversionInfo.revert() | 2139 | conversionInfo.revert() | ||
| 2140 | + | c.notifyStateChanged() | |||
| 2118 | 2141 | ||||
| 2119 | return err | 2142 | return err | ||
| 2120 | } | 2143 | } | ||
M internal/ui/coordinator/workspace_external_url_test.go +10 -10
| @@ -49,22 +49,22 @@ func TestWorkspaceCoordinator_SplitReturnsErrorWhenCreationTargetMissing(t *test | |||
| 49 | 49 | require.Error(t, coord.SplitWithURL(context.Background(), usecase.SplitRight, "https://example.com")) | |
| 50 | 50 | } | |
| 51 | 51 | ||
| 52 | - | func TestWorkspaceCoordinator_StackPaneUsesConfiguredURLThroughLegacyEntryPoint(t *testing.T) { | |
| 53 | - | const legacyURL = "https://legacy.example/" | |
| 54 | - | coord, ws := stackURLTestCoordinator(t, legacyURL) | |
| 52 | + | func TestWorkspaceCoordinator_StackPaneRollsBackAfterWebViewFailure(t *testing.T) { | |
| 53 | + | coord, ws := stackURLTestCoordinator(t, "https://legacy.example/") | |
| 55 | 54 | ||
| 56 | - | // The test coordinator intentionally has no WebView pool. Creation reaches | |
| 57 | - | // that detectable UI failure only after the requested URL enters the model. | |
| 58 | 55 | require.Error(t, coord.StackPane(context.Background())) | |
| 59 | - | assert.Equal(t, legacyURL, ws.ActivePane().Pane.URI) | |
| 56 | + | assert.Equal(t, entity.PaneID("pane-1"), ws.ActivePaneID) | |
| 57 | + | assert.Equal(t, 1, ws.PaneCount()) | |
| 58 | + | assert.Empty(t, ws.ActivePane().Pane.URI) | |
| 60 | 59 | } | |
| 61 | 60 | ||
| 62 | - | func TestWorkspaceCoordinator_StackPaneWithURLUsesExplicitURL(t *testing.T) { | |
| 63 | - | const explicitURL = "https://explicit.example/path?token=secret" | |
| 61 | + | func TestWorkspaceCoordinator_StackPaneWithURLWithoutOmniboxRollsBackAfterWebViewFailure(t *testing.T) { | |
| 64 | 62 | coord, ws := stackURLTestCoordinator(t, "https://legacy.example/") | |
| 65 | 63 | ||
| 66 | - | require.Error(t, coord.StackPaneWithURL(context.Background(), explicitURL)) | |
| 67 | - | assert.Equal(t, explicitURL, ws.ActivePane().Pane.URI) | |
| 64 | + | require.Error(t, coord.StackPaneWithURLWithoutOmnibox(context.Background(), "https://explicit.example/path?token=secret")) | |
| 65 | + | assert.Equal(t, entity.PaneID("pane-1"), ws.ActivePaneID) | |
| 66 | + | assert.Equal(t, 1, ws.PaneCount()) | |
| 67 | + | assert.Empty(t, ws.ActivePane().Pane.URI) | |
| 68 | 68 | } | |
| 69 | 69 | ||
| 70 | 70 | func stackURLTestCoordinator(t *testing.T, legacyURL string) (*WorkspaceCoordinator, *entity.Workspace) { | |
| @@ -49,22 +49,22 @@ func TestWorkspaceCoordinator_SplitReturnsErrorWhenCreationTargetMissing(t *test | |||||
| 49 | require.Error(t, coord.SplitWithURL(context.Background(), usecase.SplitRight, "https://example.com")) | 49 | require.Error(t, coord.SplitWithURL(context.Background(), usecase.SplitRight, "https://example.com")) | ||
| 50 | } | 50 | } | ||
| 51 | 51 | ||||
| 52 | - | func TestWorkspaceCoordinator_StackPaneUsesConfiguredURLThroughLegacyEntryPoint(t *testing.T) { | 52 | + | func TestWorkspaceCoordinator_StackPaneRollsBackAfterWebViewFailure(t *testing.T) { |
| 53 | - | const legacyURL = "https://legacy.example/" | 53 | + | coord, ws := stackURLTestCoordinator(t, "https://legacy.example/") |
| 54 | - | coord, ws := stackURLTestCoordinator(t, legacyURL) | |||
| 55 | 54 | ||||
| 56 | - | // The test coordinator intentionally has no WebView pool. Creation reaches | |||
| 57 | - | // that detectable UI failure only after the requested URL enters the model. | |||
| 58 | require.Error(t, coord.StackPane(context.Background())) | 55 | require.Error(t, coord.StackPane(context.Background())) | ||
| 59 | - | assert.Equal(t, legacyURL, ws.ActivePane().Pane.URI) | 56 | + | assert.Equal(t, entity.PaneID("pane-1"), ws.ActivePaneID) |
| 57 | + | assert.Equal(t, 1, ws.PaneCount()) | |||
| 58 | + | assert.Empty(t, ws.ActivePane().Pane.URI) | |||
| 60 | } | 59 | } | ||
| 61 | 60 | ||||
| 62 | - | func TestWorkspaceCoordinator_StackPaneWithURLUsesExplicitURL(t *testing.T) { | 61 | + | func TestWorkspaceCoordinator_StackPaneWithURLWithoutOmniboxRollsBackAfterWebViewFailure(t *testing.T) { |
| 63 | - | const explicitURL = "https://explicit.example/path?token=secret" | |||
| 64 | coord, ws := stackURLTestCoordinator(t, "https://legacy.example/") | 62 | coord, ws := stackURLTestCoordinator(t, "https://legacy.example/") | ||
| 65 | 63 | ||||
| 66 | - | require.Error(t, coord.StackPaneWithURL(context.Background(), explicitURL)) | 64 | + | require.Error(t, coord.StackPaneWithURLWithoutOmnibox(context.Background(), "https://explicit.example/path?token=secret")) |
| 67 | - | assert.Equal(t, explicitURL, ws.ActivePane().Pane.URI) | 65 | + | assert.Equal(t, entity.PaneID("pane-1"), ws.ActivePaneID) |
| 66 | + | assert.Equal(t, 1, ws.PaneCount()) | |||
| 67 | + | assert.Empty(t, ws.ActivePane().Pane.URI) | |||
| 68 | } | 68 | } | ||
| 69 | 69 | ||||
| 70 | func stackURLTestCoordinator(t *testing.T, legacyURL string) (*WorkspaceCoordinator, *entity.Workspace) { | 70 | func stackURLTestCoordinator(t *testing.T, legacyURL string) (*WorkspaceCoordinator, *entity.Workspace) { | ||
M internal/ui/dispatcher/keyboard_test.go +8 -8
| @@ -225,16 +225,16 @@ func TestKeyboardDispatcher_VimModeActionsRouteToCorrectScrollCommand(t *testing | |||
| 225 | 225 | ||
| 226 | 226 | tests := []struct { | |
| 227 | 227 | action input.Action | |
| 228 | - | cmd usecase.PageScrollCommand | |
| 228 | + | command port.PageScrollCommand | |
| 229 | 229 | expectedDx int | |
| 230 | 230 | expectedDy int | |
| 231 | 231 | }{ | |
| 232 | - | {input.ActionVimScrollLeft, usecase.PageScrollLeft, -80, 0}, | |
| 233 | - | {input.ActionVimScrollRight, usecase.PageScrollRight, 80, 0}, | |
| 234 | - | {input.ActionVimScrollUp, usecase.PageScrollUp, 0, -80}, | |
| 235 | - | {input.ActionVimScrollDown, usecase.PageScrollDown, 0, 80}, | |
| 236 | - | {input.ActionVimScrollUpFast, usecase.PageScrollUpFast, 0, -320}, | |
| 237 | - | {input.ActionVimScrollDownFast, usecase.PageScrollDownFast, 0, 320}, | |
| 232 | + | {input.ActionVimScrollLeft, port.PageScrollCommandLeft, -80, 0}, | |
| 233 | + | {input.ActionVimScrollRight, port.PageScrollCommandRight, 80, 0}, | |
| 234 | + | {input.ActionVimScrollUp, port.PageScrollCommandUp, 0, -80}, | |
| 235 | + | {input.ActionVimScrollDown, port.PageScrollCommandDown, 0, 80}, | |
| 236 | + | {input.ActionVimScrollUpFast, port.PageScrollCommandUpFast, 0, -320}, | |
| 237 | + | {input.ActionVimScrollDownFast, port.PageScrollCommandDownFast, 0, 320}, | |
| 238 | 238 | } | |
| 239 | 239 | ||
| 240 | 240 | for _, tc := range tests { | |
| @@ -259,7 +259,7 @@ func TestKeyboardDispatcher_VimModeActionsRouteToCorrectScrollCommand(t *testing | |||
| 259 | 259 | ) | |
| 260 | 260 | ||
| 261 | 261 | base.EXPECT().ID().Return(port.WebViewID(42)).Once() | |
| 262 | - | req := port.PageScrollRequest{Command: port.PageScrollCommand(tc.cmd), FallbackDX: tc.expectedDx, FallbackDY: tc.expectedDy} | |
| 262 | + | req := port.PageScrollRequest{Command: tc.command, FallbackDX: tc.expectedDx, FallbackDY: tc.expectedDy} | |
| 263 | 263 | scroller.EXPECT().ScrollPage(ctx, req).Return(nil).Once() | |
| 264 | 264 | ||
| 265 | 265 | err := d.Dispatch(ctx, tc.action) | |
| @@ -225,16 +225,16 @@ func TestKeyboardDispatcher_VimModeActionsRouteToCorrectScrollCommand(t *testing | |||||
| 225 | 225 | ||||
| 226 | tests := []struct { | 226 | tests := []struct { | ||
| 227 | action input.Action | 227 | action input.Action | ||
| 228 | - | cmd usecase.PageScrollCommand | 228 | + | command port.PageScrollCommand |
| 229 | expectedDx int | 229 | expectedDx int | ||
| 230 | expectedDy int | 230 | expectedDy int | ||
| 231 | }{ | 231 | }{ | ||
| 232 | - | {input.ActionVimScrollLeft, usecase.PageScrollLeft, -80, 0}, | 232 | + | {input.ActionVimScrollLeft, port.PageScrollCommandLeft, -80, 0}, |
| 233 | - | {input.ActionVimScrollRight, usecase.PageScrollRight, 80, 0}, | 233 | + | {input.ActionVimScrollRight, port.PageScrollCommandRight, 80, 0}, |
| 234 | - | {input.ActionVimScrollUp, usecase.PageScrollUp, 0, -80}, | 234 | + | {input.ActionVimScrollUp, port.PageScrollCommandUp, 0, -80}, |
| 235 | - | {input.ActionVimScrollDown, usecase.PageScrollDown, 0, 80}, | 235 | + | {input.ActionVimScrollDown, port.PageScrollCommandDown, 0, 80}, |
| 236 | - | {input.ActionVimScrollUpFast, usecase.PageScrollUpFast, 0, -320}, | 236 | + | {input.ActionVimScrollUpFast, port.PageScrollCommandUpFast, 0, -320}, |
| 237 | - | {input.ActionVimScrollDownFast, usecase.PageScrollDownFast, 0, 320}, | 237 | + | {input.ActionVimScrollDownFast, port.PageScrollCommandDownFast, 0, 320}, |
| 238 | } | 238 | } | ||
| 239 | 239 | ||||
| 240 | for _, tc := range tests { | 240 | for _, tc := range tests { | ||
| @@ -259,7 +259,7 @@ func TestKeyboardDispatcher_VimModeActionsRouteToCorrectScrollCommand(t *testing | |||||
| 259 | ) | 259 | ) | ||
| 260 | 260 | ||||
| 261 | base.EXPECT().ID().Return(port.WebViewID(42)).Once() | 261 | base.EXPECT().ID().Return(port.WebViewID(42)).Once() | ||
| 262 | - | req := port.PageScrollRequest{Command: port.PageScrollCommand(tc.cmd), FallbackDX: tc.expectedDx, FallbackDY: tc.expectedDy} | 262 | + | req := port.PageScrollRequest{Command: tc.command, FallbackDX: tc.expectedDx, FallbackDY: tc.expectedDy} |
| 263 | scroller.EXPECT().ScrollPage(ctx, req).Return(nil).Once() | 263 | scroller.EXPECT().ScrollPage(ctx, req).Return(nil).Once() | ||
| 264 | 264 | ||||
| 265 | err := d.Dispatch(ctx, tc.action) | 265 | err := d.Dispatch(ctx, tc.action) | ||
M internal/ui/input/global_shortcuts_test.go +0 -3
| @@ -63,8 +63,6 @@ func TestGlobalShortcutHandlerUsesDistinctNamedActionsAcrossReloads(t *testing.T | |||
| 63 | 63 | if display == nil { | |
| 64 | 64 | t.Skip("GTK native display prerequisite unavailable (no default GDK display)") | |
| 65 | 65 | } | |
| 66 | - | display.Unref() | |
| 67 | - | ||
| 68 | 66 | appID := "com.dumber.GlobalShortcutHandlerTest" | |
| 69 | 67 | app := gtk.NewApplication(&appID, gio.GApplicationNonUniqueValue) | |
| 70 | 68 | if app == nil { | |
| @@ -105,7 +103,6 @@ func TestGlobalShortcutHandlerUsesDistinctNamedActionsAcrossReloads(t *testing.T | |||
| 105 | 103 | t.Fatalf("shortcut %d action was nil", index) | |
| 106 | 104 | } | |
| 107 | 105 | actions[action.GoPointer()] = struct{}{} | |
| 108 | - | action.Unref() | |
| 109 | 106 | shortcut.Unref() | |
| 110 | 107 | } | |
| 111 | 108 | if got := uint(len(actions)); got != shortcutCount { | |
| @@ -63,8 +63,6 @@ func TestGlobalShortcutHandlerUsesDistinctNamedActionsAcrossReloads(t *testing.T | |||||
| 63 | if display == nil { | 63 | if display == nil { | ||
| 64 | t.Skip("GTK native display prerequisite unavailable (no default GDK display)") | 64 | t.Skip("GTK native display prerequisite unavailable (no default GDK display)") | ||
| 65 | } | 65 | } | ||
| 66 | - | display.Unref() | |||
| 67 | - | ||||
| 68 | appID := "com.dumber.GlobalShortcutHandlerTest" | 66 | appID := "com.dumber.GlobalShortcutHandlerTest" | ||
| 69 | app := gtk.NewApplication(&appID, gio.GApplicationNonUniqueValue) | 67 | app := gtk.NewApplication(&appID, gio.GApplicationNonUniqueValue) | ||
| 70 | if app == nil { | 68 | if app == nil { | ||
| @@ -105,7 +103,6 @@ func TestGlobalShortcutHandlerUsesDistinctNamedActionsAcrossReloads(t *testing.T | |||||
| 105 | t.Fatalf("shortcut %d action was nil", index) | 103 | t.Fatalf("shortcut %d action was nil", index) | ||
| 106 | } | 104 | } | ||
| 107 | actions[action.GoPointer()] = struct{}{} | 105 | actions[action.GoPointer()] = struct{}{} | ||
| 108 | - | action.Unref() | |||
| 109 | shortcut.Unref() | 106 | shortcut.Unref() | ||
| 110 | } | 107 | } | ||
| 111 | if got := uint(len(actions)); got != shortcutCount { | 108 | if got := uint(len(actions)); got != shortcutCount { | ||
M internal/ui/input/vimkeys_dispatch.go +7 -0
| @@ -11,6 +11,10 @@ import ( | |||
| 11 | 11 | "github.com/rs/zerolog/log" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | + | func isCountStartKey(key vimkeys.Key) bool { | |
| 15 | + | return key.Mods == 0 && len(key.Sym) == 1 && key.Sym[0] >= '1' && key.Sym[0] <= '9' | |
| 16 | + | } | |
| 17 | + | ||
| 14 | 18 | // trieOwnsBinding reports whether a raw config binding belongs to the Vim | |
| 15 | 19 | // sequence trie rather than the legacy single-chord Vim Mode table. | |
| 16 | 20 | // Multi-key sequences and Vim-only chords that ParseKeyString cannot read are owned. | |
| @@ -20,6 +24,9 @@ func trieOwnsBinding(raw string) (vimkeys.Sequence, bool) { | |||
| 20 | 24 | return nil, false | |
| 21 | 25 | } | |
| 22 | 26 | if len(seq) >= 2 { | |
| 27 | + | if isCountStartKey(seq[0]) { | |
| 28 | + | return nil, false | |
| 29 | + | } | |
| 23 | 30 | return seq, true | |
| 24 | 31 | } | |
| 25 | 32 | if _, ok := ParseKeyString(raw); !ok { | |
| @@ -11,6 +11,10 @@ import ( | |||||
| 11 | "github.com/rs/zerolog/log" | 11 | "github.com/rs/zerolog/log" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | + | func isCountStartKey(key vimkeys.Key) bool { | |||
| 15 | + | return key.Mods == 0 && len(key.Sym) == 1 && key.Sym[0] >= '1' && key.Sym[0] <= '9' | |||
| 16 | + | } | |||
| 17 | + | ||||
| 14 | // trieOwnsBinding reports whether a raw config binding belongs to the Vim | 18 | // trieOwnsBinding reports whether a raw config binding belongs to the Vim | ||
| 15 | // sequence trie rather than the legacy single-chord Vim Mode table. | 19 | // sequence trie rather than the legacy single-chord Vim Mode table. | ||
| 16 | // Multi-key sequences and Vim-only chords that ParseKeyString cannot read are owned. | 20 | // Multi-key sequences and Vim-only chords that ParseKeyString cannot read are owned. | ||
| @@ -20,6 +24,9 @@ func trieOwnsBinding(raw string) (vimkeys.Sequence, bool) { | |||||
| 20 | return nil, false | 24 | return nil, false | ||
| 21 | } | 25 | } | ||
| 22 | if len(seq) >= 2 { | 26 | if len(seq) >= 2 { | ||
| 27 | + | if isCountStartKey(seq[0]) { | |||
| 28 | + | return nil, false | |||
| 29 | + | } | |||
| 23 | return seq, true | 30 | return seq, true | ||
| 24 | } | 31 | } | ||
| 25 | if _, ok := ParseKeyString(raw); !ok { | 32 | if _, ok := ParseKeyString(raw); !ok { | ||
M internal/ui/theme/css.go +3 -2
| @@ -596,7 +596,7 @@ entry.find-bar-entry:focus-visible { | |||
| 596 | 596 | // generateVimModeCSS creates the pane-local Vim Mode accent and pulse styling. | |
| 597 | 597 | // It reuses --pane-mode-color (from workspace.styling.pane_mode_color). | |
| 598 | 598 | func generateVimModeCSS(transitionDurationMs int) string { | |
| 599 | - | if transitionDurationMs < 0 { | |
| 599 | + | if transitionDurationMs <= 0 { | |
| 600 | 600 | transitionDurationMs = defaultTransitionDurationMs | |
| 601 | 601 | } | |
| 602 | 602 | normalPulseMs := transitionDurationMs * 3 | |
| @@ -612,7 +612,8 @@ func generateVimModeCSS(transitionDurationMs int) string { | |||
| 612 | 612 | } | |
| 613 | 613 | ||
| 614 | 614 | /* Pane overlay pulse keyframes — start/end at the active accent state so the | |
| 615 | - | pane keeps its Vim mode border while briefly flaring brighter. */ | |
| 615 | + | pane keeps its Vim mode border while briefly flaring brighter. The separate | |
| 616 | + | -anim-a and -anim-b pairs are required to restart repeated pulse animations. */ | |
| 616 | 617 | @keyframes vim-mode-overlay-pulse-anim-a { | |
| 617 | 618 | 0%% { box-shadow: inset 0 0 0 0.125em alpha(var(--pane-mode-color), 0.35); } | |
| 618 | 619 | 35%% { box-shadow: inset 0 0 0 0.2em alpha(var(--pane-mode-color), 0.78); } | |
| @@ -596,7 +596,7 @@ entry.find-bar-entry:focus-visible { | |||||
| 596 | // generateVimModeCSS creates the pane-local Vim Mode accent and pulse styling. | 596 | // generateVimModeCSS creates the pane-local Vim Mode accent and pulse styling. | ||
| 597 | // It reuses --pane-mode-color (from workspace.styling.pane_mode_color). | 597 | // It reuses --pane-mode-color (from workspace.styling.pane_mode_color). | ||
| 598 | func generateVimModeCSS(transitionDurationMs int) string { | 598 | func generateVimModeCSS(transitionDurationMs int) string { | ||
| 599 | - | if transitionDurationMs < 0 { | 599 | + | if transitionDurationMs <= 0 { |
| 600 | transitionDurationMs = defaultTransitionDurationMs | 600 | transitionDurationMs = defaultTransitionDurationMs | ||
| 601 | } | 601 | } | ||
| 602 | normalPulseMs := transitionDurationMs * 3 | 602 | normalPulseMs := transitionDurationMs * 3 | ||
| @@ -612,7 +612,8 @@ func generateVimModeCSS(transitionDurationMs int) string { | |||||
| 612 | } | 612 | } | ||
| 613 | 613 | ||||
| 614 | /* Pane overlay pulse keyframes — start/end at the active accent state so the | 614 | /* Pane overlay pulse keyframes — start/end at the active accent state so the | ||
| 615 | - | pane keeps its Vim mode border while briefly flaring brighter. */ | 615 | + | pane keeps its Vim mode border while briefly flaring brighter. The separate |
| 616 | + | -anim-a and -anim-b pairs are required to restart repeated pulse animations. */ | |||
| 616 | @keyframes vim-mode-overlay-pulse-anim-a { | 617 | @keyframes vim-mode-overlay-pulse-anim-a { | ||
| 617 | 0%% { box-shadow: inset 0 0 0 0.125em alpha(var(--pane-mode-color), 0.35); } | 618 | 0%% { box-shadow: inset 0 0 0 0.125em alpha(var(--pane-mode-color), 0.35); } | ||
| 618 | 35%% { box-shadow: inset 0 0 0 0.2em alpha(var(--pane-mode-color), 0.78); } | 619 | 35%% { box-shadow: inset 0 0 0 0.2em alpha(var(--pane-mode-color), 0.78); } | ||
M internal/ui/theme/favorites_sidebar_css.go +12 -0
| @@ -28,6 +28,12 @@ func generateFavoritesSidebarCSS(_ Palette) string { | |||
| 28 | 28 | background-color: alpha(var(--accent), 0.10); | |
| 29 | 29 | } | |
| 30 | 30 | ||
| 31 | + | .favorites-sidebar-tag-filter:focus { | |
| 32 | + | color: var(--text); | |
| 33 | + | border-color: var(--accent); | |
| 34 | + | box-shadow: inset 0 0 0 0.0625em var(--accent); | |
| 35 | + | } | |
| 36 | + | ||
| 31 | 37 | .favorites-sidebar-tag-filter-active { | |
| 32 | 38 | color: var(--accent); | |
| 33 | 39 | background-color: alpha(var(--accent), 0.16); | |
| @@ -113,6 +119,12 @@ func generateFavoritesSidebarCSS(_ Palette) string { | |||
| 113 | 119 | background-color: alpha(var(--accent), 0.12); | |
| 114 | 120 | } | |
| 115 | 121 | ||
| 122 | + | .favorites-sidebar-form-tag-match:focus { | |
| 123 | + | color: var(--text); | |
| 124 | + | border-color: var(--accent); | |
| 125 | + | box-shadow: inset 0 0 0 0.0625em var(--accent); | |
| 126 | + | } | |
| 127 | + | ||
| 116 | 128 | .favorites-sidebar-form button { | |
| 117 | 129 | min-height: 0; | |
| 118 | 130 | padding: 0.1875em 0.5em; | |
| @@ -28,6 +28,12 @@ func generateFavoritesSidebarCSS(_ Palette) string { | |||||
| 28 | background-color: alpha(var(--accent), 0.10); | 28 | background-color: alpha(var(--accent), 0.10); | ||
| 29 | } | 29 | } | ||
| 30 | 30 | ||||
| 31 | + | .favorites-sidebar-tag-filter:focus { | |||
| 32 | + | color: var(--text); | |||
| 33 | + | border-color: var(--accent); | |||
| 34 | + | box-shadow: inset 0 0 0 0.0625em var(--accent); | |||
| 35 | + | } | |||
| 36 | + | ||||
| 31 | .favorites-sidebar-tag-filter-active { | 37 | .favorites-sidebar-tag-filter-active { | ||
| 32 | color: var(--accent); | 38 | color: var(--accent); | ||
| 33 | background-color: alpha(var(--accent), 0.16); | 39 | background-color: alpha(var(--accent), 0.16); | ||
| @@ -113,6 +119,12 @@ func generateFavoritesSidebarCSS(_ Palette) string { | |||||
| 113 | background-color: alpha(var(--accent), 0.12); | 119 | background-color: alpha(var(--accent), 0.12); | ||
| 114 | } | 120 | } | ||
| 115 | 121 | ||||
| 122 | + | .favorites-sidebar-form-tag-match:focus { | |||
| 123 | + | color: var(--text); | |||
| 124 | + | border-color: var(--accent); | |||
| 125 | + | box-shadow: inset 0 0 0 0.0625em var(--accent); | |||
| 126 | + | } | |||
| 127 | + | ||||
| 116 | .favorites-sidebar-form button { | 128 | .favorites-sidebar-form button { | ||
| 117 | min-height: 0; | 129 | min-height: 0; | ||
| 118 | padding: 0.1875em 0.5em; | 130 | padding: 0.1875em 0.5em; | ||
M internal/ui/theme/manager.go +1 -4
| @@ -31,7 +31,7 @@ type Manager struct { | |||
| 31 | 31 | func NewManager(ctx context.Context, resolved entity.ResolvedTheme) *Manager { | |
| 32 | 32 | log := logging.FromContext(ctx) | |
| 33 | 33 | ||
| 34 | - | m := &Manager{} | |
| 34 | + | m := &Manager{transitionDurationMs: defaultTransitionDurationMs} | |
| 35 | 35 | m.applyResolvedTheme(resolved) | |
| 36 | 36 | ||
| 37 | 37 | log.Debug(). | |
| @@ -59,9 +59,6 @@ func (m *Manager) applyResolvedTheme(resolved entity.ResolvedTheme) { | |||
| 59 | 59 | } | |
| 60 | 60 | m.gtkFont = m.fonts.GtkFont | |
| 61 | 61 | m.modeColors = ModeColorsFromEntity(resolved.ModeColors) | |
| 62 | - | if m.transitionDurationMs <= 0 { | |
| 63 | - | m.transitionDurationMs = defaultTransitionDurationMs | |
| 64 | - | } | |
| 65 | 62 | } | |
| 66 | 63 | ||
| 67 | 64 | // SetTransitionDuration sets the CSS transition duration used for theme-driven | |
| @@ -31,7 +31,7 @@ type Manager struct { | |||||
| 31 | func NewManager(ctx context.Context, resolved entity.ResolvedTheme) *Manager { | 31 | func NewManager(ctx context.Context, resolved entity.ResolvedTheme) *Manager { | ||
| 32 | log := logging.FromContext(ctx) | 32 | log := logging.FromContext(ctx) | ||
| 33 | 33 | ||||
| 34 | - | m := &Manager{} | 34 | + | m := &Manager{transitionDurationMs: defaultTransitionDurationMs} |
| 35 | m.applyResolvedTheme(resolved) | 35 | m.applyResolvedTheme(resolved) | ||
| 36 | 36 | ||||
| 37 | log.Debug(). | 37 | log.Debug(). | ||
| @@ -59,9 +59,6 @@ func (m *Manager) applyResolvedTheme(resolved entity.ResolvedTheme) { | |||||
| 59 | } | 59 | } | ||
| 60 | m.gtkFont = m.fonts.GtkFont | 60 | m.gtkFont = m.fonts.GtkFont | ||
| 61 | m.modeColors = ModeColorsFromEntity(resolved.ModeColors) | 61 | m.modeColors = ModeColorsFromEntity(resolved.ModeColors) | ||
| 62 | - | if m.transitionDurationMs <= 0 { | |||
| 63 | - | m.transitionDurationMs = defaultTransitionDurationMs | |||
| 64 | - | } | |||
| 65 | } | 62 | } | ||
| 66 | 63 | ||||
| 67 | // SetTransitionDuration sets the CSS transition duration used for theme-driven | 64 | // SetTransitionDuration sets the CSS transition duration used for theme-driven | ||
M internal/ui/theme/sidebar_css.go +3 -1
| @@ -30,7 +30,7 @@ func generateSidebarCSS(_ Palette) string { | |||
| 30 | 30 | .sidebar-row { | |
| 31 | 31 | padding: 0.1875em 0.625em; | |
| 32 | 32 | min-height: 0; | |
| 33 | - | border-bottom: 0.0625em solid alpha(var(--border), 0.4); | |
| 33 | + | border-bottom: 0.0625em solid color-mix(in srgb, var(--border) 40%, transparent); | |
| 34 | 34 | background-color: var(--surface); | |
| 35 | 35 | transition: background-color 100ms ease; | |
| 36 | 36 | } | |
| @@ -45,6 +45,8 @@ func generateSidebarCSS(_ Palette) string { | |||
| 45 | 45 | ||
| 46 | 46 | .sidebar-row:focus { | |
| 47 | 47 | background-color: alpha(var(--accent), 0.18); | |
| 48 | + | outline: 0.0625em solid var(--accent); | |
| 49 | + | outline-offset: -0.0625em; | |
| 48 | 50 | } | |
| 49 | 51 | ||
| 50 | 52 | .sidebar-row-title { | |
| @@ -30,7 +30,7 @@ func generateSidebarCSS(_ Palette) string { | |||||
| 30 | .sidebar-row { | 30 | .sidebar-row { | ||
| 31 | padding: 0.1875em 0.625em; | 31 | padding: 0.1875em 0.625em; | ||
| 32 | min-height: 0; | 32 | min-height: 0; | ||
| 33 | - | border-bottom: 0.0625em solid alpha(var(--border), 0.4); | 33 | + | border-bottom: 0.0625em solid color-mix(in srgb, var(--border) 40%, transparent); |
| 34 | background-color: var(--surface); | 34 | background-color: var(--surface); | ||
| 35 | transition: background-color 100ms ease; | 35 | transition: background-color 100ms ease; | ||
| 36 | } | 36 | } | ||
| @@ -45,6 +45,8 @@ func generateSidebarCSS(_ Palette) string { | |||||
| 45 | 45 | ||||
| 46 | .sidebar-row:focus { | 46 | .sidebar-row:focus { | ||
| 47 | background-color: alpha(var(--accent), 0.18); | 47 | background-color: alpha(var(--accent), 0.18); | ||
| 48 | + | outline: 0.0625em solid var(--accent); | |||
| 49 | + | outline-offset: -0.0625em; | |||
| 48 | } | 50 | } | ||
| 49 | 51 | ||||
| 50 | .sidebar-row-title { | 52 | .sidebar-row-title { | ||