Return to previous window/workspace with `Open App`

[Batwam]

with reference to #44, this first pass code will refocus the window which was focused before the player is activated.

I haven't quite worked out how to activate alt+tab. For info, the reference code should be here: https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/main/js/ui/altTab.js

In the meantime, I am saving the previously focused window and re-activating it if the player is focussed.

One thing I would also like to include is minimising the player if it was minimised before being activated.

[Batwam]

This implementation has the limitations discussed in the other threads, namely the fact that the maximised windows is identified when the label is clicked. If the player activates another app, then goes back to the player, then clicks in the label. The old app will be refocused. So, calling Alt+Tab would still be preferable but I can't figure out how to call it...

....or implementing something similar to what alt+tab does. windows might be ordered in a list for instance.

some example of use of AltTab here if this makes sense to you: https://github.com/G-dH/advanced-alttab-window-switcher/blob/main/extension.js

[Batwam]

ok, we can get an get a list of windows with global.get_window_actors().

interestingly enough, this list ends with the player and then Gnome-Shell so it's ordered backwards. Just before the focused player, 3rd from the end is the window we want to re-focus (kgx in the example above). If I open another app, it will go to the bottom just before Gnome-Shell and when I refocus Spotify, Spotify will take its spot.

See proposed code in next commit. This works around the issue mentioned above and is essentially a manual implementation of alt+tab which avoids having to save the name of the previously focused window.

[Batwam]

code confirmed on both x11 and wayland as well as will multiple mpris sources and auto_switch.

[Batwam]

With regards to identifying the minimise status, we have previous code which allowed us to identify the window corresponding to a given app here: 7ff41998ea3ebb4875f12987284300c78c93c147

I don't think that it's bullet proof as we had to work with inconsistencies between identity and wm_class (Mozilla Firefox has wm_class firefox while Chrome has a wm_class google-chrome) which is why we didn't end up using it to activate the player. More importantly, some apps like Amberol and Rhythmbox can litterally be closed and still play music so they have no open window.

However, it was close enough as a backup so if we are merely using it to do a best guess for whether the source was minimised or not, knowing that as soon as the app is activated, we will have a reliable ID for the window, that should be good enough?

[Moon-0xff]

I was wrong about AppSwitcherPopup being part of Main, the only reference to the object is on ui/windowManager

By the way, I investigate the gnome-shell code mostly using grep:

$ cd gnome-shell/js/
$ grep -ri AppSwitcherPopup

ui/windowManager.js:            constructor = AltTab.AppSwitcherPopup;
ui/altTab.js:/* exported AppSwitcherPopup, GroupCyclerPopup, WindowSwitcherPopup,
ui/altTab.js:var AppSwitcherPopup = GObject.registerClass(
ui/altTab.js:class AppSwitcherPopup extends SwitcherPopup.SwitcherPopup {

[Moon-0xff]

Code works, but it's a lot more complicated than it needs to be, I also don't see a real advantage in storing the player window, see commit above.

[Moon-0xff]

That's all i guess, please review the current head, i might be missing something.

I'm also curious about this line: app_window.activate(global.get_current_time()) Why it needs the current time? Can you share a link to the documentation?

[Moon-0xff]

I'm not very satisfied with the changelog writing, if you can explain the change better, please do

[Batwam]

altTab info is available here: https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/main/js/ui/altTab.js

The reason I was storing the window is because guessWindow() isn't as reliable as opening the app using the .desktop for the reasons described here. Picking the focussed window just after the app is activated seemed like a good way to identify that window reliably.

The main reason we didn't use this method to focus the app originally is that some apps like Amberol or Rhythmbox can be running in the background and have no window at all and therefore can't be activated with this methog. However, for this particular application where we just want to see if the window is open, then it should work ok.

Because of the secondary issue with a window for spot which could be mstaken for spotify using a simple .include(), I would as a minimum suggest to include a first pass identification with exact match identity == wm_class to minimise the risk of false positives. Finally, I am not sure if there are other issues with wm_class information not being accurate enough to allow identification so if we really have to avoid saving the player_window at all cost, we might need to check with some other apps to see what wm_class they report.

With regards to minimizing, I'd really like to maintain the functionality as I personally minimise windows to declutter my desktop so maximise/minimise on click is what I would expect the open app action to do. This can be done in addition from the existing code and if the user doesn't minimize windows, it won't make any difference so I don't think that it takes away from the experience. Can we put this back in?

[Moon-0xff]

Ah, that's a good reason to store the player window, and maybe it's better to do it on the constructor function then.

Regarding the minimizing, I don't feel like both actions can coexist, maybe adding a behaviour option would be the best way to do it but honestly i feel like minimizing windows is pointless, and isn't a vanilla feature of gnome since 3.0 (i think) so it looks like the gnome devs think the same.

[Batwam]

While I get that it's not part of vanilla gnome, the minimize function still built-in and can be enabled in dconf or with gnome-tweaks so I don't think that we can assume that users don't use it: gsettings set org.gnome.desktop.wm.preferences button-layout "appmenu:minimize,maximize,close". It's also enabled by default in major distros including Ubuntu as far as I can tell.

If you look at the way I implemented it originally, it simply minimises if the player was originally minimized. I am not looking to minimise the window on second click by default, only if it was already minimized in the first place. For anyone not minimising windows, there is no impact so they can definitely both coexist. The idea is to restore the player window to its original state from before the click, nothing more.

[Moon-0xff]

No. I didn't noticed that behaviour. Then sure, sounds nice.

[Batwam]

see proposed implementation in the last commit for the minimise if previously minimised.

Note that I haven't touched the player_window part so it's still based on _guessAppWindow() for now but I made it a little bit more robust by starting with an exact match which works with the majority of players like spotify, rhythmbox, ... just not the browsers which seem quite liberal with the naming convention...

[Moon-0xff]

Looks good. I don't have anything more to add to this. If you think it's good for a merge, then I will merge it.

[Moon-0xff]

Oh! Almost forgot, we should get the player window on init right?

[Batwam]

We could but we don't have to.

Currently, we are scanning for it on every click using guessAppWindow. As long as we can trust guessAppWindow() we don't need to save it unless we feel like it's inefficient to run it on every click.

[Moon-0xff]

the unreliability comes from the fact that some players can play music without an open window, as you mentioned it. So it would be better to get the player window on init when the player window is more likely to be open.

But now that i think of it, if the player window is closed, then the action would be to activate the app, which would (hopefully) bring the window back, so there's not a reason to add this. Furthermore, the new window would probably be different than the one we catch on init, so now the function is broken.

Well then, i would revert this last commit but maybe you have another idea. If you revert the commit and give me the signal i will merge this as previously discussed.

[Batwam]

Knowing the window upfront is only needed to work out if it is minimized or not. This is what I originally introduced guessAppWindow() for and used the term "guess" to highlight that it's not 100% reliable.

Technically we don't need to know the app window upfront to activate it as we use the .desktop only for that.

What we could do if we are worried about the window changing is overwrite this.player_window using get_focused_window() right after app.activate() just to be safe so the value gets refreshed every time we do app.activate().

[Moon-0xff]

Maybe I'm looking at this wrong but it seems to me that we don't need to worry the window changing. In any case is up to you if you want to add more changes to the branch. I definitely feel like this is already complete but there's no harm in adding some safety guards against probable edge cases.

[Moon-0xff]

I'm going to bed now by the way, see you in 10 hours.

[Batwam]

based on my checks, the rhythmbox window will indeed change if rhythmbox is closed but the player won't be reinitialised so we definitly can't identify the window on init. We don't need to worry about the window changing as long as we identify it within activatePlayer(). Also, every time we save a value, there is a risk it may not longer be the current one on the next click as we don't monitor (luckily) what the user does between the two click and he/she could have re-spawned the window in the meantime.

On this basis, I think that the current implementation is best until someone runs into a situation where _guessAppWindow() fails to identify the correct window but even then, I would only expect this to mean that the player would be activated on each click so it's not like it would open the wrong app.

[Batwam]

Confirmed to work fine with chrome, firefox, shortwave, spotify, vivaldi, rhythmbox, chromium, opera, amberol, brave, vlc (not sure if you tried looking glass before (alt+F2 -> lg) but it's quite useful to get the wm_class and .desktop

[Batwam]

just one more thing... if I have the player on a separate workspace, the first click will take me there. The second one however will not take me back to my original desktop. it will focus whatever window (if any) is secondary on that workspace after the player.

Edit: my bad, global.get_window_actors() does return a ful list but it turns out I had a conky window inserting itself between the player and the focused window when switching. I noticed that these windows have a different window_type and so does Gnome-Shell so I'm filtering these out and selecting the second last entry since Gnome-Shell is also excluded from the list. Normal windows all have code 0:

[Batwam]

arg... I was a bit puzzled by why these conky windows "inserted" themselves so I thought I'd try to open another app on the second workspace alongside the player. Turns out that window will go in the list before the app previously focused so the extension will switch from the current workspace to the one with the player, then instead of coming back to teh original workspace, it will cycle between the windows on the other workspace. Essentially, the windows within the other workspace take priority in the list and it will cycle between them. The previous method only works if the player is the only app on that other workspace which we can't guarantee...

The only easy way I can think of would be to keep things, simple, go back to square 1 and simply save the focused window before activating the player as that method just works. It also simplifies the code quite a bit but it's a bit less "smart"... let me know what you think.

[Batwam]

note that the Super+Tab seems to be getting the list right. just not sure how to access this list... it must be burried somewhere here: https://gitlab.gnome.org/GNOME/gnome-shell/-/blob/main/js/ui/altTab.js

[Batwam]

ok, I guess you knew I wasn't going to give up that easily...

It looks like Shell.AppSystem.get_default().get_running() is giving us the list we need and entry [1] returns the second app which is the one we are looking for!

[Moon-0xff]

Workspaces didn't even come to my mind! Good catch. As mentioned the approach is somewhat flawed but it looks to be on the right path.

[Batwam]

ok, I had to give up on the fancier alt+tab equivalent and simply saving the window as it just makes things much easier and allows not to worry about workspaces, only the window.

The reality is that most people will expect the action to flick between app/workspace in quick succession so this implementation should make sense. I also checked and there is no issue, even if the previously focused window is closed before the button is click again.

[Batwam]

more of a nice to have as it's quite robust but I'd also still prefer using _guessAppWindow() for initial guess only since we have a reliable way to get the window by checking the focused window right after having activated the player. This saves having to cycle on every click which is relatively wasteful. Even if the window was to close like with Rhythmbox, it won't match with focusedWindow for the next click so it will go straight to activate()and load the new window.

[Batwam]

Sorry, still doing some checks as the latest changes seem to have had some unintended consequences on some edge cases. Please don't treat this as final.

I'll need to put the workspace save back in as well for the scenario you had previously when clicking from a workspace without windows.

Could you revert everything back to d618a4af80cca43d907e8bcd1d87e886f9ff8e8b? I'm not sure how to do it.

[Moon-0xff]

I'm not sure how to do it.

$ git restore --source d618a4a players.js

I suggest you to consult the manpages, if you don't already do. Very enlightening $ man git

[Batwam]

ok, here is a new solution which I believe is more robust. We can positively match between Shell.AppSystem.get_default().lookup_app(this.desktopApp) and global.get_window_actors() using the pid info available in both. I believe that this is more robust than the previous implementation which involved some guesswork I wasn't a fan of. With this method, we have 100% match rate as far as I can tell.

[Moon-0xff]

using pids would indeed be more robust. But what happens if two instances of one player(application) are running simultaneously?

I think I know the answer of this one (we can't know which one is the corresponding player unless we are able to get a pid from d-bus) but I'm curious about the output.

[Batwam]

yeah, that did cross my mind. This is most likely to occur with browsers and as far as I can tell, both windows have the same PID so they are impossible to tell apart from each other. I'm guessing that a browser window is just like another tab and sharing the same PID. See below, both chrome windows have the same PID:

Even if we were to try to save the window info, the reality is that activate() wouldn't be able to know what to activate either.

[Moon-0xff]

Now, we might be able to get the pid from a player, long ago i noticed this line on mpris-indicator

[Batwam]

yeah, I think that if we can get the pid, we could potentially do without the .desktop altogether, as long as we can extract the icon info we need from the window using window.get_icon_geometry(). That could clean up a lot of what we had to do to reliably identify the .desktop with Gio.DesktopAppInfo.search(this.identity) and having to match against running apps. I don't believe that the .desktop is used elsewhere.

Probably one for a separate PR though please :-) I didn't expect this one to take this long!

[Moon-0xff]

Software development! Small changes? think twice!

I think it has some comedic value. A favorite of mine is this one: (08:00) youtube.com/watch?v=T-BoDW1_9P4

[Moon-0xff]

I agree to work in a pid-based implementation in another PR, and i have no problems merging this. Although i haven't tested the current version.

[Batwam]

ok, please have a play around just to make sure I haven't missed anything. I have checked with the VM containing every mpris player known to mankind but there could still be situations ike the one you picked up previously with the empty workspace.

[Moon-0xff]

Already did, and on my testing behaves as it should. Should i merge this? I will merge this.

[Batwam]

Ok, let's do it!