Skip to content

Bug #239: fix lockscreen dimming #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

IBBoard
Copy link
Contributor

@IBBoard IBBoard commented Jul 13, 2024

I think I've managed to fix the lockscreen dimming (#239) and some related issues.

The commit messages have more detail, but basically:

  • Gnome 46 seems to have changed one of the blur parameters
  • The events were hooked up a little strangely and weren't applying consistently (or were applying repeatedly!) until I adjusted the setting change handling

I'm running this locally and I haven't seen any issues yet.

IBBoard added 4 commits July 13, 2024 14:56
JavaScript was evaluatinig string+func as bool for the
ternary rather than appending the ternary
Re-applying the same value is harmless, so use the `_setBlur`
function to ensure we have the current value
We connect to setting change events, but never disconnect.
Destroying the object doesn't disconnect the events. Disabling
and re-enabling the extension then caused repeated updates.
And locking and unlocking the screen caused the extension to
be disabled and re-enabled.

We now clean up the setting events when the object gets destroyed.
@IBBoard
Copy link
Contributor Author

IBBoard commented Jul 27, 2024

There may be a bug in this that I need to track down.

I've not worked out when. But sometimes it seems to use some default settings for the blur. I have it set for a slight blur, but sometimes when I wake the monitor and unlock it then I see a strong blur.

But at least it is blurring, so the prompts are easier to see 🙂

@neffo
Copy link
Owner

neffo commented Jul 28, 2024

Thanks for looking into this, and yeah this has been a cause of some problems for a while. There is a bit of inconsistent behavior happening in the existing code which I never tracked down (seemed to trigger when recovering from standby as you note). I suspected this might be due to something that GNOME was doing during that state change, but I'll grant this code is ugly. Other extensions do this in other ways, but trying to tap into the state change of password entry is maybe unique? I think this is key to making it usable though.

@IBBoard
Copy link
Contributor Author

IBBoard commented Sep 16, 2024

After running this for a while, it's not 100% consistent, but it sometimes seems to go for a heavy blur, and it seems to be when I have to wake my monitor up (e.g. I've left my PC along for an hour or more). It's not every time. And it's not related to hibernating (the PC is still on, it's just the monitor going into power saving and then turning itself off). It could be a race condition during the wake up.

But the next lock after that has the right blur. And too much blur is better for readability than not enough blur, so I'm accepting the compromise for now 🙂

@neffo
Copy link
Owner

neffo commented Sep 17, 2024

Yeah I had similar problems trying to debug this, not clear why it wasn't inconsistent.

Are you ready to merge it?

@IBBoard
Copy link
Contributor Author

IBBoard commented Sep 17, 2024

I've not put in logging to chase it down yet. You could merge it if you want, because I think "mostly gets the right dimming/blur" is better than "rarely gets it right". But I don't mind if you'd rather wait until the bug is fixed as well.

@neffo
Copy link
Owner

neffo commented Sep 18, 2024

There's no rush, I just want to put out a version to support the new GNOME version with whatever is fixed. I've not tested your branch yet myself, but obviously appreciate the effort you've put into it and would be keen to merge when you are happy with it. (It can be merged whenever.)

@IBBoard
Copy link
Contributor Author

IBBoard commented Sep 18, 2024

If you want a version for Gnome support then merge this and I'll work on the the occasional default blur levels separately.

@neffo neffo changed the base branch from master to version-50 September 19, 2024 11:09
@neffo neffo merged commit df89ae6 into neffo:version-50 Sep 19, 2024
2 checks passed
@neffo neffo mentioned this pull request Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants