Conversation
a0fccac to
eb326b9
Compare
eb326b9 to
014233e
Compare
When locking screen, first open the window with a color (specified with `--color`) as background, then, if blurring is requested, do that and redraw window. Relates to Raymo111#239
If user has entered a correct password and pressed enter before the initialization has completed, i3lock used to verify the password but not exit correctly, which was easy to encounter with a high sigma value for blurring. I wonder if there is a more elegant implementation...
|
Hello! |
Raymo111
left a comment
There was a problem hiding this comment.
Review — Approve with nits
What it does: Takes the desktop screenshot before mapping the lock window, then maps the window with the solid --color background and grabs pointer/keyboard, and only afterwards runs the (potentially slow) blur and redraws. Also adds a global bool unlocked so that if the user typed the password and hit Enter before init finished, the process exits instead of blocking in the event loop.
Security — net improvement, no leak introduced: In both old and new code the grab happens after xcb_map_window and after the grab-retry block; the map→grab ordering is unchanged. What changes is that the slow blur now runs after the grab instead of before it — previously, with a high --sigma, the desktop stayed ungrabbed and interactive for the whole blur duration. This closes an existing exposure window rather than opening one. The screenshot is still captured before the lock window is mapped (so it captures the real desktop), and early keystrokes are only drained at ev_invoke, which runs after the grab is active.
Correctness: The unlocked flag covers all three ev_break sites (--no-verify, OpenBSD, PAM success) and not the auth-failure path, so a wrong early password still enters the loop correctly. The early-Enter fix is sound: ev_break during ev_invoke sets libev's loop_done, but ev_run still does one full blocking iteration before checking it — with the keyboard grabbed and no input, that blocks forever; gating ev_loop on !unlocked avoids it. NULL blur_bg_img during the early render is safe (guarded, falls back to solid color). No double-free on bg_pixmap.
Nit: xcb_pixmap_t bg_pixmap = bg_pixmap; (self-initialization to silence -Wmaybe-uninitialized) is a code smell and may itself trip -Winit-self. Prefer = XCB_NONE.
Recommendation: Merge after the = XCB_NONE tweak — improves lock-screen input security and fixes a genuine early-unlock hang.
Review comment (Raymo111#278 (review)): > **Nit:** `xcb_pixmap_t bg_pixmap = bg_pixmap;` (self-initialization to silence `-Wmaybe-uninitialized`) is a code smell and may itself trip `-Winit-self`. Prefer `= XCB_NONE`.
|
👍 |
Description
When locking screen, first open the window with a color (specified with
--color) as background, then, if blurring is requested, do that and redraw window.My use case is to lock screen when the laptop lid is closed (I use
xss-lockfor that) but sometimesi3lock-colordoesn't manage to do the blurring in time, as a consequence, when the lid is opened, one has about a second or two to see the screen before the screen is locked. The solution is to first open the window earlier, and update it after processing the blur.Relates to #239. Not sure if the PR resolves it, but it can certainly act as an intermediate solution before a fancier locking mechanism, as requested by the issue author, is implemented. By the way, I think, the requested behavior is achieved with running
i3lock -B <sigma> --color '#00000000': it will first open the window with fully transparent background, then blur the screenshot and update the window whenever it has finished the processing.Release notes
Notes: open the lock window earlier when locking with blurring
Behavior worth noticing
unless the enter key was pressed. But if it was, the lock screen at first seems to behave the same way (that is, all the decorations appear and it then shows the "verifying..." caption), but it remains stuck in the verifying state until I try to retype my password and verify again. Even with the--no-verifyoption, pressing enter on the early window stage will bring it to the verifying state.I would appreciate a hint on why this could be happening. If you know how to fix this, feel free to submit a patch, otherwise I will try to dig deeper into this later when I have time.
No 'unless' anymore, everything is fine now :)
i3lock-coloris supposed to support the--imageand--bluroptions together and, if yes, what behavior is expected, but it would probably make sense to display the background image while the blurring is in progress, not a solid color. Should it?