fix(hints): reject stale geometry and bound scan discovery
- commit
- 638a7997fe6ebfa31b55958e1b700a33e50a6e98
- author
- brice · · 2026-10-19 18:49 UTC
- parent
- 353b4e3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +390 -56
M .mockery.yml +5 -0
| @@ -113,6 +113,11 @@ packages: | |||
| 113 | 113 | filename: proc_tree_mock_test.go | |
| 114 | 114 | github.com/bnema/neferwl/internal/adapters/hints: | |
| 115 | 115 | interfaces: | |
| 116 | + | languageRunner: | |
| 117 | + | config: | |
| 118 | + | dir: internal/adapters/hints | |
| 119 | + | pkgname: hints | |
| 120 | + | filename: language_runner_mock_test.go | |
| 116 | 121 | targetSource: | |
| 117 | 122 | config: | |
| 118 | 123 | dir: internal/adapters/hints | |
| @@ -113,6 +113,11 @@ packages: | |||||
| 113 | filename: proc_tree_mock_test.go | 113 | filename: proc_tree_mock_test.go | ||
| 114 | github.com/bnema/neferwl/internal/adapters/hints: | 114 | github.com/bnema/neferwl/internal/adapters/hints: | ||
| 115 | interfaces: | 115 | interfaces: | ||
| 116 | + | languageRunner: | |||
| 117 | + | config: | |||
| 118 | + | dir: internal/adapters/hints | |||
| 119 | + | pkgname: hints | |||
| 120 | + | filename: language_runner_mock_test.go | |||
| 116 | targetSource: | 121 | targetSource: | ||
| 117 | config: | 122 | config: | ||
| 118 | dir: internal/adapters/hints | 123 | dir: internal/adapters/hints | ||
M docs/config.md +2 -1
| @@ -335,7 +335,8 @@ small or low-contrast text. | |||
| 335 | 335 | ||
| 336 | 336 | ### Requirements | |
| 337 | 337 | ||
| 338 | - | - Accessibility: NeferWL turns it on at startup. Some apps need a flag: | |
| 338 | + | - Accessibility: NeferWL enables it lazily on the first hint scan that queries | |
| 339 | + | app windows. Apps started before then may need restarting. Some apps need a flag: | |
| 339 | 340 | - Chromium: `--force-renderer-accessibility` | |
| 340 | 341 | - Electron: `ACCESSIBILITY_ENABLED=1` | |
| 341 | 342 | - Qt: `QT_LINUX_ACCESSIBILITY_ALWAYS_ON=1` | |
| @@ -335,7 +335,8 @@ small or low-contrast text. | |||||
| 335 | 335 | ||||
| 336 | ### Requirements | 336 | ### Requirements | ||
| 337 | 337 | ||||
| 338 | - | - Accessibility: NeferWL turns it on at startup. Some apps need a flag: | 338 | + | - Accessibility: NeferWL enables it lazily on the first hint scan that queries |
| 339 | + | app windows. Apps started before then may need restarting. Some apps need a flag: | |||
| 339 | - Chromium: `--force-renderer-accessibility` | 340 | - Chromium: `--force-renderer-accessibility` | ||
| 340 | - Electron: `ACCESSIBILITY_ENABLED=1` | 341 | - Electron: `ACCESSIBILITY_ENABLED=1` | ||
| 341 | - Qt: `QT_LINUX_ACCESSIBILITY_ALWAYS_ON=1` | 342 | - Qt: `QT_LINUX_ACCESSIBILITY_ALWAYS_ON=1` | ||
M internal/adapters/atspi/source.go +4 -5
| @@ -459,7 +459,8 @@ func (s *Source) walk(do func([]call) []reply, root ref, linksOnly bool) []ref { | |||
| 459 | 459 | call{op: opState, name: n.name, path: n.path}) | |
| 460 | 460 | } | |
| 461 | 461 | rep := do(calls) | |
| 462 | - | var next []ref | |
| 462 | + | room := max(0, maxWalkNodes-seen-len(level)) | |
| 463 | + | next := make([]ref, 0, room) | |
| 463 | 464 | for i, n := range level { | |
| 464 | 465 | ch, role, state := rep[3*i], rep[3*i+1], rep[3*i+2] | |
| 465 | 466 | if state.err == nil && !hasState(state.state, stateShowing) { | |
| @@ -469,13 +470,11 @@ func (s *Source) walk(do func([]call) []reply, root ref, linksOnly bool) []ref { | |||
| 469 | 470 | out = append(out, n) | |
| 470 | 471 | } | |
| 471 | 472 | if ch.err == nil { | |
| 472 | - | next = append(next, ch.refs...) | |
| 473 | + | take := min(len(ch.refs), room-len(next)) | |
| 474 | + | next = append(next, ch.refs[:take]...) | |
| 473 | 475 | } | |
| 474 | 476 | } | |
| 475 | 477 | seen += len(level) | |
| 476 | - | if room := maxWalkNodes - seen; len(next) > room { | |
| 477 | - | next = next[:max(room, 0)] | |
| 478 | - | } | |
| 479 | 478 | level = next | |
| 480 | 479 | } | |
| 481 | 480 | return out | |
| @@ -459,7 +459,8 @@ func (s *Source) walk(do func([]call) []reply, root ref, linksOnly bool) []ref { | |||||
| 459 | call{op: opState, name: n.name, path: n.path}) | 459 | call{op: opState, name: n.name, path: n.path}) | ||
| 460 | } | 460 | } | ||
| 461 | rep := do(calls) | 461 | rep := do(calls) | ||
| 462 | - | var next []ref | 462 | + | room := max(0, maxWalkNodes-seen-len(level)) |
| 463 | + | next := make([]ref, 0, room) | |||
| 463 | for i, n := range level { | 464 | for i, n := range level { | ||
| 464 | ch, role, state := rep[3*i], rep[3*i+1], rep[3*i+2] | 465 | ch, role, state := rep[3*i], rep[3*i+1], rep[3*i+2] | ||
| 465 | if state.err == nil && !hasState(state.state, stateShowing) { | 466 | if state.err == nil && !hasState(state.state, stateShowing) { | ||
| @@ -469,13 +470,11 @@ func (s *Source) walk(do func([]call) []reply, root ref, linksOnly bool) []ref { | |||||
| 469 | out = append(out, n) | 470 | out = append(out, n) | ||
| 470 | } | 471 | } | ||
| 471 | if ch.err == nil { | 472 | if ch.err == nil { | ||
| 472 | - | next = append(next, ch.refs...) | 473 | + | take := min(len(ch.refs), room-len(next)) |
| 474 | + | next = append(next, ch.refs[:take]...) | |||
| 473 | } | 475 | } | ||
| 474 | } | 476 | } | ||
| 475 | seen += len(level) | 477 | seen += len(level) | ||
| 476 | - | if room := maxWalkNodes - seen; len(next) > room { | |||
| 477 | - | next = next[:max(room, 0)] | |||
| 478 | - | } | |||
| 479 | level = next | 478 | level = next | ||
| 480 | } | 479 | } | ||
| 481 | return out | 480 | return out | ||
M internal/adapters/atspi/source_test.go +40 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "context" | |
| 5 | 5 | "errors" | |
| 6 | 6 | "fmt" | |
| 7 | + | "runtime" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| @@ -313,3 +314,42 @@ func TestCloseReapsLateTransport(t *testing.T) { | |||
| 313 | 314 | require.Empty(t, covered) | |
| 314 | 315 | require.NoError(t, s.Close()) // idempotent | |
| 315 | 316 | } | |
| 317 | + | ||
| 318 | + | func TestWalkWideTreeStaysWithinBudget(t *testing.T) { | |
| 319 | + | tr := newMocktransport(t) | |
| 320 | + | children := make([]ref, 4096) | |
| 321 | + | for i := range children { | |
| 322 | + | children[i] = ref{":1.5", fmt.Sprintf("/node%d", i)} | |
| 323 | + | } | |
| 324 | + | visited := 0 | |
| 325 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 326 | + | func(_ time.Time, _ time.Duration, calls []call) []reply { | |
| 327 | + | visited += len(calls) / 3 | |
| 328 | + | require.LessOrEqual(t, visited, maxWalkNodes) | |
| 329 | + | rep := make([]reply, len(calls)) | |
| 330 | + | for i, c := range calls { | |
| 331 | + | switch c.op { | |
| 332 | + | case opChildren: | |
| 333 | + | rep[i].refs = children | |
| 334 | + | case opRole: | |
| 335 | + | rep[i].u = roleLink | |
| 336 | + | case opState: | |
| 337 | + | rep[i].state = stShowing | 1<<stateVisible | |
| 338 | + | } | |
| 339 | + | } | |
| 340 | + | return rep | |
| 341 | + | }).Twice() | |
| 342 | + | s := &Source{tr: tr} | |
| 343 | + | // A full second level replies with another 4096 children each. There is | |
| 344 | + | // no remaining budget, so none of those millions of refs may be copied. | |
| 345 | + | var out []ref | |
| 346 | + | var before, after runtime.MemStats | |
| 347 | + | runtime.ReadMemStats(&before) | |
| 348 | + | out = s.walk(func(calls []call) []reply { return tr.Do(time.Time{}, time.Second, calls) }, ref{":1.5", "/frame"}, false) | |
| 349 | + | runtime.ReadMemStats(&after) | |
| 350 | + | require.Len(t, out, maxWalkNodes) | |
| 351 | + | require.Equal(t, maxWalkNodes, visited) | |
| 352 | + | // Normal replies and call batches take only a few MB; the old append- | |
| 353 | + | // then-truncate path copied hundreds of MB for this tree. | |
| 354 | + | require.Less(t, after.TotalAlloc-before.TotalAlloc, uint64(32<<20)) | |
| 355 | + | } | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "context" | 4 | "context" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "fmt" | 6 | "fmt" | ||
| 7 | + | "runtime" | |||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| @@ -313,3 +314,42 @@ func TestCloseReapsLateTransport(t *testing.T) { | |||||
| 313 | require.Empty(t, covered) | 314 | require.Empty(t, covered) | ||
| 314 | require.NoError(t, s.Close()) // idempotent | 315 | require.NoError(t, s.Close()) // idempotent | ||
| 315 | } | 316 | } | ||
| 317 | + | ||||
| 318 | + | func TestWalkWideTreeStaysWithinBudget(t *testing.T) { | |||
| 319 | + | tr := newMocktransport(t) | |||
| 320 | + | children := make([]ref, 4096) | |||
| 321 | + | for i := range children { | |||
| 322 | + | children[i] = ref{":1.5", fmt.Sprintf("/node%d", i)} | |||
| 323 | + | } | |||
| 324 | + | visited := 0 | |||
| 325 | + | tr.EXPECT().Do(mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |||
| 326 | + | func(_ time.Time, _ time.Duration, calls []call) []reply { | |||
| 327 | + | visited += len(calls) / 3 | |||
| 328 | + | require.LessOrEqual(t, visited, maxWalkNodes) | |||
| 329 | + | rep := make([]reply, len(calls)) | |||
| 330 | + | for i, c := range calls { | |||
| 331 | + | switch c.op { | |||
| 332 | + | case opChildren: | |||
| 333 | + | rep[i].refs = children | |||
| 334 | + | case opRole: | |||
| 335 | + | rep[i].u = roleLink | |||
| 336 | + | case opState: | |||
| 337 | + | rep[i].state = stShowing | 1<<stateVisible | |||
| 338 | + | } | |||
| 339 | + | } | |||
| 340 | + | return rep | |||
| 341 | + | }).Twice() | |||
| 342 | + | s := &Source{tr: tr} | |||
| 343 | + | // A full second level replies with another 4096 children each. There is | |||
| 344 | + | // no remaining budget, so none of those millions of refs may be copied. | |||
| 345 | + | var out []ref | |||
| 346 | + | var before, after runtime.MemStats | |||
| 347 | + | runtime.ReadMemStats(&before) | |||
| 348 | + | out = s.walk(func(calls []call) []reply { return tr.Do(time.Time{}, time.Second, calls) }, ref{":1.5", "/frame"}, false) | |||
| 349 | + | runtime.ReadMemStats(&after) | |||
| 350 | + | require.Len(t, out, maxWalkNodes) | |||
| 351 | + | require.Equal(t, maxWalkNodes, visited) | |||
| 352 | + | // Normal replies and call batches take only a few MB; the old append- | |||
| 353 | + | // then-truncate path copied hundreds of MB for this tree. | |||
| 354 | + | require.Less(t, after.TotalAlloc-before.TotalAlloc, uint64(32<<20)) | |||
| 355 | + | } | |||
A internal/adapters/hints/language_runner_mock_test.go +130 -0
| @@ -0,0 +1,130 @@ | |||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |
| 2 | + | // github.com/vektra/mockery | |
| 3 | + | // template: testify | |
| 4 | + | ||
| 5 | + | package hints | |
| 6 | + | ||
| 7 | + | import ( | |
| 8 | + | "context" | |
| 9 | + | ||
| 10 | + | mock "github.com/stretchr/testify/mock" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // newMocklanguageRunner creates a new instance of mocklanguageRunner. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 14 | + | // The first argument is typically a *testing.T value. | |
| 15 | + | func newMocklanguageRunner(t interface { | |
| 16 | + | mock.TestingT | |
| 17 | + | Cleanup(func()) | |
| 18 | + | }) *mocklanguageRunner { | |
| 19 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 20 | + | helper.Helper() | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | mock := &mocklanguageRunner{} | |
| 24 | + | mock.Mock.Test(t) | |
| 25 | + | ||
| 26 | + | t.Cleanup(func() { | |
| 27 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 28 | + | helper.Helper() | |
| 29 | + | } | |
| 30 | + | mock.AssertExpectations(t) | |
| 31 | + | }) | |
| 32 | + | ||
| 33 | + | return mock | |
| 34 | + | } | |
| 35 | + | ||
| 36 | + | // mocklanguageRunner is an autogenerated mock type for the languageRunner type | |
| 37 | + | type mocklanguageRunner struct { | |
| 38 | + | mock.Mock | |
| 39 | + | } | |
| 40 | + | ||
| 41 | + | type mocklanguageRunner_Expecter struct { | |
| 42 | + | mock *mock.Mock | |
| 43 | + | } | |
| 44 | + | ||
| 45 | + | func (_m *mocklanguageRunner) EXPECT() *mocklanguageRunner_Expecter { | |
| 46 | + | return &mocklanguageRunner_Expecter{mock: &_m.Mock} | |
| 47 | + | } | |
| 48 | + | ||
| 49 | + | // Output provides a mock function for the type mocklanguageRunner | |
| 50 | + | func (_mock *mocklanguageRunner) Output(context1 context.Context, s string, strings ...string) ([]byte, error) { | |
| 51 | + | var tmpRet mock.Arguments | |
| 52 | + | if len(strings) > 0 { | |
| 53 | + | tmpRet = _mock.Called(context1, s, strings) | |
| 54 | + | } else { | |
| 55 | + | tmpRet = _mock.Called(context1, s) | |
| 56 | + | } | |
| 57 | + | ret := tmpRet | |
| 58 | + | ||
| 59 | + | if len(ret) == 0 { | |
| 60 | + | panic("no return value specified for Output") | |
| 61 | + | } | |
| 62 | + | ||
| 63 | + | var r0 []byte | |
| 64 | + | var r1 error | |
| 65 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string, ...string) ([]byte, error)); ok { | |
| 66 | + | return returnFunc(context1, s, strings...) | |
| 67 | + | } | |
| 68 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string, ...string) []byte); ok { | |
| 69 | + | r0 = returnFunc(context1, s, strings...) | |
| 70 | + | } else { | |
| 71 | + | if ret.Get(0) != nil { | |
| 72 | + | r0 = ret.Get(0).([]byte) | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string, ...string) error); ok { | |
| 76 | + | r1 = returnFunc(context1, s, strings...) | |
| 77 | + | } else { | |
| 78 | + | r1 = ret.Error(1) | |
| 79 | + | } | |
| 80 | + | return r0, r1 | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | // mocklanguageRunner_Output_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Output' | |
| 84 | + | type mocklanguageRunner_Output_Call struct { | |
| 85 | + | *mock.Call | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | // Output is a helper method to define mock.On call | |
| 89 | + | // - context1 context.Context | |
| 90 | + | // - s string | |
| 91 | + | // - strings ...string | |
| 92 | + | func (_e *mocklanguageRunner_Expecter) Output(context1 any, s any, strings ...any) *mocklanguageRunner_Output_Call { | |
| 93 | + | return &mocklanguageRunner_Output_Call{Call: _e.mock.On("Output", | |
| 94 | + | append([]any{context1, s}, strings...)...)} | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | func (_c *mocklanguageRunner_Output_Call) Run(run func(context1 context.Context, s string, strings ...string)) *mocklanguageRunner_Output_Call { | |
| 98 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 99 | + | var arg0 context.Context | |
| 100 | + | if args[0] != nil { | |
| 101 | + | arg0 = args[0].(context.Context) | |
| 102 | + | } | |
| 103 | + | var arg1 string | |
| 104 | + | if args[1] != nil { | |
| 105 | + | arg1 = args[1].(string) | |
| 106 | + | } | |
| 107 | + | var arg2 []string | |
| 108 | + | var variadicArgs []string | |
| 109 | + | if len(args) > 2 { | |
| 110 | + | variadicArgs = args[2].([]string) | |
| 111 | + | } | |
| 112 | + | arg2 = variadicArgs | |
| 113 | + | run( | |
| 114 | + | arg0, | |
| 115 | + | arg1, | |
| 116 | + | arg2..., | |
| 117 | + | ) | |
| 118 | + | }) | |
| 119 | + | return _c | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | func (_c *mocklanguageRunner_Output_Call) Return(bytes []byte, err error) *mocklanguageRunner_Output_Call { | |
| 123 | + | _c.Call.Return(bytes, err) | |
| 124 | + | return _c | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | func (_c *mocklanguageRunner_Output_Call) RunAndReturn(run func(context1 context.Context, s string, strings ...string) ([]byte, error)) *mocklanguageRunner_Output_Call { | |
| 128 | + | _c.Call.Return(run) | |
| 129 | + | return _c | |
| 130 | + | } | |
| @@ -0,0 +1,130 @@ | |||||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |||
| 2 | + | // github.com/vektra/mockery | |||
| 3 | + | // template: testify | |||
| 4 | + | ||||
| 5 | + | package hints | |||
| 6 | + | ||||
| 7 | + | import ( | |||
| 8 | + | "context" | |||
| 9 | + | ||||
| 10 | + | mock "github.com/stretchr/testify/mock" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // newMocklanguageRunner creates a new instance of mocklanguageRunner. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |||
| 14 | + | // The first argument is typically a *testing.T value. | |||
| 15 | + | func newMocklanguageRunner(t interface { | |||
| 16 | + | mock.TestingT | |||
| 17 | + | Cleanup(func()) | |||
| 18 | + | }) *mocklanguageRunner { | |||
| 19 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 20 | + | helper.Helper() | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | mock := &mocklanguageRunner{} | |||
| 24 | + | mock.Mock.Test(t) | |||
| 25 | + | ||||
| 26 | + | t.Cleanup(func() { | |||
| 27 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 28 | + | helper.Helper() | |||
| 29 | + | } | |||
| 30 | + | mock.AssertExpectations(t) | |||
| 31 | + | }) | |||
| 32 | + | ||||
| 33 | + | return mock | |||
| 34 | + | } | |||
| 35 | + | ||||
| 36 | + | // mocklanguageRunner is an autogenerated mock type for the languageRunner type | |||
| 37 | + | type mocklanguageRunner struct { | |||
| 38 | + | mock.Mock | |||
| 39 | + | } | |||
| 40 | + | ||||
| 41 | + | type mocklanguageRunner_Expecter struct { | |||
| 42 | + | mock *mock.Mock | |||
| 43 | + | } | |||
| 44 | + | ||||
| 45 | + | func (_m *mocklanguageRunner) EXPECT() *mocklanguageRunner_Expecter { | |||
| 46 | + | return &mocklanguageRunner_Expecter{mock: &_m.Mock} | |||
| 47 | + | } | |||
| 48 | + | ||||
| 49 | + | // Output provides a mock function for the type mocklanguageRunner | |||
| 50 | + | func (_mock *mocklanguageRunner) Output(context1 context.Context, s string, strings ...string) ([]byte, error) { | |||
| 51 | + | var tmpRet mock.Arguments | |||
| 52 | + | if len(strings) > 0 { | |||
| 53 | + | tmpRet = _mock.Called(context1, s, strings) | |||
| 54 | + | } else { | |||
| 55 | + | tmpRet = _mock.Called(context1, s) | |||
| 56 | + | } | |||
| 57 | + | ret := tmpRet | |||
| 58 | + | ||||
| 59 | + | if len(ret) == 0 { | |||
| 60 | + | panic("no return value specified for Output") | |||
| 61 | + | } | |||
| 62 | + | ||||
| 63 | + | var r0 []byte | |||
| 64 | + | var r1 error | |||
| 65 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string, ...string) ([]byte, error)); ok { | |||
| 66 | + | return returnFunc(context1, s, strings...) | |||
| 67 | + | } | |||
| 68 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string, ...string) []byte); ok { | |||
| 69 | + | r0 = returnFunc(context1, s, strings...) | |||
| 70 | + | } else { | |||
| 71 | + | if ret.Get(0) != nil { | |||
| 72 | + | r0 = ret.Get(0).([]byte) | |||
| 73 | + | } | |||
| 74 | + | } | |||
| 75 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string, ...string) error); ok { | |||
| 76 | + | r1 = returnFunc(context1, s, strings...) | |||
| 77 | + | } else { | |||
| 78 | + | r1 = ret.Error(1) | |||
| 79 | + | } | |||
| 80 | + | return r0, r1 | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | // mocklanguageRunner_Output_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Output' | |||
| 84 | + | type mocklanguageRunner_Output_Call struct { | |||
| 85 | + | *mock.Call | |||
| 86 | + | } | |||
| 87 | + | ||||
| 88 | + | // Output is a helper method to define mock.On call | |||
| 89 | + | // - context1 context.Context | |||
| 90 | + | // - s string | |||
| 91 | + | // - strings ...string | |||
| 92 | + | func (_e *mocklanguageRunner_Expecter) Output(context1 any, s any, strings ...any) *mocklanguageRunner_Output_Call { | |||
| 93 | + | return &mocklanguageRunner_Output_Call{Call: _e.mock.On("Output", | |||
| 94 | + | append([]any{context1, s}, strings...)...)} | |||
| 95 | + | } | |||
| 96 | + | ||||
| 97 | + | func (_c *mocklanguageRunner_Output_Call) Run(run func(context1 context.Context, s string, strings ...string)) *mocklanguageRunner_Output_Call { | |||
| 98 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 99 | + | var arg0 context.Context | |||
| 100 | + | if args[0] != nil { | |||
| 101 | + | arg0 = args[0].(context.Context) | |||
| 102 | + | } | |||
| 103 | + | var arg1 string | |||
| 104 | + | if args[1] != nil { | |||
| 105 | + | arg1 = args[1].(string) | |||
| 106 | + | } | |||
| 107 | + | var arg2 []string | |||
| 108 | + | var variadicArgs []string | |||
| 109 | + | if len(args) > 2 { | |||
| 110 | + | variadicArgs = args[2].([]string) | |||
| 111 | + | } | |||
| 112 | + | arg2 = variadicArgs | |||
| 113 | + | run( | |||
| 114 | + | arg0, | |||
| 115 | + | arg1, | |||
| 116 | + | arg2..., | |||
| 117 | + | ) | |||
| 118 | + | }) | |||
| 119 | + | return _c | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | func (_c *mocklanguageRunner_Output_Call) Return(bytes []byte, err error) *mocklanguageRunner_Output_Call { | |||
| 123 | + | _c.Call.Return(bytes, err) | |||
| 124 | + | return _c | |||
| 125 | + | } | |||
| 126 | + | ||||
| 127 | + | func (_c *mocklanguageRunner_Output_Call) RunAndReturn(run func(context1 context.Context, s string, strings ...string) ([]byte, error)) *mocklanguageRunner_Output_Call { | |||
| 128 | + | _c.Call.Return(run) | |||
| 129 | + | return _c | |||
| 130 | + | } | |||
M internal/adapters/hints/ocr.go +21 -1
| @@ -41,7 +41,25 @@ type OCRWord struct { | |||
| 41 | 41 | var languageWarning sync.Once | |
| 42 | 42 | var failureWarning sync.Once | |
| 43 | 43 | ||
| 44 | + | // languageRunner bounds the external language-discovery process independently | |
| 45 | + | // of recognition, which has not started yet when discovery runs. | |
| 46 | + | type languageRunner interface { | |
| 47 | + | Output(context.Context, string, ...string) ([]byte, error) | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | type execLanguageRunner struct{} | |
| 51 | + | ||
| 52 | + | func (execLanguageRunner) Output(ctx context.Context, binary string, args ...string) ([]byte, error) { | |
| 53 | + | return exec.CommandContext(ctx, binary, args...).Output() | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | const languageTimeout = 5 * time.Second | |
| 57 | + | ||
| 44 | 58 | func ocrLanguage(ctx context.Context, binary string) (string, error) { | |
| 59 | + | return discoverLanguage(ctx, binary, execLanguageRunner{}, languageTimeout) | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | func discoverLanguage(ctx context.Context, binary string, runner languageRunner, timeout time.Duration) (string, error) { | |
| 45 | 63 | dir := os.Getenv("TESSDATA_PREFIX") | |
| 46 | 64 | if dir == "" { | |
| 47 | 65 | dir = "/usr/share/tessdata" | |
| @@ -49,7 +67,9 @@ func ocrLanguage(ctx context.Context, binary string) (string, error) { | |||
| 49 | 67 | if _, err := os.Stat(filepath.Join(dir, "eng.traineddata")); err == nil { | |
| 50 | 68 | return "eng", nil | |
| 51 | 69 | } | |
| 52 | - | out, err := exec.CommandContext(ctx, binary, "--list-langs").Output() | |
| 70 | + | ctx, cancel := context.WithTimeout(ctx, timeout) | |
| 71 | + | defer cancel() | |
| 72 | + | out, err := runner.Output(ctx, binary, "--list-langs") | |
| 53 | 73 | if err != nil { | |
| 54 | 74 | return "", err | |
| 55 | 75 | } | |
| @@ -41,7 +41,25 @@ type OCRWord struct { | |||||
| 41 | var languageWarning sync.Once | 41 | var languageWarning sync.Once | ||
| 42 | var failureWarning sync.Once | 42 | var failureWarning sync.Once | ||
| 43 | 43 | ||||
| 44 | + | // languageRunner bounds the external language-discovery process independently | |||
| 45 | + | // of recognition, which has not started yet when discovery runs. | |||
| 46 | + | type languageRunner interface { | |||
| 47 | + | Output(context.Context, string, ...string) ([]byte, error) | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | type execLanguageRunner struct{} | |||
| 51 | + | ||||
| 52 | + | func (execLanguageRunner) Output(ctx context.Context, binary string, args ...string) ([]byte, error) { | |||
| 53 | + | return exec.CommandContext(ctx, binary, args...).Output() | |||
| 54 | + | } | |||
| 55 | + | ||||
| 56 | + | const languageTimeout = 5 * time.Second | |||
| 57 | + | ||||
| 44 | func ocrLanguage(ctx context.Context, binary string) (string, error) { | 58 | func ocrLanguage(ctx context.Context, binary string) (string, error) { | ||
| 59 | + | return discoverLanguage(ctx, binary, execLanguageRunner{}, languageTimeout) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | func discoverLanguage(ctx context.Context, binary string, runner languageRunner, timeout time.Duration) (string, error) { | |||
| 45 | dir := os.Getenv("TESSDATA_PREFIX") | 63 | dir := os.Getenv("TESSDATA_PREFIX") | ||
| 46 | if dir == "" { | 64 | if dir == "" { | ||
| 47 | dir = "/usr/share/tessdata" | 65 | dir = "/usr/share/tessdata" | ||
| @@ -49,7 +67,9 @@ func ocrLanguage(ctx context.Context, binary string) (string, error) { | |||||
| 49 | if _, err := os.Stat(filepath.Join(dir, "eng.traineddata")); err == nil { | 67 | if _, err := os.Stat(filepath.Join(dir, "eng.traineddata")); err == nil { | ||
| 50 | return "eng", nil | 68 | return "eng", nil | ||
| 51 | } | 69 | } | ||
| 52 | - | out, err := exec.CommandContext(ctx, binary, "--list-langs").Output() | 70 | + | ctx, cancel := context.WithTimeout(ctx, timeout) |
| 71 | + | defer cancel() | |||
| 72 | + | out, err := runner.Output(ctx, binary, "--list-langs") | |||
| 53 | if err != nil { | 73 | if err != nil { | ||
| 54 | return "", err | 74 | return "", err | ||
| 55 | } | 75 | } | ||
M internal/adapters/hints/ocr_test.go +34 -0
| @@ -2,6 +2,7 @@ package hints | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "image" | |
| 6 | 7 | "image/draw" | |
| 7 | 8 | "image/png" | |
| @@ -14,6 +15,8 @@ import ( | |||
| 14 | 15 | "testing" | |
| 15 | 16 | "time" | |
| 16 | 17 | ||
| 18 | + | "github.com/stretchr/testify/mock" | |
| 19 | + | ||
| 17 | 20 | "github.com/bnema/neferwl/internal/ports" | |
| 18 | 21 | ) | |
| 19 | 22 | ||
| @@ -236,3 +239,34 @@ func TestRecognizeCapsProcesses(t *testing.T) { | |||
| 236 | 239 | } | |
| 237 | 240 | } | |
| 238 | 241 | } | |
| 242 | + | ||
| 243 | + | func TestLanguageDiscoveryTimeout(t *testing.T) { | |
| 244 | + | t.Setenv("TESSDATA_PREFIX", t.TempDir()) | |
| 245 | + | runner := newMocklanguageRunner(t) | |
| 246 | + | parent := context.Background() | |
| 247 | + | runner.EXPECT().Output(mock.Anything, "tesseract", []string{"--list-langs"}).RunAndReturn( | |
| 248 | + | func(ctx context.Context, _ string, _ ...string) ([]byte, error) { | |
| 249 | + | deadline, ok := ctx.Deadline() | |
| 250 | + | if !ok || time.Until(deadline) > languageTimeout { | |
| 251 | + | t.Fatal("language discovery has no bounded deadline") | |
| 252 | + | } | |
| 253 | + | <-ctx.Done() | |
| 254 | + | return nil, ctx.Err() | |
| 255 | + | }).Once() | |
| 256 | + | lang, err := discoverLanguage(parent, "tesseract", runner, 0) | |
| 257 | + | if lang != "" || !errors.Is(err, context.DeadlineExceeded) || parent.Err() != nil { | |
| 258 | + | t.Fatalf("language=%q error=%v parent=%v", lang, err, parent.Err()) | |
| 259 | + | } | |
| 260 | + | } | |
| 261 | + | ||
| 262 | + | func TestLanguageDiscoveryFailureKeepsAccessibilityOnly(t *testing.T) { | |
| 263 | + | w := &worker{ocr: ocrState{done: true, err: context.DeadlineExceeded}} | |
| 264 | + | ar := atspiResult{targets: []ports.HintTarget{{Window: 1, Rect: ports.Rect{W: 20, H: 10}}}} | |
| 265 | + | req := ports.HintScanRequest{Width: 100, Height: 100, Scale: 1, Windows: []ports.HintWindow{{ID: 1, Rect: ports.Rect{W: 100, H: 100}}}} | |
| 266 | + | if w.needFrame(context.Background(), req, ar) { | |
| 267 | + | t.Fatal("timed out discovery must not require an OCR capture") | |
| 268 | + | } | |
| 269 | + | if err := w.ocr.resolve(context.Background()); !errors.Is(err, context.DeadlineExceeded) { | |
| 270 | + | t.Fatalf("discovery failure was not retained: %v", err) | |
| 271 | + | } | |
| 272 | + | } | |
| @@ -2,6 +2,7 @@ package hints | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 5 | "image" | 6 | "image" | ||
| 6 | "image/draw" | 7 | "image/draw" | ||
| 7 | "image/png" | 8 | "image/png" | ||
| @@ -14,6 +15,8 @@ import ( | |||||
| 14 | "testing" | 15 | "testing" | ||
| 15 | "time" | 16 | "time" | ||
| 16 | 17 | ||||
| 18 | + | "github.com/stretchr/testify/mock" | |||
| 19 | + | ||||
| 17 | "github.com/bnema/neferwl/internal/ports" | 20 | "github.com/bnema/neferwl/internal/ports" | ||
| 18 | ) | 21 | ) | ||
| 19 | 22 | ||||
| @@ -236,3 +239,34 @@ func TestRecognizeCapsProcesses(t *testing.T) { | |||||
| 236 | } | 239 | } | ||
| 237 | } | 240 | } | ||
| 238 | } | 241 | } | ||
| 242 | + | ||||
| 243 | + | func TestLanguageDiscoveryTimeout(t *testing.T) { | |||
| 244 | + | t.Setenv("TESSDATA_PREFIX", t.TempDir()) | |||
| 245 | + | runner := newMocklanguageRunner(t) | |||
| 246 | + | parent := context.Background() | |||
| 247 | + | runner.EXPECT().Output(mock.Anything, "tesseract", []string{"--list-langs"}).RunAndReturn( | |||
| 248 | + | func(ctx context.Context, _ string, _ ...string) ([]byte, error) { | |||
| 249 | + | deadline, ok := ctx.Deadline() | |||
| 250 | + | if !ok || time.Until(deadline) > languageTimeout { | |||
| 251 | + | t.Fatal("language discovery has no bounded deadline") | |||
| 252 | + | } | |||
| 253 | + | <-ctx.Done() | |||
| 254 | + | return nil, ctx.Err() | |||
| 255 | + | }).Once() | |||
| 256 | + | lang, err := discoverLanguage(parent, "tesseract", runner, 0) | |||
| 257 | + | if lang != "" || !errors.Is(err, context.DeadlineExceeded) || parent.Err() != nil { | |||
| 258 | + | t.Fatalf("language=%q error=%v parent=%v", lang, err, parent.Err()) | |||
| 259 | + | } | |||
| 260 | + | } | |||
| 261 | + | ||||
| 262 | + | func TestLanguageDiscoveryFailureKeepsAccessibilityOnly(t *testing.T) { | |||
| 263 | + | w := &worker{ocr: ocrState{done: true, err: context.DeadlineExceeded}} | |||
| 264 | + | ar := atspiResult{targets: []ports.HintTarget{{Window: 1, Rect: ports.Rect{W: 20, H: 10}}}} | |||
| 265 | + | req := ports.HintScanRequest{Width: 100, Height: 100, Scale: 1, Windows: []ports.HintWindow{{ID: 1, Rect: ports.Rect{W: 100, H: 100}}}} | |||
| 266 | + | if w.needFrame(context.Background(), req, ar) { | |||
| 267 | + | t.Fatal("timed out discovery must not require an OCR capture") | |||
| 268 | + | } | |||
| 269 | + | if err := w.ocr.resolve(context.Background()); !errors.Is(err, context.DeadlineExceeded) { | |||
| 270 | + | t.Fatalf("discovery failure was not retained: %v", err) | |||
| 271 | + | } | |||
| 272 | + | } | |||
M internal/core/flash.go +35 -33
| @@ -87,30 +87,9 @@ func (c *Core) flashTick() bool { | |||
| 87 | 87 | f.timerC, f.timerStop = newTimer(c.opts.Clock, wait) | |
| 88 | 88 | return false | |
| 89 | 89 | } | |
| 90 | - | layout := sc.mon.Layout() | |
| 91 | - | var rect Rect | |
| 92 | - | if target.level == flashMonitor { | |
| 93 | - | o := sc.mon.Output() | |
| 94 | - | rect = Rect{W: o.W, H: o.H} | |
| 95 | - | } else { | |
| 96 | - | at := slices.IndexFunc(layout, func(p Placement) bool { return p.ID == target.window && !p.Hidden && p.Preview == 0 }) | |
| 97 | - | if at < 0 || layout[at].Fullscreen { | |
| 98 | - | return false | |
| 99 | - | } | |
| 100 | - | rect = layout[at].Rect | |
| 101 | - | if target.level == flashColumn { | |
| 102 | - | w := sc.mon.Current() | |
| 103 | - | col := w.columnOf(target.window) | |
| 104 | - | if col < 0 { | |
| 105 | - | return false | |
| 106 | - | } | |
| 107 | - | for _, p := range layout { | |
| 108 | - | if p.Hidden || !slices.Contains(w.Columns[col].Windows, p.ID) { | |
| 109 | - | continue | |
| 110 | - | } | |
| 111 | - | rect = union(rect, p.Rect) | |
| 112 | - | } | |
| 113 | - | } | |
| 90 | + | rect, ok := c.flashRect(target, sc) | |
| 91 | + | if !ok { | |
| 92 | + | return false | |
| 114 | 93 | } | |
| 115 | 94 | if at, ok := f.lastStarts[target]; ok && now.Sub(at) < flashCooldown { | |
| 116 | 95 | return false | |
| @@ -139,7 +118,7 @@ func (c *Core) advanceFlash(only *screen) { | |||
| 139 | 118 | return | |
| 140 | 119 | } | |
| 141 | 120 | sc := c.screens[c.screenIndex(f.running.output)] | |
| 142 | - | if !c.flashVisible(*f.running, sc) { | |
| 121 | + | if rect, ok := c.flashRect(*f.running, sc); !ok || rect != f.rect { | |
| 143 | 122 | c.stopFlash() | |
| 144 | 123 | return | |
| 145 | 124 | } | |
| @@ -156,27 +135,50 @@ func (c *Core) advanceFlash(only *screen) { | |||
| 156 | 135 | f.value = c.cfg.Focus.FlashOpacity * (1 - float64(max(elapsed, 0))/float64(d)) | |
| 157 | 136 | } | |
| 158 | 137 | ||
| 159 | - | // flashVisible checks the current layout, not leaving placements retained for | |
| 160 | - | // transitions: an old border must not outlive its target's visibility. | |
| 161 | - | func (c *Core) flashVisible(target flashTarget, sc *screen) bool { | |
| 138 | + | // flashRect resolves the current target, never retaining a border after its | |
| 139 | + | // visibility or geometry changes. stopFlash leaves the published sample intact | |
| 140 | + | // so the scene diff also damages the old border. | |
| 141 | + | func (c *Core) flashRect(target flashTarget, sc *screen) (Rect, bool) { | |
| 162 | 142 | if sc.mon.ov.open { | |
| 163 | - | return false | |
| 143 | + | return Rect{}, false | |
| 164 | 144 | } | |
| 165 | 145 | if target.level == flashMonitor { | |
| 166 | - | return true | |
| 146 | + | o := sc.mon.Output() | |
| 147 | + | return Rect{W: o.W, H: o.H}, true | |
| 167 | 148 | } | |
| 168 | 149 | owner, _ := c.screenOf(target.window) | |
| 169 | 150 | if owner != sc || c.windows.lookup(target.window).client.ID == 0 { | |
| 170 | - | return false | |
| 151 | + | return Rect{}, false | |
| 171 | 152 | } | |
| 172 | 153 | view := sc.mon.viewport() | |
| 173 | 154 | c.flash.layout = sc.mon.layoutInto(c.flash.layout) | |
| 155 | + | var rect Rect | |
| 156 | + | found := false | |
| 174 | 157 | for _, p := range c.flash.layout { | |
| 175 | 158 | if p.ID == target.window { | |
| 176 | - | return !p.Fullscreen && p.Preview == 0 && view.shows(p) | |
| 159 | + | if p.Fullscreen || p.Preview != 0 || !view.shows(p) { | |
| 160 | + | return Rect{}, false | |
| 161 | + | } | |
| 162 | + | rect, found = p.Rect, true | |
| 163 | + | break | |
| 164 | + | } | |
| 165 | + | } | |
| 166 | + | if !found { | |
| 167 | + | return Rect{}, false | |
| 168 | + | } | |
| 169 | + | if target.level == flashColumn { | |
| 170 | + | w := sc.mon.Current() | |
| 171 | + | col := w.columnOf(target.window) | |
| 172 | + | if col < 0 { | |
| 173 | + | return Rect{}, false | |
| 174 | + | } | |
| 175 | + | for _, p := range c.flash.layout { | |
| 176 | + | if !p.Hidden && slices.Contains(w.Columns[col].Windows, p.ID) { | |
| 177 | + | rect = union(rect, p.Rect) | |
| 178 | + | } | |
| 177 | 179 | } | |
| 178 | 180 | } | |
| 179 | - | return false | |
| 181 | + | return rect, true | |
| 180 | 182 | } | |
| 181 | 183 | ||
| 182 | 184 | func (c *Core) flashFor(sc *screen) *ports.FocusFlash { | |
| @@ -87,30 +87,9 @@ func (c *Core) flashTick() bool { | |||||
| 87 | f.timerC, f.timerStop = newTimer(c.opts.Clock, wait) | 87 | f.timerC, f.timerStop = newTimer(c.opts.Clock, wait) | ||
| 88 | return false | 88 | return false | ||
| 89 | } | 89 | } | ||
| 90 | - | layout := sc.mon.Layout() | 90 | + | rect, ok := c.flashRect(target, sc) |
| 91 | - | var rect Rect | 91 | + | if !ok { |
| 92 | - | if target.level == flashMonitor { | 92 | + | return false |
| 93 | - | o := sc.mon.Output() | |||
| 94 | - | rect = Rect{W: o.W, H: o.H} | |||
| 95 | - | } else { | |||
| 96 | - | at := slices.IndexFunc(layout, func(p Placement) bool { return p.ID == target.window && !p.Hidden && p.Preview == 0 }) | |||
| 97 | - | if at < 0 || layout[at].Fullscreen { | |||
| 98 | - | return false | |||
| 99 | - | } | |||
| 100 | - | rect = layout[at].Rect | |||
| 101 | - | if target.level == flashColumn { | |||
| 102 | - | w := sc.mon.Current() | |||
| 103 | - | col := w.columnOf(target.window) | |||
| 104 | - | if col < 0 { | |||
| 105 | - | return false | |||
| 106 | - | } | |||
| 107 | - | for _, p := range layout { | |||
| 108 | - | if p.Hidden || !slices.Contains(w.Columns[col].Windows, p.ID) { | |||
| 109 | - | continue | |||
| 110 | - | } | |||
| 111 | - | rect = union(rect, p.Rect) | |||
| 112 | - | } | |||
| 113 | - | } | |||
| 114 | } | 93 | } | ||
| 115 | if at, ok := f.lastStarts[target]; ok && now.Sub(at) < flashCooldown { | 94 | if at, ok := f.lastStarts[target]; ok && now.Sub(at) < flashCooldown { | ||
| 116 | return false | 95 | return false | ||
| @@ -139,7 +118,7 @@ func (c *Core) advanceFlash(only *screen) { | |||||
| 139 | return | 118 | return | ||
| 140 | } | 119 | } | ||
| 141 | sc := c.screens[c.screenIndex(f.running.output)] | 120 | sc := c.screens[c.screenIndex(f.running.output)] | ||
| 142 | - | if !c.flashVisible(*f.running, sc) { | 121 | + | if rect, ok := c.flashRect(*f.running, sc); !ok || rect != f.rect { |
| 143 | c.stopFlash() | 122 | c.stopFlash() | ||
| 144 | return | 123 | return | ||
| 145 | } | 124 | } | ||
| @@ -156,27 +135,50 @@ func (c *Core) advanceFlash(only *screen) { | |||||
| 156 | f.value = c.cfg.Focus.FlashOpacity * (1 - float64(max(elapsed, 0))/float64(d)) | 135 | f.value = c.cfg.Focus.FlashOpacity * (1 - float64(max(elapsed, 0))/float64(d)) | ||
| 157 | } | 136 | } | ||
| 158 | 137 | ||||
| 159 | - | // flashVisible checks the current layout, not leaving placements retained for | 138 | + | // flashRect resolves the current target, never retaining a border after its |
| 160 | - | // transitions: an old border must not outlive its target's visibility. | 139 | + | // visibility or geometry changes. stopFlash leaves the published sample intact |
| 161 | - | func (c *Core) flashVisible(target flashTarget, sc *screen) bool { | 140 | + | // so the scene diff also damages the old border. |
| 141 | + | func (c *Core) flashRect(target flashTarget, sc *screen) (Rect, bool) { | |||
| 162 | if sc.mon.ov.open { | 142 | if sc.mon.ov.open { | ||
| 163 | - | return false | 143 | + | return Rect{}, false |
| 164 | } | 144 | } | ||
| 165 | if target.level == flashMonitor { | 145 | if target.level == flashMonitor { | ||
| 166 | - | return true | 146 | + | o := sc.mon.Output() |
| 147 | + | return Rect{W: o.W, H: o.H}, true | |||
| 167 | } | 148 | } | ||
| 168 | owner, _ := c.screenOf(target.window) | 149 | owner, _ := c.screenOf(target.window) | ||
| 169 | if owner != sc || c.windows.lookup(target.window).client.ID == 0 { | 150 | if owner != sc || c.windows.lookup(target.window).client.ID == 0 { | ||
| 170 | - | return false | 151 | + | return Rect{}, false |
| 171 | } | 152 | } | ||
| 172 | view := sc.mon.viewport() | 153 | view := sc.mon.viewport() | ||
| 173 | c.flash.layout = sc.mon.layoutInto(c.flash.layout) | 154 | c.flash.layout = sc.mon.layoutInto(c.flash.layout) | ||
| 155 | + | var rect Rect | |||
| 156 | + | found := false | |||
| 174 | for _, p := range c.flash.layout { | 157 | for _, p := range c.flash.layout { | ||
| 175 | if p.ID == target.window { | 158 | if p.ID == target.window { | ||
| 176 | - | return !p.Fullscreen && p.Preview == 0 && view.shows(p) | 159 | + | if p.Fullscreen || p.Preview != 0 || !view.shows(p) { |
| 160 | + | return Rect{}, false | |||
| 161 | + | } | |||
| 162 | + | rect, found = p.Rect, true | |||
| 163 | + | break | |||
| 164 | + | } | |||
| 165 | + | } | |||
| 166 | + | if !found { | |||
| 167 | + | return Rect{}, false | |||
| 168 | + | } | |||
| 169 | + | if target.level == flashColumn { | |||
| 170 | + | w := sc.mon.Current() | |||
| 171 | + | col := w.columnOf(target.window) | |||
| 172 | + | if col < 0 { | |||
| 173 | + | return Rect{}, false | |||
| 174 | + | } | |||
| 175 | + | for _, p := range c.flash.layout { | |||
| 176 | + | if !p.Hidden && slices.Contains(w.Columns[col].Windows, p.ID) { | |||
| 177 | + | rect = union(rect, p.Rect) | |||
| 178 | + | } | |||
| 177 | } | 179 | } | ||
| 178 | } | 180 | } | ||
| 179 | - | return false | 181 | + | return rect, true |
| 180 | } | 182 | } | ||
| 181 | 183 | ||||
| 182 | func (c *Core) flashFor(sc *screen) *ports.FocusFlash { | 184 | func (c *Core) flashFor(sc *screen) *ports.FocusFlash { | ||
M internal/core/flash_test.go +47 -0
| @@ -670,3 +670,50 @@ func TestFlashSameScreenColumnFocusKeepsColumnLevel(t *testing.T) { | |||
| 670 | 670 | } | |
| 671 | 671 | } | |
| 672 | 672 | } | |
| 673 | + | ||
| 674 | + | func TestFlashCancelsOnGeometryChange(t *testing.T) { | |
| 675 | + | for _, level := range []flashLevel{flashWindow, flashColumn, flashMonitor} { | |
| 676 | + | for _, change := range []string{"resize", "new window"} { | |
| 677 | + | t.Run(string(rune('0'+level))+"/"+change, func(t *testing.T) { | |
| 678 | + | c, clock := flashCore(t) | |
| 679 | + | startFlash(t, c, clock, level, 3) | |
| 680 | + | before := indicatorScene(t, c) | |
| 681 | + | if before.Flash == nil { | |
| 682 | + | t.Fatal("setup: no flash") | |
| 683 | + | } | |
| 684 | + | if level == flashMonitor { | |
| 685 | + | c.cur().mon.SetOutput(400, 300) | |
| 686 | + | } else if change == "resize" { | |
| 687 | + | c.cur().mon.Current().Apply(ActionCycleColumnWidth) | |
| 688 | + | } else { | |
| 689 | + | c.cur().mon.AddWindow(4) | |
| 690 | + | c.windows.mapped(ports.WindowMapped{ID: 4}, clock.now) | |
| 691 | + | } | |
| 692 | + | after := indicatorScene(t, c) | |
| 693 | + | if after.Flash != nil || c.flash.running != nil { | |
| 694 | + | t.Fatal("stale geometry retained") | |
| 695 | + | } | |
| 696 | + | if before.Flash.Rect != c.flash.published.Rect { | |
| 697 | + | t.Fatal("old flash sample mutated") | |
| 698 | + | } | |
| 699 | + | }) | |
| 700 | + | } | |
| 701 | + | } | |
| 702 | + | } | |
| 703 | + | ||
| 704 | + | func TestFlashGeometryRevalidationAllocations(t *testing.T) { | |
| 705 | + | for _, level := range []flashLevel{flashWindow, flashColumn, flashMonitor} { | |
| 706 | + | t.Run(string(rune('0'+level)), func(t *testing.T) { | |
| 707 | + | c, clock := flashCore(t) | |
| 708 | + | startFlash(t, c, clock, level, 3) | |
| 709 | + | // Measure production work, not the generated clock mock's call log. | |
| 710 | + | c.opts.Clock = nil | |
| 711 | + | c.flash.start = time.Now() | |
| 712 | + | c.cfg.Focus.FlashDuration = time.Hour | |
| 713 | + | c.advanceFlash(nil) // warm owner-owned layout scratch | |
| 714 | + | if n := testing.AllocsPerRun(100, func() { c.advanceFlash(nil) }); n != 0 { | |
| 715 | + | t.Fatalf("flash geometry validation allocates %v times per frame", n) | |
| 716 | + | } | |
| 717 | + | }) | |
| 718 | + | } | |
| 719 | + | } | |
| @@ -670,3 +670,50 @@ func TestFlashSameScreenColumnFocusKeepsColumnLevel(t *testing.T) { | |||||
| 670 | } | 670 | } | ||
| 671 | } | 671 | } | ||
| 672 | } | 672 | } | ||
| 673 | + | ||||
| 674 | + | func TestFlashCancelsOnGeometryChange(t *testing.T) { | |||
| 675 | + | for _, level := range []flashLevel{flashWindow, flashColumn, flashMonitor} { | |||
| 676 | + | for _, change := range []string{"resize", "new window"} { | |||
| 677 | + | t.Run(string(rune('0'+level))+"/"+change, func(t *testing.T) { | |||
| 678 | + | c, clock := flashCore(t) | |||
| 679 | + | startFlash(t, c, clock, level, 3) | |||
| 680 | + | before := indicatorScene(t, c) | |||
| 681 | + | if before.Flash == nil { | |||
| 682 | + | t.Fatal("setup: no flash") | |||
| 683 | + | } | |||
| 684 | + | if level == flashMonitor { | |||
| 685 | + | c.cur().mon.SetOutput(400, 300) | |||
| 686 | + | } else if change == "resize" { | |||
| 687 | + | c.cur().mon.Current().Apply(ActionCycleColumnWidth) | |||
| 688 | + | } else { | |||
| 689 | + | c.cur().mon.AddWindow(4) | |||
| 690 | + | c.windows.mapped(ports.WindowMapped{ID: 4}, clock.now) | |||
| 691 | + | } | |||
| 692 | + | after := indicatorScene(t, c) | |||
| 693 | + | if after.Flash != nil || c.flash.running != nil { | |||
| 694 | + | t.Fatal("stale geometry retained") | |||
| 695 | + | } | |||
| 696 | + | if before.Flash.Rect != c.flash.published.Rect { | |||
| 697 | + | t.Fatal("old flash sample mutated") | |||
| 698 | + | } | |||
| 699 | + | }) | |||
| 700 | + | } | |||
| 701 | + | } | |||
| 702 | + | } | |||
| 703 | + | ||||
| 704 | + | func TestFlashGeometryRevalidationAllocations(t *testing.T) { | |||
| 705 | + | for _, level := range []flashLevel{flashWindow, flashColumn, flashMonitor} { | |||
| 706 | + | t.Run(string(rune('0'+level)), func(t *testing.T) { | |||
| 707 | + | c, clock := flashCore(t) | |||
| 708 | + | startFlash(t, c, clock, level, 3) | |||
| 709 | + | // Measure production work, not the generated clock mock's call log. | |||
| 710 | + | c.opts.Clock = nil | |||
| 711 | + | c.flash.start = time.Now() | |||
| 712 | + | c.cfg.Focus.FlashDuration = time.Hour | |||
| 713 | + | c.advanceFlash(nil) // warm owner-owned layout scratch | |||
| 714 | + | if n := testing.AllocsPerRun(100, func() { c.advanceFlash(nil) }); n != 0 { | |||
| 715 | + | t.Fatalf("flash geometry validation allocates %v times per frame", n) | |||
| 716 | + | } | |||
| 717 | + | }) | |||
| 718 | + | } | |||
| 719 | + | } | |||
M internal/core/hint.go +26 -16
| @@ -33,11 +33,17 @@ const ( | |||
| 33 | 33 | type hintTarget struct { | |
| 34 | 34 | x, y float64 | |
| 35 | 35 | window WindowID | |
| 36 | + | source hintSourceGeom // source window and its request-time client geometry | |
| 36 | 37 | label string | |
| 37 | 38 | rect ports.Rect | |
| 38 | 39 | text bool // found by OCR | |
| 39 | 40 | } | |
| 40 | 41 | ||
| 42 | + | type hintSourceGeom struct { | |
| 43 | + | id WindowID | |
| 44 | + | rect ports.Rect | |
| 45 | + | } | |
| 46 | + | ||
| 41 | 47 | type hintState struct { | |
| 42 | 48 | phase hintPhase | |
| 43 | 49 | seq uint64 // last request sequence number | |
| @@ -161,10 +167,11 @@ func (c *Core) hintResult(res ports.HintScanResult) bool { | |||
| 161 | 167 | // A layer surface, or a window of another output. | |
| 162 | 168 | continue | |
| 163 | 169 | } | |
| 164 | - | if !c.hintOwned(sc, t, root) { | |
| 170 | + | source, ok := c.hintSource(sc, t, root) | |
| 171 | + | if !ok { | |
| 165 | 172 | continue | |
| 166 | 173 | } | |
| 167 | - | kept = append(kept, hintTarget{x: x, y: y, window: id, rect: t.Rect, text: t.Text}) | |
| 174 | + | kept = append(kept, hintTarget{x: x, y: y, window: id, source: source, rect: t.Rect, text: t.Text}) | |
| 168 | 175 | } | |
| 169 | 176 | if len(kept) == 0 { | |
| 170 | 177 | c.hintCancel() | |
| @@ -188,22 +195,24 @@ func (c *Core) hintResult(res ports.HintScanResult) bool { | |||
| 188 | 195 | return true | |
| 189 | 196 | } | |
| 190 | 197 | ||
| 191 | - | // hintOwned reports whether the window under a target is the one it was | |
| 192 | - | // scanned on. An accessibility target names its window; an OCR target only | |
| 193 | - | // counts over a window that was visible when the scan was requested and has | |
| 194 | - | // not moved or resized since. | |
| 195 | - | func (c *Core) hintOwned(sc *screen, t ports.HintTarget, root WindowID) bool { | |
| 196 | - | if t.Window != 0 { | |
| 197 | - | return root == t.Window | |
| 198 | + | // hintSource resolves the request-time source window only while its client | |
| 199 | + | // geometry is unchanged. This applies equally to accessibility and OCR targets. | |
| 200 | + | func (c *Core) hintSource(sc *screen, t ports.HintTarget, root WindowID) (hintSourceGeom, bool) { | |
| 201 | + | if t.Window != 0 && root != t.Window { | |
| 202 | + | return hintSourceGeom{}, false | |
| 198 | 203 | } | |
| 199 | 204 | for _, w := range c.hint.windows { | |
| 200 | - | if w.ID != root { | |
| 201 | - | continue | |
| 205 | + | if w.ID == root && c.hintSourceUnchanged(sc, hintSourceGeom{id: w.ID, rect: w.Rect}) { | |
| 206 | + | return hintSourceGeom{id: w.ID, rect: w.Rect}, true | |
| 202 | 207 | } | |
| 203 | - | for _, p := range sc.shownLayout() { | |
| 204 | - | if p.ID == root { | |
| 205 | - | return c.clientRect(p) == w.Rect | |
| 206 | - | } | |
| 208 | + | } | |
| 209 | + | return hintSourceGeom{}, false | |
| 210 | + | } | |
| 211 | + | ||
| 212 | + | func (c *Core) hintSourceUnchanged(sc *screen, w hintSourceGeom) bool { | |
| 213 | + | for _, p := range sc.shownLayout() { | |
| 214 | + | if p.ID == w.id { | |
| 215 | + | return c.clientRect(p) == w.rect | |
| 207 | 216 | } | |
| 208 | 217 | } | |
| 209 | 218 | return false | |
| @@ -340,7 +349,8 @@ func (c *Core) hintClickable(t hintTarget) bool { | |||
| 340 | 349 | return false | |
| 341 | 350 | } | |
| 342 | 351 | id, _, _ := c.hit(t.x, t.y) | |
| 343 | - | return id == t.window | |
| 352 | + | sc, _ := c.screenOf(t.source.id) | |
| 353 | + | return id == t.window && sc != nil && c.hintSourceUnchanged(sc, t.source) | |
| 344 | 354 | } | |
| 345 | 355 | ||
| 346 | 356 | // hintLabels are the labels to draw on sc: those still matching what was | |
| @@ -33,11 +33,17 @@ const ( | |||||
| 33 | type hintTarget struct { | 33 | type hintTarget struct { | ||
| 34 | x, y float64 | 34 | x, y float64 | ||
| 35 | window WindowID | 35 | window WindowID | ||
| 36 | + | source hintSourceGeom // source window and its request-time client geometry | |||
| 36 | label string | 37 | label string | ||
| 37 | rect ports.Rect | 38 | rect ports.Rect | ||
| 38 | text bool // found by OCR | 39 | text bool // found by OCR | ||
| 39 | } | 40 | } | ||
| 40 | 41 | ||||
| 42 | + | type hintSourceGeom struct { | |||
| 43 | + | id WindowID | |||
| 44 | + | rect ports.Rect | |||
| 45 | + | } | |||
| 46 | + | ||||
| 41 | type hintState struct { | 47 | type hintState struct { | ||
| 42 | phase hintPhase | 48 | phase hintPhase | ||
| 43 | seq uint64 // last request sequence number | 49 | seq uint64 // last request sequence number | ||
| @@ -161,10 +167,11 @@ func (c *Core) hintResult(res ports.HintScanResult) bool { | |||||
| 161 | // A layer surface, or a window of another output. | 167 | // A layer surface, or a window of another output. | ||
| 162 | continue | 168 | continue | ||
| 163 | } | 169 | } | ||
| 164 | - | if !c.hintOwned(sc, t, root) { | 170 | + | source, ok := c.hintSource(sc, t, root) |
| 171 | + | if !ok { | |||
| 165 | continue | 172 | continue | ||
| 166 | } | 173 | } | ||
| 167 | - | kept = append(kept, hintTarget{x: x, y: y, window: id, rect: t.Rect, text: t.Text}) | 174 | + | kept = append(kept, hintTarget{x: x, y: y, window: id, source: source, rect: t.Rect, text: t.Text}) |
| 168 | } | 175 | } | ||
| 169 | if len(kept) == 0 { | 176 | if len(kept) == 0 { | ||
| 170 | c.hintCancel() | 177 | c.hintCancel() | ||
| @@ -188,22 +195,24 @@ func (c *Core) hintResult(res ports.HintScanResult) bool { | |||||
| 188 | return true | 195 | return true | ||
| 189 | } | 196 | } | ||
| 190 | 197 | ||||
| 191 | - | // hintOwned reports whether the window under a target is the one it was | 198 | + | // hintSource resolves the request-time source window only while its client |
| 192 | - | // scanned on. An accessibility target names its window; an OCR target only | 199 | + | // geometry is unchanged. This applies equally to accessibility and OCR targets. |
| 193 | - | // counts over a window that was visible when the scan was requested and has | 200 | + | func (c *Core) hintSource(sc *screen, t ports.HintTarget, root WindowID) (hintSourceGeom, bool) { |
| 194 | - | // not moved or resized since. | 201 | + | if t.Window != 0 && root != t.Window { |
| 195 | - | func (c *Core) hintOwned(sc *screen, t ports.HintTarget, root WindowID) bool { | 202 | + | return hintSourceGeom{}, false |
| 196 | - | if t.Window != 0 { | |||
| 197 | - | return root == t.Window | |||
| 198 | } | 203 | } | ||
| 199 | for _, w := range c.hint.windows { | 204 | for _, w := range c.hint.windows { | ||
| 200 | - | if w.ID != root { | 205 | + | if w.ID == root && c.hintSourceUnchanged(sc, hintSourceGeom{id: w.ID, rect: w.Rect}) { |
| 201 | - | continue | 206 | + | return hintSourceGeom{id: w.ID, rect: w.Rect}, true |
| 202 | } | 207 | } | ||
| 203 | - | for _, p := range sc.shownLayout() { | 208 | + | } |
| 204 | - | if p.ID == root { | 209 | + | return hintSourceGeom{}, false |
| 205 | - | return c.clientRect(p) == w.Rect | 210 | + | } |
| 206 | - | } | 211 | + | |
| 212 | + | func (c *Core) hintSourceUnchanged(sc *screen, w hintSourceGeom) bool { | |||
| 213 | + | for _, p := range sc.shownLayout() { | |||
| 214 | + | if p.ID == w.id { | |||
| 215 | + | return c.clientRect(p) == w.rect | |||
| 207 | } | 216 | } | ||
| 208 | } | 217 | } | ||
| 209 | return false | 218 | return false | ||
| @@ -340,7 +349,8 @@ func (c *Core) hintClickable(t hintTarget) bool { | |||||
| 340 | return false | 349 | return false | ||
| 341 | } | 350 | } | ||
| 342 | id, _, _ := c.hit(t.x, t.y) | 351 | id, _, _ := c.hit(t.x, t.y) | ||
| 343 | - | return id == t.window | 352 | + | sc, _ := c.screenOf(t.source.id) |
| 353 | + | return id == t.window && sc != nil && c.hintSourceUnchanged(sc, t.source) | |||
| 344 | } | 354 | } | ||
| 345 | 355 | ||||
| 346 | // hintLabels are the labels to draw on sc: those still matching what was | 356 | // hintLabels are the labels to draw on sc: those still matching what was | ||
M internal/core/hint_test.go +46 -0
| @@ -832,3 +832,49 @@ func TestHintDisabledNotesOnce(t *testing.T) { | |||
| 832 | 832 | t.Fatalf("disabled note ran %d times, want 1", got) | |
| 833 | 833 | } | |
| 834 | 834 | } | |
| 835 | + | ||
| 836 | + | // Resizing window 1 leaves the old target centre inside it. Identity alone | |
| 837 | + | // therefore cannot protect either admission or the eventual synthetic click. | |
| 838 | + | func TestHintRejectsResizedSource(t *testing.T) { | |
| 839 | + | for _, accessibility := range []bool{false, true} { | |
| 840 | + | for _, beforeResult := range []bool{false, true} { | |
| 841 | + | t.Run(fmt.Sprintf("accessibility=%v/beforeResult=%v", accessibility, beforeResult), func(t *testing.T) { | |
| 842 | + | h := newHint(t, core.Options{}) | |
| 843 | + | tg := h.inside(0) | |
| 844 | + | tg.Text = !accessibility | |
| 845 | + | if accessibility { | |
| 846 | + | tg.Window = 1 | |
| 847 | + | } | |
| 848 | + | req := h.start() | |
| 849 | + | if !beforeResult { | |
| 850 | + | h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}} | |
| 851 | + | h.labelled(1, 0) | |
| 852 | + | } | |
| 853 | + | h.client <- ports.WindowMapped{ID: 2} | |
| 854 | + | s := h.until(func(s ports.Scene) bool { return len(s.Windows) == 2 }) | |
| 855 | + | var resized ports.Rect | |
| 856 | + | for _, w := range s.Windows { | |
| 857 | + | if w.ID == 1 { | |
| 858 | + | resized = w.Rect | |
| 859 | + | } | |
| 860 | + | } | |
| 861 | + | x, y := tg.Rect.X+tg.Rect.W/2, tg.Rect.Y+tg.Rect.H/2 | |
| 862 | + | if resized == h.win || x < resized.X || x >= resized.X+resized.W || y < resized.Y || y >= resized.Y+resized.H { | |
| 863 | + | t.Fatalf("setup: old target %+v not inside resized window %+v (was %+v)", tg.Rect, resized, h.win) | |
| 864 | + | } | |
| 865 | + | if beforeResult { | |
| 866 | + | h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}} | |
| 867 | + | h.labelled(0, 0) | |
| 868 | + | } else { | |
| 869 | + | h.drain() | |
| 870 | + | h.press("f", 0) | |
| 871 | + | for _, cmd := range h.barrier() { | |
| 872 | + | if _, ok := cmd.(ports.PointerButtonTo); ok { | |
| 873 | + | t.Fatal("clicked resized source window") | |
| 874 | + | } | |
| 875 | + | } | |
| 876 | + | } | |
| 877 | + | }) | |
| 878 | + | } | |
| 879 | + | } | |
| 880 | + | } | |
| @@ -832,3 +832,49 @@ func TestHintDisabledNotesOnce(t *testing.T) { | |||||
| 832 | t.Fatalf("disabled note ran %d times, want 1", got) | 832 | t.Fatalf("disabled note ran %d times, want 1", got) | ||
| 833 | } | 833 | } | ||
| 834 | } | 834 | } | ||
| 835 | + | ||||
| 836 | + | // Resizing window 1 leaves the old target centre inside it. Identity alone | |||
| 837 | + | // therefore cannot protect either admission or the eventual synthetic click. | |||
| 838 | + | func TestHintRejectsResizedSource(t *testing.T) { | |||
| 839 | + | for _, accessibility := range []bool{false, true} { | |||
| 840 | + | for _, beforeResult := range []bool{false, true} { | |||
| 841 | + | t.Run(fmt.Sprintf("accessibility=%v/beforeResult=%v", accessibility, beforeResult), func(t *testing.T) { | |||
| 842 | + | h := newHint(t, core.Options{}) | |||
| 843 | + | tg := h.inside(0) | |||
| 844 | + | tg.Text = !accessibility | |||
| 845 | + | if accessibility { | |||
| 846 | + | tg.Window = 1 | |||
| 847 | + | } | |||
| 848 | + | req := h.start() | |||
| 849 | + | if !beforeResult { | |||
| 850 | + | h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}} | |||
| 851 | + | h.labelled(1, 0) | |||
| 852 | + | } | |||
| 853 | + | h.client <- ports.WindowMapped{ID: 2} | |||
| 854 | + | s := h.until(func(s ports.Scene) bool { return len(s.Windows) == 2 }) | |||
| 855 | + | var resized ports.Rect | |||
| 856 | + | for _, w := range s.Windows { | |||
| 857 | + | if w.ID == 1 { | |||
| 858 | + | resized = w.Rect | |||
| 859 | + | } | |||
| 860 | + | } | |||
| 861 | + | x, y := tg.Rect.X+tg.Rect.W/2, tg.Rect.Y+tg.Rect.H/2 | |||
| 862 | + | if resized == h.win || x < resized.X || x >= resized.X+resized.W || y < resized.Y || y >= resized.Y+resized.H { | |||
| 863 | + | t.Fatalf("setup: old target %+v not inside resized window %+v (was %+v)", tg.Rect, resized, h.win) | |||
| 864 | + | } | |||
| 865 | + | if beforeResult { | |||
| 866 | + | h.hints <- ports.HintScanResult{Seq: req.Seq, Output: req.Output, Targets: []ports.HintTarget{tg}} | |||
| 867 | + | h.labelled(0, 0) | |||
| 868 | + | } else { | |||
| 869 | + | h.drain() | |||
| 870 | + | h.press("f", 0) | |||
| 871 | + | for _, cmd := range h.barrier() { | |||
| 872 | + | if _, ok := cmd.(ports.PointerButtonTo); ok { | |||
| 873 | + | t.Fatal("clicked resized source window") | |||
| 874 | + | } | |||
| 875 | + | } | |||
| 876 | + | } | |||
| 877 | + | }) | |||
| 878 | + | } | |||
| 879 | + | } | |||
| 880 | + | } | |||