Windows doesn't get unminimized after "hide-all-normal windows shortcut"

[micheleg]

bad63r on the extension site says: "I don't know is this a bug or shold be like that but : when you make keyboard shortcut-navigation-hide all normal windows, and then you try to unminimize it via click on icon on dash it is not working(i'm talking when you have chaked option in dash to dock settings to minimize programs on clik on icons on dash)"

More info: It works with the cycle through windows option but not with the minimize on click option.

[franglais125]

I have been trying to reproduce this but I haven't been able to do it at all.

I tested this on both 3.14 and 3.20. @micheleg can you reproduce it?

[micheleg]

Yes, on 3.14 (Debian Jessie). Set the click option to minimize and enable the hide all normal windows shortcut. Hide all windows and then try to click on the launcher. Nothing happens. Instead, with the cycle though window option it seems to work normally.

[franglais125]

Ah! I managed to do it now... I didn't have the bug as I use "Icons on desktop". Essentially focus was being given to the desktop, so technically not everything was minimized!

[franglais125]

I think I found out what the problem is.

• I can't reproduce the bug on 3.20, so I guess that's good news...
• On 3.14, use the Built-in theme, so that the 'focus' style is used (for the next test)
• Minimize all windows (e.g. Super+d)
• The last focused app hasn't lost focus! (If you try another app it works).
• Hence, this condition is 'true' https://github.com/micheleg/dash-to-dock/blob/gnome-3.14/myDash.js#L1508
• activateAllWindows never gets called

I don't know that there is an easy way to fix this... The focus is not removed by the command (this seems fixed in 3.20, I don't know about 3.18, 3.16).
We would need to use something else than app.focused

[micheleg]

I'm not sure when the change happened. Maybe https://git.gnome.org/browse/mutter/commit/src/core?id=6c05eb583e10293b9979cd254a77cf9ea02473b1

In any case, as a workaround, what about adding:

                else 
                    this.app.activate();

on line https://github.com/micheleg/dash-to-dock/blob/gnome-3.14/myDash.js#L1515?

This should restore the default behaviour, which unfortunately is not optimal as it show only one window.

Maybe we could add a check of this kind instead:

                else if (!global.display.get_focus_window().showing_on_its_workspace())
                        activateAllWindows(this.app);

?

[franglais125]

Thanks for the suggestions! Using global.display.get_focus_window().showing_on_its_workspace() I managed to fix the problem. See patch [571fc30].

Can you test this on 3.14? As I said before, this doesn't happen on 3.20, but I don't know about 3.16 and 3.18.

[micheleg]

Can you test this on 3.14?

I've just updated my main machine to 3.20! I'll check on another system. I'm not sure right now if it is correct, but one problem that I see: what if get_focus_window() returns null?

I would also add comments to document the workaround (and maybe we can remove for Gnome Shell 3.20+)

[franglais125]

Ah, good catch! I'll make it cleaner and add comments (not right now, but maybe by tomorrow).
And hey, I guess Debian Stretch was getting irresistible?

Btw, I am preparing a PR for #148. I am trying to improve on this one too: #347.
I'll get this to you tomorrow or the day after.

[franglais125]

(Apparently my comment was never published, trying again...)

With this patch [2be8eb0] and this one [59abadc] I moved the fix to the minimizeWindows function, by calling activateAllWindows, which is what should be called anyway. I also reverted the previous patch.

What do you think?

[franglais125]

I just checked the problem on other versions:

• 3.14: needs to be fixed
• 3.16: needs to be fixed
• 3.18: I couldn't reproduce the bug

So apparently only 3.14 and 3.16 need to be fixed!

[micheleg]

I moved the fix in the activate function (https://github.com/micheleg/dash-to-dock/tree/fix_unminimization). It makes more sense there: the minimizeAllWIndoes function should always do what its name states.

[franglais125]

Sounds good. Do you want me to test the branch? Or we can call it done?

[micheleg]

I quickly tested it, but if you can test it further to double check it
would be better. In any case, if I decide to backport the new features
now in master this patch will have to be adapter in the future

On Thu, 14 Jul, 2016 at 10:12 PM, franglais125
[email protected] wrote:

Sounds good. Do you want me to test the branch? Or we can call it
done?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[franglais125]

I had to apply a small fix [72a51ce]. I just added "this." where needed, and removed "settings" as input.

This is actually my fault, the patches I linked in this thread are from a branch that had the workspace isolation implemented. I rebased my 'gnome-3.14' to yours, and then separated the other stuff in a new branch, as I use locally for 3.14!

[micheleg]

Ok, I updated the branch.

[franglais125]

It seems we are never done with this, but I think this is the last time I bring it up.

Here is a small patch [7f09eec]. We are indeed checking that we don't get a null, but if global.display.get_focus_window() is indeed null, nothing happens!

[micheleg]

I think you can safely unify the if; Javascript should not evaluate the second part of the expression if the first part is already false.

if (global.display.get_focus_window() !== null  && !global.display.get_focus_window().showing_on_its_workspace()) 
    activateAllWindows(this.app);
 else
    minimizeWindow(this.app, true);

[franglais125]

Ah, didn't know, but suspected as much.
I updated my 3.14 branch with [c30c1ec].
https://github.com/franglais125/dash-to-dock/commits/gnome-3.14

I think the exact same fix works on 3.16 too.

[micheleg]

Ok, fixed in both the gnome-3.14[7d9b562] and the gnome-3.16 branch [7d9b562].