Merge pull request 'fix(core): focus the lock surface of the focused output' (#1) from fix/lock-focus-output into main
Reviewed-on: https://git.bnema.dev/bnema/neferwl/pulls/1
4 files · +86 -11
M internal/adapters/wayland/presentation_test.go +14 -2
| @@ -166,8 +166,20 @@ func TestPresentationFeedback(t *testing.T) { | |||
| 166 | 166 | t.Fatalf("hidden feedback: %+v", ev) | |
| 167 | 167 | } | |
| 168 | 168 | commands <- ports.ConfigureWindow{ID: w.ID, Width: 100, Height: 100, Output: "HEADLESS-1", Visible: true} | |
| 169 | - | // Synchronize the resume before committing the next feedback. | |
| 170 | - | s.display.Do(func() {}) | |
| 169 | + | // Wait for the resume before committing the next feedback: the command | |
| 170 | + | // goroutine applies it in its own display round trip, which an empty Do | |
| 171 | + | // here may overtake. | |
| 172 | + | for deadline := time.Now().Add(2 * time.Second); ; { | |
| 173 | + | var visible bool | |
| 174 | + | s.display.Do(func() { visible = s.windows[w.ID].last.Visible }) | |
| 175 | + | if visible { | |
| 176 | + | break | |
| 177 | + | } | |
| 178 | + | if time.Now().After(deadline) { | |
| 179 | + | t.Fatal("resume not applied") | |
| 180 | + | } | |
| 181 | + | time.Sleep(2 * time.Millisecond) | |
| 182 | + | } | |
| 171 | 183 | // Presented with the flip's time, counter and flags. | |
| 172 | 184 | a := feedback() | |
| 173 | 185 | commit() | |
| @@ -166,8 +166,20 @@ func TestPresentationFeedback(t *testing.T) { | |||||
| 166 | t.Fatalf("hidden feedback: %+v", ev) | 166 | t.Fatalf("hidden feedback: %+v", ev) | ||
| 167 | } | 167 | } | ||
| 168 | commands <- ports.ConfigureWindow{ID: w.ID, Width: 100, Height: 100, Output: "HEADLESS-1", Visible: true} | 168 | commands <- ports.ConfigureWindow{ID: w.ID, Width: 100, Height: 100, Output: "HEADLESS-1", Visible: true} | ||
| 169 | - | // Synchronize the resume before committing the next feedback. | 169 | + | // Wait for the resume before committing the next feedback: the command |
| 170 | - | s.display.Do(func() {}) | 170 | + | // goroutine applies it in its own display round trip, which an empty Do |
| 171 | + | // here may overtake. | |||
| 172 | + | for deadline := time.Now().Add(2 * time.Second); ; { | |||
| 173 | + | var visible bool | |||
| 174 | + | s.display.Do(func() { visible = s.windows[w.ID].last.Visible }) | |||
| 175 | + | if visible { | |||
| 176 | + | break | |||
| 177 | + | } | |||
| 178 | + | if time.Now().After(deadline) { | |||
| 179 | + | t.Fatal("resume not applied") | |||
| 180 | + | } | |||
| 181 | + | time.Sleep(2 * time.Millisecond) | |||
| 182 | + | } | |||
| 171 | // Presented with the flip's time, counter and flags. | 183 | // Presented with the flip's time, counter and flags. | ||
| 172 | a := feedback() | 184 | a := feedback() | ||
| 173 | commit() | 185 | commit() | ||
M internal/core/core.go +1 -0
| @@ -70,6 +70,7 @@ type Core struct { | |||
| 70 | 70 | security ports.SecurityState | |
| 71 | 71 | lockSurfaces []ports.LockSurfacePlacement | |
| 72 | 72 | lockFocus WindowID | |
| 73 | + | lockPinned bool // lockFocus was clicked or typed into: it keeps focus | |
| 73 | 74 | inputKeys map[string]bool | |
| 74 | 75 | inputActive bool | |
| 75 | 76 | startup [][]string | |
| @@ -70,6 +70,7 @@ type Core struct { | |||||
| 70 | security ports.SecurityState | 70 | security ports.SecurityState | ||
| 71 | lockSurfaces []ports.LockSurfacePlacement | 71 | lockSurfaces []ports.LockSurfacePlacement | ||
| 72 | lockFocus WindowID | 72 | lockFocus WindowID | ||
| 73 | + | lockPinned bool // lockFocus was clicked or typed into: it keeps focus | |||
| 73 | inputKeys map[string]bool | 74 | inputKeys map[string]bool | ||
| 74 | inputActive bool | 75 | inputActive bool | ||
| 75 | startup [][]string | 76 | startup [][]string | ||
M internal/core/security.go +21 -9
| @@ -21,7 +21,7 @@ func (c *Core) syncSecurity() bool { | |||
| 21 | 21 | return false | |
| 22 | 22 | } | |
| 23 | 23 | c.security = state | |
| 24 | - | c.lockSurfaces, c.lockFocus = nil, 0 | |
| 24 | + | c.lockSurfaces, c.lockFocus, c.lockPinned = nil, 0, false | |
| 25 | 25 | c.pressed, c.buttons, c.inputKeys = map[string]bool{}, map[uint32]bool{}, map[string]bool{} | |
| 26 | 26 | c.pointer, c.grab, c.pointerOutput = 0, 0, "" | |
| 27 | 27 | c.drag, c.swallow, c.mods, c.lastButton = nil, map[uint32]bool{}, 0, 0 | |
| @@ -135,18 +135,26 @@ func (c *Core) validLockID(id WindowID) bool { | |||
| 135 | 135 | return false | |
| 136 | 136 | } | |
| 137 | 137 | ||
| 138 | + | // lockKeyboardFocus prefers the lock surface of the focused output, where the | |
| 139 | + | // user is looking, then any other. A surface the user clicked or typed into is | |
| 140 | + | // pinned and keeps focus. Until then, focus moves to the focused output's | |
| 141 | + | // surface once it maps: lock clients map their surfaces one by one, in any | |
| 142 | + | // order. | |
| 138 | 143 | func (c *Core) lockKeyboardFocus() WindowID { | |
| 139 | - | if c.validLockID(c.lockFocus) { | |
| 144 | + | valid := c.validLockID(c.lockFocus) | |
| 145 | + | if valid && c.lockPinned { | |
| 140 | 146 | return c.lockFocus | |
| 141 | 147 | } | |
| 142 | - | c.lockFocus = 0 | |
| 143 | - | // Prefer the focused output, where the user is looking, then any other. | |
| 144 | 148 | if c.focusScreen >= 0 && c.focusScreen < len(c.screens) { | |
| 145 | 149 | if s, ok := c.lockSurface(c.screens[c.focusScreen]); ok { | |
| 146 | - | c.lockFocus = s.ID | |
| 150 | + | c.lockFocus, c.lockPinned = s.ID, false | |
| 147 | 151 | return c.lockFocus | |
| 148 | 152 | } | |
| 149 | 153 | } | |
| 154 | + | if valid { | |
| 155 | + | return c.lockFocus | |
| 156 | + | } | |
| 157 | + | c.lockFocus, c.lockPinned = 0, false | |
| 150 | 158 | for _, sc := range c.screens { | |
| 151 | 159 | if s, ok := c.lockSurface(sc); ok { | |
| 152 | 160 | c.lockFocus = s.ID | |
| @@ -239,6 +247,7 @@ func (c *Core) protectedInput(ctx context.Context, ev ports.InputEvent) error { | |||
| 239 | 247 | switch v := ev.(type) { | |
| 240 | 248 | case ports.KeyEvent: | |
| 241 | 249 | if id := c.lockKeyboardFocus(); id != 0 { | |
| 250 | + | c.lockPinned = true // typing must not move focus mid-secret | |
| 242 | 251 | return c.command(ctx, ports.ForwardKey{ID: id, Key: v}) | |
| 243 | 252 | } | |
| 244 | 253 | case ports.PointerMotion: | |
| @@ -272,10 +281,13 @@ func (c *Core) protectedInput(ctx context.Context, ev ports.InputEvent) error { | |||
| 272 | 281 | delete(c.buttons, v.Button) | |
| 273 | 282 | } | |
| 274 | 283 | if c.validLockID(id) { | |
| 275 | - | if v.Pressed && c.lockFocus != id { | |
| 276 | - | c.lockFocus = id | |
| 277 | - | if err := c.publishProtected(ctx); err != nil { | |
| 278 | - | return err | |
| 284 | + | if v.Pressed { | |
| 285 | + | changed := c.lockFocus != id | |
| 286 | + | c.lockFocus, c.lockPinned = id, true | |
| 287 | + | if changed { | |
| 288 | + | if err := c.publishProtected(ctx); err != nil { | |
| 289 | + | return err | |
| 290 | + | } | |
| 279 | 291 | } | |
| 280 | 292 | } | |
| 281 | 293 | if err := c.command(ctx, ports.PointerButtonTo{ID: id, Button: v.Button, Pressed: v.Pressed, TimeMsec: v.TimeMsec}); err != nil { | |
| @@ -21,7 +21,7 @@ func (c *Core) syncSecurity() bool { | |||||
| 21 | return false | 21 | return false | ||
| 22 | } | 22 | } | ||
| 23 | c.security = state | 23 | c.security = state | ||
| 24 | - | c.lockSurfaces, c.lockFocus = nil, 0 | 24 | + | c.lockSurfaces, c.lockFocus, c.lockPinned = nil, 0, false |
| 25 | c.pressed, c.buttons, c.inputKeys = map[string]bool{}, map[uint32]bool{}, map[string]bool{} | 25 | c.pressed, c.buttons, c.inputKeys = map[string]bool{}, map[uint32]bool{}, map[string]bool{} | ||
| 26 | c.pointer, c.grab, c.pointerOutput = 0, 0, "" | 26 | c.pointer, c.grab, c.pointerOutput = 0, 0, "" | ||
| 27 | c.drag, c.swallow, c.mods, c.lastButton = nil, map[uint32]bool{}, 0, 0 | 27 | c.drag, c.swallow, c.mods, c.lastButton = nil, map[uint32]bool{}, 0, 0 | ||
| @@ -135,18 +135,26 @@ func (c *Core) validLockID(id WindowID) bool { | |||||
| 135 | return false | 135 | return false | ||
| 136 | } | 136 | } | ||
| 137 | 137 | ||||
| 138 | + | // lockKeyboardFocus prefers the lock surface of the focused output, where the | |||
| 139 | + | // user is looking, then any other. A surface the user clicked or typed into is | |||
| 140 | + | // pinned and keeps focus. Until then, focus moves to the focused output's | |||
| 141 | + | // surface once it maps: lock clients map their surfaces one by one, in any | |||
| 142 | + | // order. | |||
| 138 | func (c *Core) lockKeyboardFocus() WindowID { | 143 | func (c *Core) lockKeyboardFocus() WindowID { | ||
| 139 | - | if c.validLockID(c.lockFocus) { | 144 | + | valid := c.validLockID(c.lockFocus) |
| 145 | + | if valid && c.lockPinned { | |||
| 140 | return c.lockFocus | 146 | return c.lockFocus | ||
| 141 | } | 147 | } | ||
| 142 | - | c.lockFocus = 0 | |||
| 143 | - | // Prefer the focused output, where the user is looking, then any other. | |||
| 144 | if c.focusScreen >= 0 && c.focusScreen < len(c.screens) { | 148 | if c.focusScreen >= 0 && c.focusScreen < len(c.screens) { | ||
| 145 | if s, ok := c.lockSurface(c.screens[c.focusScreen]); ok { | 149 | if s, ok := c.lockSurface(c.screens[c.focusScreen]); ok { | ||
| 146 | - | c.lockFocus = s.ID | 150 | + | c.lockFocus, c.lockPinned = s.ID, false |
| 147 | return c.lockFocus | 151 | return c.lockFocus | ||
| 148 | } | 152 | } | ||
| 149 | } | 153 | } | ||
| 154 | + | if valid { | |||
| 155 | + | return c.lockFocus | |||
| 156 | + | } | |||
| 157 | + | c.lockFocus, c.lockPinned = 0, false | |||
| 150 | for _, sc := range c.screens { | 158 | for _, sc := range c.screens { | ||
| 151 | if s, ok := c.lockSurface(sc); ok { | 159 | if s, ok := c.lockSurface(sc); ok { | ||
| 152 | c.lockFocus = s.ID | 160 | c.lockFocus = s.ID | ||
| @@ -239,6 +247,7 @@ func (c *Core) protectedInput(ctx context.Context, ev ports.InputEvent) error { | |||||
| 239 | switch v := ev.(type) { | 247 | switch v := ev.(type) { | ||
| 240 | case ports.KeyEvent: | 248 | case ports.KeyEvent: | ||
| 241 | if id := c.lockKeyboardFocus(); id != 0 { | 249 | if id := c.lockKeyboardFocus(); id != 0 { | ||
| 250 | + | c.lockPinned = true // typing must not move focus mid-secret | |||
| 242 | return c.command(ctx, ports.ForwardKey{ID: id, Key: v}) | 251 | return c.command(ctx, ports.ForwardKey{ID: id, Key: v}) | ||
| 243 | } | 252 | } | ||
| 244 | case ports.PointerMotion: | 253 | case ports.PointerMotion: | ||
| @@ -272,10 +281,13 @@ func (c *Core) protectedInput(ctx context.Context, ev ports.InputEvent) error { | |||||
| 272 | delete(c.buttons, v.Button) | 281 | delete(c.buttons, v.Button) | ||
| 273 | } | 282 | } | ||
| 274 | if c.validLockID(id) { | 283 | if c.validLockID(id) { | ||
| 275 | - | if v.Pressed && c.lockFocus != id { | 284 | + | if v.Pressed { |
| 276 | - | c.lockFocus = id | 285 | + | changed := c.lockFocus != id |
| 277 | - | if err := c.publishProtected(ctx); err != nil { | 286 | + | c.lockFocus, c.lockPinned = id, true |
| 278 | - | return err | 287 | + | if changed { |
| 288 | + | if err := c.publishProtected(ctx); err != nil { | |||
| 289 | + | return err | |||
| 290 | + | } | |||
| 279 | } | 291 | } | ||
| 280 | } | 292 | } | ||
| 281 | if err := c.command(ctx, ports.PointerButtonTo{ID: id, Button: v.Button, Pressed: v.Pressed, TimeMsec: v.TimeMsec}); err != nil { | 293 | if err := c.command(ctx, ports.PointerButtonTo{ID: id, Button: v.Button, Pressed: v.Pressed, TimeMsec: v.TimeMsec}); err != nil { | ||
M internal/core/security_internal_test.go +50 -0
| @@ -72,6 +72,56 @@ func TestProtectedDropsPrivateOffscreenCaptureAndDesktopEffects(t *testing.T) { | |||
| 72 | 72 | } | |
| 73 | 73 | } | |
| 74 | 74 | ||
| 75 | + | // Lock clients map their surfaces one by one: the focused output's surface | |
| 76 | + | // takes keyboard focus when it maps after another one, unless the user | |
| 77 | + | // already clicked or typed into a surface. | |
| 78 | + | func TestLockFocusMovesToFocusedOutputSurfaceUntilPinned(t *testing.T) { | |
| 79 | + | for _, tc := range []struct { | |
| 80 | + | name string | |
| 81 | + | input []ports.InputEvent // on A, before B maps | |
| 82 | + | want WindowID | |
| 83 | + | }{ | |
| 84 | + | {"no input", nil, 11}, | |
| 85 | + | {"click on focused A", []ports.InputEvent{ports.PointerMotion{X: 10, Y: 10}, ports.PointerButton{Button: 0x110, Pressed: true}}, 10}, | |
| 86 | + | {"typing", []ports.InputEvent{ports.KeyEvent{Keysym: "a", Keycode: 30, Pressed: true}}, 10}, | |
| 87 | + | } { | |
| 88 | + | t.Run(tc.name, func(t *testing.T) { | |
| 89 | + | c, _, _ := hiddenCaptureCommands(t) | |
| 90 | + | c.addScreen(ports.OutputInfo{Name: "B", Width: 400, Height: 300}) | |
| 91 | + | c.focusScreen = c.screenIndex("B") | |
| 92 | + | state := ports.SecurityState{Generation: 1, Protected: true} | |
| 93 | + | gate := portsmocks.NewMockSessionSecurity(t) | |
| 94 | + | gate.EXPECT().Snapshot().RunAndReturn(func() ports.SecurityState { return state }) | |
| 95 | + | c.ch.Security = gate | |
| 96 | + | c.syncSecurity() | |
| 97 | + | ctx := context.Background() | |
| 98 | + | mapped := func(s ...ports.LockSurfacePlacement) { | |
| 99 | + | t.Helper() | |
| 100 | + | c.applyLockChanged(ports.SessionLockChanged{State: state, Surfaces: s}) | |
| 101 | + | if err := c.publishProtected(ctx); err != nil { | |
| 102 | + | t.Fatal(err) | |
| 103 | + | } | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | a := ports.LockSurfacePlacement{ID: 10, Output: "A", Width: 300, Height: 200} | |
| 107 | + | b := ports.LockSurfacePlacement{ID: 11, Output: "B", Width: 400, Height: 300} | |
| 108 | + | mapped(a) | |
| 109 | + | if c.keyboard.sent != 10 { | |
| 110 | + | t.Fatalf("only mapped surface not focused: %d", c.keyboard.sent) | |
| 111 | + | } | |
| 112 | + | for _, ev := range tc.input { | |
| 113 | + | if err := c.protectedInput(ctx, ev); err != nil { | |
| 114 | + | t.Fatal(err) | |
| 115 | + | } | |
| 116 | + | } | |
| 117 | + | mapped(a, b) | |
| 118 | + | if c.keyboard.sent != tc.want { | |
| 119 | + | t.Fatalf("keyboard focus %d, want %d", c.keyboard.sent, tc.want) | |
| 120 | + | } | |
| 121 | + | }) | |
| 122 | + | } | |
| 123 | + | } | |
| 124 | + | ||
| 75 | 125 | func TestWiredZeroEpochRejectsRawInputAndCommandKeepsOwnerEpoch(t *testing.T) { | |
| 76 | 126 | c, _, commands := hiddenCaptureCommands(t) | |
| 77 | 127 | gate := portsmocks.NewMockSessionSecurity(t) | |
| @@ -72,6 +72,56 @@ func TestProtectedDropsPrivateOffscreenCaptureAndDesktopEffects(t *testing.T) { | |||||
| 72 | } | 72 | } | ||
| 73 | } | 73 | } | ||
| 74 | 74 | ||||
| 75 | + | // Lock clients map their surfaces one by one: the focused output's surface | |||
| 76 | + | // takes keyboard focus when it maps after another one, unless the user | |||
| 77 | + | // already clicked or typed into a surface. | |||
| 78 | + | func TestLockFocusMovesToFocusedOutputSurfaceUntilPinned(t *testing.T) { | |||
| 79 | + | for _, tc := range []struct { | |||
| 80 | + | name string | |||
| 81 | + | input []ports.InputEvent // on A, before B maps | |||
| 82 | + | want WindowID | |||
| 83 | + | }{ | |||
| 84 | + | {"no input", nil, 11}, | |||
| 85 | + | {"click on focused A", []ports.InputEvent{ports.PointerMotion{X: 10, Y: 10}, ports.PointerButton{Button: 0x110, Pressed: true}}, 10}, | |||
| 86 | + | {"typing", []ports.InputEvent{ports.KeyEvent{Keysym: "a", Keycode: 30, Pressed: true}}, 10}, | |||
| 87 | + | } { | |||
| 88 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 89 | + | c, _, _ := hiddenCaptureCommands(t) | |||
| 90 | + | c.addScreen(ports.OutputInfo{Name: "B", Width: 400, Height: 300}) | |||
| 91 | + | c.focusScreen = c.screenIndex("B") | |||
| 92 | + | state := ports.SecurityState{Generation: 1, Protected: true} | |||
| 93 | + | gate := portsmocks.NewMockSessionSecurity(t) | |||
| 94 | + | gate.EXPECT().Snapshot().RunAndReturn(func() ports.SecurityState { return state }) | |||
| 95 | + | c.ch.Security = gate | |||
| 96 | + | c.syncSecurity() | |||
| 97 | + | ctx := context.Background() | |||
| 98 | + | mapped := func(s ...ports.LockSurfacePlacement) { | |||
| 99 | + | t.Helper() | |||
| 100 | + | c.applyLockChanged(ports.SessionLockChanged{State: state, Surfaces: s}) | |||
| 101 | + | if err := c.publishProtected(ctx); err != nil { | |||
| 102 | + | t.Fatal(err) | |||
| 103 | + | } | |||
| 104 | + | } | |||
| 105 | + | ||||
| 106 | + | a := ports.LockSurfacePlacement{ID: 10, Output: "A", Width: 300, Height: 200} | |||
| 107 | + | b := ports.LockSurfacePlacement{ID: 11, Output: "B", Width: 400, Height: 300} | |||
| 108 | + | mapped(a) | |||
| 109 | + | if c.keyboard.sent != 10 { | |||
| 110 | + | t.Fatalf("only mapped surface not focused: %d", c.keyboard.sent) | |||
| 111 | + | } | |||
| 112 | + | for _, ev := range tc.input { | |||
| 113 | + | if err := c.protectedInput(ctx, ev); err != nil { | |||
| 114 | + | t.Fatal(err) | |||
| 115 | + | } | |||
| 116 | + | } | |||
| 117 | + | mapped(a, b) | |||
| 118 | + | if c.keyboard.sent != tc.want { | |||
| 119 | + | t.Fatalf("keyboard focus %d, want %d", c.keyboard.sent, tc.want) | |||
| 120 | + | } | |||
| 121 | + | }) | |||
| 122 | + | } | |||
| 123 | + | } | |||
| 124 | + | ||||
| 75 | func TestWiredZeroEpochRejectsRawInputAndCommandKeepsOwnerEpoch(t *testing.T) { | 125 | func TestWiredZeroEpochRejectsRawInputAndCommandKeepsOwnerEpoch(t *testing.T) { | ||
| 76 | c, _, commands := hiddenCaptureCommands(t) | 126 | c, _, commands := hiddenCaptureCommands(t) | ||
| 77 | gate := portsmocks.NewMockSessionSecurity(t) | 127 | gate := portsmocks.NewMockSessionSecurity(t) | ||