search

mate-desktop / mate-screensaver

MATE screen saver and locker
https://mate-desktop.org
GNU General Public License v2.0
48 stars 40 forks source link

Switch to GdkSeat for GTK+ 3.20 and newer #116

Closed XRevan86 closed 7 years ago

XRevan86 commented 7 years ago

gdk_keyboard_grab and gdk_pointer_grab are removed from GTK+ 4 making this an important change for the future. Yet at the same time I can't be 100% confident because I'm touching (a lot) a very important part of screenlocking potentionally creating a "security issue".

lukefromdc commented 7 years ago

I don't have this one installed at the moment, which means I would not see any behavioral changes other than either being unable to lock or unable to unlock the screen at all.

lukefromdc commented 7 years ago

In a quick test installation, the screensaver panel applet could not lock the screen nor activate the screensaver. The preferences dialog worked. I have no idea however if this is ongoing or a result of this PR. Build was fine.

XRevan86 commented 7 years ago

@lukefromdc, maybe you should have started the mate-screensaver daemon.

lukefromdc commented 7 years ago

Starting mate-screensaver, then using the screensaver applet led to the screensaver being displayed for a few seconds, then crashing and returning to the desktop. Same if a new session was started with the screensaver daemon active. This really needs testing by someone who normally uses it and has it installed though,

EDIT: when started from terminal, no text went to the terminal when the screensaver closed after a few seconds

flexiondotorg commented 7 years ago

Erm, did we establish if this actually works? We may also need to branch this repo for 1.18, from before this commit, so we can cherry pick for a point release if required.

XRevan86 commented 7 years ago

@flexiondotorg, this most definitely basically works %). Testing is still desired. Some freaky ways of trying to break the grab of mate-screensaver on the keyboard or something.

sbalneav commented 7 years ago

I meant to approve the merge, instead I just did it. But I have been testing it all day, and it appears to work just fine for me.

flexiondotorg commented 7 years ago

OK, good to hear ☺️

raveit65 commented 7 years ago

I see no regression here. Restarting the daemon after update and using the lock applet works out of box here. In a VM i restarted the session and locked the screen several times, no problems.

chr-chr commented 5 years ago

Hello,

this patch plus another bug makes this a security issue.

https://github.com/mate-desktop/mate-screensaver/issues/180

raveit65 commented 5 years ago

No

chr-chr commented 5 years ago

No

You're right. That code was buggy beforehand ;)