gala icon indicating copy to clipboard operation
gala copied to clipboard

Revert "Remove WindowAttentionTracker"

Open danirabbit opened this issue 6 months ago • 11 comments

Reverts elementary/gala#2405 Fixes #2450

I don't think removing this was the right move. We're getting folks in Discord complaining that windows aren't coming to the foreground when they're supposed to. For example, if you have System Settings already open somewhere and try to open it from an indicator, now nothing happens instead of it being focused as expected

danirabbit avatar Jun 20 '25 18:06 danirabbit

This is an issue in indicators, not in gala

lenemter avatar Jun 20 '25 19:06 lenemter

@lenemter it also happens with the desktop context menu, "open In" menu in Code and Files, and I'd imagine in other places. I think we should maybe figure out how to fix that first and then merge this because this is a significant change in behavior and a regression people are complaining about.

I'd like to have a release with this revert and then maybe we can re-merge and go around and fix everything else and then re-release gala only after all the other components have been updated

danirabbit avatar Jun 20 '25 21:06 danirabbit

because this is a significant change in behavior and a regression people are complaining about

Just like WindowAttentionTracker. It also introduces unwanted focus stealing when some messengers (Telegram, Teams, etc.) get new messages because window_demands_attention and window_marked_urgent should be used in dock to indicate that there's something new in the app, NOT to focus the app window.

lenemter avatar Jun 22 '25 20:06 lenemter

@lenemter all these components are just using appinfo.launch. So you're kind of breaking anything using glib here. I think you'd need to argue that point upstream maybe. But basically the entire desktop relies on this working as it did before your branch.

I don't think we should be optimized around a couple of non-native apps instead of GLib

danirabbit avatar Jun 23 '25 00:06 danirabbit

@lenemter it seems like Mutter is supposed to have a concept of focus stealing prevention that maybe need to look into more https://blogs.gnome.org/shell-dev/2024/09/20/understanding-gnome-shells-focus-stealing-prevention/

But I think my overall argument here is that this branch is a major breaking change that we shouldn't merge until after we have some replacement. Otherwise we're breaking the entirety of our own desktop with no plan on how to fix it

danirabbit avatar Jun 23 '25 00:06 danirabbit

all these components are just using appinfo.launch. So you're kind of breaking anything using glib here. I think you'd need to argue that point upstream maybe. But basically the entire desktop relies on this working as it did before your branch.

It is applications problem that they rely on a bug. I pushed https://github.com/elementary/switchboard/pull/348 for a proper fix.

I don't think we should be optimized around a couple of non-native apps instead of GLib

Fixing a bug is not 'optimizing around a non-native apps'

@lenemter it seems like Mutter is supposed to have a concept of focus stealing prevention that maybe need to look into more https://blogs.gnome.org/shell-dev/2024/09/20/understanding-gnome-shells-focus-stealing-prevention/

This is universal to Wayland in general. Windows cannot request focus which was a big security concern on X11.

But I think my overall argument here is that this branch is a major breaking change that we shouldn't merge until after we have some replacement. Otherwise we're breaking the entirety of our own desktop with no plan on how to fix it

And again, you were relying on a broken behavior and as I said a proper fix should be proposed in the app.

lenemter avatar Jun 23 '25 05:06 lenemter

I don't think that doing https://github.com/elementary/switchboard/pull/348 is realistic, I'd look at what GNOME Shell is doing with focus stealing prevention, there is a timestamp that get sent when requesting an action, and we should be using this (i.e. link a keystroke/click with the timestamp of the window opening action)

tintou avatar Jun 23 '25 06:06 tintou

@lenemter regardless of whether it's relying on a bug or not, this change has broken the desktop. I'm not saying we should never pursue a different approach, just that for now the entire desktop relies on this and we should have a plan to fix the other components before making this change in gala to prevent breakage

Again, this is a massively breaking change in behavior. I'm personally affected here and I've had several people message me about it wanting a fix right away

danirabbit avatar Jun 23 '25 13:06 danirabbit

The switchboard issue is that the indicators should be using Gtk.UriLauncher (or, in gtk3 Gtk.show_uri_on_window()) to launch the uri, that will set the xdg_activation token as expected.

Marukesu avatar Jun 23 '25 21:06 Marukesu

@Marukesu yup again not saying we should never make this change, just that we need to go around and fix everything else first before releasing this change because as is, this breaks functionality

danirabbit avatar Jun 23 '25 21:06 danirabbit

@Marukesu afaict #2450 is still reproducible with UriLauncher, so I think we're missing some mechanism in Gala still https://github.com/elementary/panel-keyboard/pull/148

danirabbit avatar Jun 23 '25 21:06 danirabbit

We have https://github.com/elementary/switchboard/pull/349 which fixes the issue for Switchboard. I also filed:

  • [ ] https://github.com/elementary/code/issues/1610
  • [ ] https://github.com/elementary/appcenter/issues/2300

We need to file issues/make branches for:

  • [ ] Web
  • [ ] File Roller
  • [ ] Fonts
  • [ ] Calendar
  • [ ] Files
  • [ ] Terminal

Also need to check behavior of

  • [ ] Document Viewer
  • [ ] Videos

Focusing works already afaict for:

  • [x] Mail

danirabbit avatar Jun 24 '25 20:06 danirabbit

Here it is in practice - "nothing happens" when I click on a link in Mail, then I discover that the link has already been opened multiple times on a completely different workspace

recording.webm

I assume it means it doesn't work in Mail without WindowAttentionTracker?

TomiOhl avatar Jun 26 '25 15:06 TomiOhl

Okay since it seems like we've got some open questions about how to resolve this properly and it affects kind of a lot of things, I'm gonna go ahead and merge this. And then I'll re-open a new draft PR to remove the attention tracker with some more information about pre-requisites/test cases

danirabbit avatar Jun 26 '25 19:06 danirabbit

Opened https://github.com/elementary/gala/pull/2455 to keep discussion going

danirabbit avatar Jun 26 '25 19:06 danirabbit