fix(cef): cap the visual copy allowance at one and tighten accent color validation
- commit
- a877e408d24dd128b5b18663115c0111e2e7ba67
- author
- brice · · 2026-10-04 16:42 UTC
- committer
- brice · · 2026-10-10 08:28 UTC
- parent
- fea7b72
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email - refs
- feat/vim-hints-visual-yank
2 files · +17 -4
M internal/infrastructure/cef/vim_page.go +5 -3
| @@ -29,7 +29,7 @@ var ( | |||
| 29 | 29 | errVimPagePayloadTooLarge = errors.New("vim page: payload too large") | |
| 30 | 30 | // vimPageAccentColor admits the CSS hex colors the runtime paints with; | |
| 31 | 31 | // anything else is replaced by the runtime default before reaching JS. | |
| 32 | - | vimPageAccentColor = regexp.MustCompile(`^#[0-9a-fA-F]{3,8}$`) | |
| 32 | + | vimPageAccentColor = regexp.MustCompile(`^#(?:[0-9a-fA-F]{3,4}|[0-9a-fA-F]{6}|[0-9a-fA-F]{8})$`) | |
| 33 | 33 | ) | |
| 34 | 34 | ||
| 35 | 35 | // vimPageBridgePayload is one result reported by the page runtime. Token is | |
| @@ -323,14 +323,16 @@ func vimPageKeyRequestsCopy(key string) bool { | |||
| 323 | 323 | } | |
| 324 | 324 | ||
| 325 | 325 | // grantVimPageCopyAllowance records that the user pressed a copy key for the | |
| 326 | - | // armed visual interaction, so one visual copy result will be accepted. | |
| 326 | + | // armed visual interaction, so one visual copy result will be accepted. The | |
| 327 | + | // allowance is capped at one: y/Enter presses the page ignores (caret or hint | |
| 328 | + | // phase, empty selection) must not bank copies for a page holding the token. | |
| 327 | 329 | func (wv *WebView) grantVimPageCopyAllowance(token, key string) { | |
| 328 | 330 | if !vimPageKeyRequestsCopy(key) { | |
| 329 | 331 | return | |
| 330 | 332 | } | |
| 331 | 333 | wv.mu.Lock() | |
| 332 | 334 | if wv.vimPageToken == token && wv.vimPageKind == dto.VimPageVisual { | |
| 333 | - | wv.vimPageCopyAllowance++ | |
| 335 | + | wv.vimPageCopyAllowance = 1 | |
| 334 | 336 | } | |
| 335 | 337 | wv.mu.Unlock() | |
| 336 | 338 | } | |
| @@ -29,7 +29,7 @@ var ( | |||||
| 29 | errVimPagePayloadTooLarge = errors.New("vim page: payload too large") | 29 | errVimPagePayloadTooLarge = errors.New("vim page: payload too large") | ||
| 30 | // vimPageAccentColor admits the CSS hex colors the runtime paints with; | 30 | // vimPageAccentColor admits the CSS hex colors the runtime paints with; | ||
| 31 | // anything else is replaced by the runtime default before reaching JS. | 31 | // anything else is replaced by the runtime default before reaching JS. | ||
| 32 | - | vimPageAccentColor = regexp.MustCompile(`^#[0-9a-fA-F]{3,8}$`) | 32 | + | vimPageAccentColor = regexp.MustCompile(`^#(?:[0-9a-fA-F]{3,4}|[0-9a-fA-F]{6}|[0-9a-fA-F]{8})$`) |
| 33 | ) | 33 | ) | ||
| 34 | 34 | ||||
| 35 | // vimPageBridgePayload is one result reported by the page runtime. Token is | 35 | // vimPageBridgePayload is one result reported by the page runtime. Token is | ||
| @@ -323,14 +323,16 @@ func vimPageKeyRequestsCopy(key string) bool { | |||||
| 323 | } | 323 | } | ||
| 324 | 324 | ||||
| 325 | // grantVimPageCopyAllowance records that the user pressed a copy key for the | 325 | // grantVimPageCopyAllowance records that the user pressed a copy key for the | ||
| 326 | - | // armed visual interaction, so one visual copy result will be accepted. | 326 | + | // armed visual interaction, so one visual copy result will be accepted. The |
| 327 | + | // allowance is capped at one: y/Enter presses the page ignores (caret or hint | |||
| 328 | + | // phase, empty selection) must not bank copies for a page holding the token. | |||
| 327 | func (wv *WebView) grantVimPageCopyAllowance(token, key string) { | 329 | func (wv *WebView) grantVimPageCopyAllowance(token, key string) { | ||
| 328 | if !vimPageKeyRequestsCopy(key) { | 330 | if !vimPageKeyRequestsCopy(key) { | ||
| 329 | return | 331 | return | ||
| 330 | } | 332 | } | ||
| 331 | wv.mu.Lock() | 333 | wv.mu.Lock() | ||
| 332 | if wv.vimPageToken == token && wv.vimPageKind == dto.VimPageVisual { | 334 | if wv.vimPageToken == token && wv.vimPageKind == dto.VimPageVisual { | ||
| 333 | - | wv.vimPageCopyAllowance++ | 335 | + | wv.vimPageCopyAllowance = 1 |
| 334 | } | 336 | } | ||
| 335 | wv.mu.Unlock() | 337 | wv.mu.Unlock() | ||
| 336 | } | 338 | } | ||
M internal/infrastructure/cef/vim_page_test.go +12 -1
| @@ -110,6 +110,8 @@ func TestVimPageRequestJSONValidatesAccentColor(t *testing.T) { | |||
| 110 | 110 | "": `{"kind":"visual"}`, | |
| 111 | 111 | "red": `{"kind":"visual"}`, | |
| 112 | 112 | "#12": `{"kind":"visual"}`, | |
| 113 | + | "#12345": `{"kind":"visual"}`, | |
| 114 | + | "#1234567": `{"kind":"visual"}`, | |
| 113 | 115 | "#123456789": `{"kind":"visual"}`, | |
| 114 | 116 | "#abc;} body{display:none": `{"kind":"visual"}`, | |
| 115 | 117 | "url(javascript:alert(1))": `{"kind":"visual"}`, | |
| @@ -402,10 +404,19 @@ func TestWebViewHandleVimPageResultVisualCopyNeedsForwardedCopyKey(t *testing.T) | |||
| 402 | 404 | wv.grantVimPageCopyAllowance("other", "y") | |
| 403 | 405 | wv.handleVimPageResult(copyResult) | |
| 404 | 406 | assert.Len(t, *queued, 2) | |
| 407 | + | ||
| 408 | + | // y presses the page ignored (caret phase, empty selection) do not bank | |
| 409 | + | // copies for later. | |
| 410 | + | wv.grantVimPageCopyAllowance("tok", "y") | |
| 411 | + | wv.grantVimPageCopyAllowance("tok", "y") | |
| 412 | + | wv.grantVimPageCopyAllowance("tok", "y") | |
| 413 | + | wv.handleVimPageResult(copyResult) | |
| 414 | + | wv.handleVimPageResult(copyResult) | |
| 415 | + | assert.Len(t, *queued, 3, "ignored copy keys do not accumulate allowance") | |
| 405 | 416 | } | |
| 406 | 417 | ||
| 407 | 418 | func TestWebViewSendVimPageKeyGrantsCopyAllowanceOnlyForVisual(t *testing.T) { | |
| 408 | - | for kind, want := range map[dto.VimPageInteractionKind]int{dto.VimPageVisual: 2, dto.VimPageHintYankURL: 0} { | |
| 419 | + | for kind, want := range map[dto.VimPageInteractionKind]int{dto.VimPageVisual: 1, dto.VimPageHintYankURL: 0} { | |
| 409 | 420 | browser := cefmocks.NewMockBrowser(t) | |
| 410 | 421 | frame := cefmocks.NewMockFrame(t) | |
| 411 | 422 | browser.EXPECT().GetMainFrame().Return(frame).Times(3) | |
| @@ -110,6 +110,8 @@ func TestVimPageRequestJSONValidatesAccentColor(t *testing.T) { | |||||
| 110 | "": `{"kind":"visual"}`, | 110 | "": `{"kind":"visual"}`, | ||
| 111 | "red": `{"kind":"visual"}`, | 111 | "red": `{"kind":"visual"}`, | ||
| 112 | "#12": `{"kind":"visual"}`, | 112 | "#12": `{"kind":"visual"}`, | ||
| 113 | + | "#12345": `{"kind":"visual"}`, | |||
| 114 | + | "#1234567": `{"kind":"visual"}`, | |||
| 113 | "#123456789": `{"kind":"visual"}`, | 115 | "#123456789": `{"kind":"visual"}`, | ||
| 114 | "#abc;} body{display:none": `{"kind":"visual"}`, | 116 | "#abc;} body{display:none": `{"kind":"visual"}`, | ||
| 115 | "url(javascript:alert(1))": `{"kind":"visual"}`, | 117 | "url(javascript:alert(1))": `{"kind":"visual"}`, | ||
| @@ -402,10 +404,19 @@ func TestWebViewHandleVimPageResultVisualCopyNeedsForwardedCopyKey(t *testing.T) | |||||
| 402 | wv.grantVimPageCopyAllowance("other", "y") | 404 | wv.grantVimPageCopyAllowance("other", "y") | ||
| 403 | wv.handleVimPageResult(copyResult) | 405 | wv.handleVimPageResult(copyResult) | ||
| 404 | assert.Len(t, *queued, 2) | 406 | assert.Len(t, *queued, 2) | ||
| 407 | + | ||||
| 408 | + | // y presses the page ignored (caret phase, empty selection) do not bank | |||
| 409 | + | // copies for later. | |||
| 410 | + | wv.grantVimPageCopyAllowance("tok", "y") | |||
| 411 | + | wv.grantVimPageCopyAllowance("tok", "y") | |||
| 412 | + | wv.grantVimPageCopyAllowance("tok", "y") | |||
| 413 | + | wv.handleVimPageResult(copyResult) | |||
| 414 | + | wv.handleVimPageResult(copyResult) | |||
| 415 | + | assert.Len(t, *queued, 3, "ignored copy keys do not accumulate allowance") | |||
| 405 | } | 416 | } | ||
| 406 | 417 | ||||
| 407 | func TestWebViewSendVimPageKeyGrantsCopyAllowanceOnlyForVisual(t *testing.T) { | 418 | func TestWebViewSendVimPageKeyGrantsCopyAllowanceOnlyForVisual(t *testing.T) { | ||
| 408 | - | for kind, want := range map[dto.VimPageInteractionKind]int{dto.VimPageVisual: 2, dto.VimPageHintYankURL: 0} { | 419 | + | for kind, want := range map[dto.VimPageInteractionKind]int{dto.VimPageVisual: 1, dto.VimPageHintYankURL: 0} { |
| 409 | browser := cefmocks.NewMockBrowser(t) | 420 | browser := cefmocks.NewMockBrowser(t) | ||
| 410 | frame := cefmocks.NewMockFrame(t) | 421 | frame := cefmocks.NewMockFrame(t) | ||
| 411 | browser.EXPECT().GetMainFrame().Return(frame).Times(3) | 422 | browser.EXPECT().GetMainFrame().Return(frame).Times(3) | ||