Skip to content

Conversation

@danirabbit
Copy link
Member

@danirabbit danirabbit commented Jun 20, 2025

Reverts #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 danirabbit requested a review from a team June 20, 2025 18:36
@lenemter
Copy link
Member

This is an issue in indicators, not in gala

@danirabbit
Copy link
Member Author

danirabbit commented Jun 20, 2025

@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

@lenemter
Copy link
Member

lenemter commented Jun 22, 2025

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.

@danirabbit
Copy link
Member Author

danirabbit commented Jun 23, 2025

@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
Copy link
Member Author

@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

@lenemter
Copy link
Member

lenemter commented Jun 23, 2025

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 elementary/switchboard#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.

@tintou
Copy link
Member

tintou commented Jun 23, 2025

I don't think that doing elementary/switchboard#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)

@danirabbit
Copy link
Member Author

@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

Copy link
Member

@leolost2605 leolost2605 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @danirabbit here. This really has a big impact on the day to day usability. No matter if it means we rely on a bug but the current behavior is definitely better than this one. Also the revert doesn't introduce much tech debt nor does it block any major changes, improvements or anything else in gala. So I guess we should revert, and then discuss and figure out a way to move forward. For example there should be a way to activate other windows from the currently active window via xdg activation but I'm not sure how that's handled in mutter and what we have to do to make that work but that is the proper way to make elementary/switchboard#348 work (at least from what I understand what that PR should do).

@Marukesu
Copy link
Contributor

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.

@danirabbit
Copy link
Member Author

@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
Copy link
Member Author

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

@danirabbit
Copy link
Member Author

danirabbit commented Jun 24, 2025

We have elementary/switchboard#349 which fixes the issue for Switchboard. I also filed:

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:

  • Mail

@TomiOhl
Copy link
Contributor

TomiOhl commented Jun 26, 2025

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?

@danirabbit
Copy link
Member Author

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 danirabbit enabled auto-merge (squash) June 26, 2025 19:06
@danirabbit danirabbit merged commit 9839e8c into main Jun 26, 2025
5 checks passed
@danirabbit danirabbit deleted the revert-2405-lenemter/remove-windowattentiontracker branch June 26, 2025 19:08
@danirabbit danirabbit mentioned this pull request Jun 26, 2025
14 tasks
@danirabbit
Copy link
Member Author

Opened #2455 to keep discussion going

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.

Apps not being brought to front

7 participants