fix: Resizing Applet PopupMenus will go behind panel if panel is set to auto hide

[Gr3q]

See Weather Applet if the hourly weather button is clicked and the panel is set to auto hide.

The problem is that the panel reports it's not visible even though it is, so the x,y calculation for PopupMenus are incorrect.

Panels are not hidden until global.menuStackLength is bigger than one (meaning a PopupMenu is open for example)

Note I do not know what manipulates global.menuStackLength so it contains something something else other than applet PopupMenu states this change probably will have unintended side effects. I checked it looks like it's only for PopupMenus.

Also we don't know which panels the stackItems belong to, so this will report that all of them are visible. In theory this should not be a problem because you are only able to open only one, right?... but I somehow doubt that.

Now this PR has an opinionated decision to show panels that has open popup menus. This seems to be the easiest way to make sure panels report the correct visibility and popupMenus have correct positioning with using both mouse and keyboard bindings to open them.

• Fix Toggle switch for the hourly weather display (weather@mockturtl) cinnamon-spices-applets#4964

[ghost]

Not sure whether there is any difference in regard to this issue between Mint 19.2 and Mint 21.x but at least here in Mint 19.2 it seems to be some delay in either reporting the real value of global.menuStackLength, the updating of this._shouldShow, or both.
I say this because, after applying this suggested fix to panel.js and restarting Cinnamon the bug is still present at various times just as it has been before. The only difference is, now it fixes itself after a certain amount of time, while previously it would only fix right after the restart of Cinnamon and would stay buggy after a certain amount of time until next Cinnamon restart.

Below there are two screenshots. I used a previously modified version of the 'Download and upload speed' applet which adds a blank space to the tooltip/menu in order to circumvent the bug.

After applying the fix and restarting Cinnamon all was fine. After a small amount of time the menu position again got miscalculated (first screenshot).

I kept the pointer over the applet icon and after an amount of time (less than the clock in the panel shows as difference) the menu jumped by itself into its correct position (second screenshot).

I can only speculate that either or both of the values involved in the calculation are being polled on a timer, maybe in order to avoid some heavy calculations or whatever. Bottom line is the bug will still show apparently randomly if the fix will be left as is.

I also wonder who and why has forbidden me to reply to the original issue where this bug has been reported:

[fredcw]

This PR has a side effect: In cinnamenu applet, if I open the applet with the super key when panel auto hide is on, the applet is positioned higher up as if the panel is showing even though it isn't. This also happens in the menu applet if it's opened with the super key and "force panel to be visible when opening the menu" option is off in the menu applet config. I can easily fix this in cinnamenu though by always using peekPanel(). In fact the only reason I removed peekPanel() was because of bug #10970

[Gr3q]

It feels like the default behaviour should be that if a popupmenu is visible for a panel the panel should always show.

It probably needs some work on the stack variable so panels can use that. This way we could depend on it properly and eliminate other problems too.

[fredcw]

@Gr3q >It feels like the default behaviour should be that if a popupmenu is visible for a panel the panel should always show.

I agree. There seems no useful purpose in having an applet open and not have the panel show at the same time and it seems an unnecessary complication to the panel.js code.

[ghost]

This PR has a side effect: In cinnamenu applet, if I open the applet with the super key when panel auto hide is on, the applet is positioned higher up as if the panel is showing even though it isn't.

It happens with any menu applet, even the default one. Not only that, but having another autohidden panel placed vertically to the same side where the menu is would make the menu open shifted horizontally for the amount of pixels equal to vertical panel's width.

There seems no useful purpose in having an applet open and not have the panel show at the same time [...]

At first sight that'd be correct but thinking thoroughly I could envision an applet that does some automatic work in the background and could be instructed in the code to show a popup menu in certain situation(s) without the user acting upon the applet or the panel itself. This may be the situation for which the panel visibility check is in place. I'm not sure though whether the popup menu display would force the panel to show, or not.

[fredcw]

Using:

return !this._hidden;

instead of:

return this._shouldShow || global.menuStackLength > 0;

seems to fix the problem with applet resizing though it doesn't fix the peekPanel() bug I mentioned above. Well. it does some of the time with the menu applet but not all the time strangely.

[ghost]

Well, it does some of the time with the menu applet but not all the time strangely.

The only explanation would be that neither this._hidden nor this._shouldShow or any other variables that deal with panel's visible state are getting their values in real time. As long as there is a delay in retrieving panel's real state nothing will ever work 100% correctly - not a popup menu, not anything else. So that's where the real issue is. Well, in my opinion at least.

[fredcw]

@Drugwash2 I think I've found the solution to this in cinnamenu and the menu applet. When the super key is pressed, peekPanel just needs to be called before the menu is toggled open.

In menu applet line 1321:

if (this.forceShowPanel && !this.isOpen) {
    this.panel.peekPanel();
}
this.menu.toggle_with_options(this.enableAnimation);

[ghost]

When the super key is pressed, peekPanel just needs to be called before the menu is toggled open.

Looking at the panel.js code it seems peekPanel() just displays the panel through _showPanel() and sets this._hidden to false. While this may be a forced way to have the popup menu calculate the correct position the user may not want the panel displayed when invoking the menu through the keyboard shortcut.

And implementing such workaround in each and every applet hardly seems like the correct solution. Shouldn't this be cared for in the popup menu code itself? I'm not familiar with the code but the principle would be for the popup menu code to actually check for the real panel's visible state (something similar to isWindowVisible() in MS Windows) right before calculating the menu position and showing it - not to rely on some variable whose value could be inaccurate due to a certain delay, or on a workaround that forces the panel to be displayed even when that may not be desired.

[fredcw]

@Drugwash2 peekPanel is only called by an applet when it wants to show the panel so this is up to the applet. For instance, you can turn off the option "force panel to be visible when opening the menu" in the menu applet, in which case peekPanel is not called and the panel is not shown.

[ghost]

peekPanel is only called by an applet when it wants to show the panel so this is up to the applet.

Well, I tried using peekPanel() in 'download and upload speed' (appletGui.js > _on_hover_enter() ) in conjunction with the originally proposed fix, and it works exactly the same as in my previous comment (with the screenshots): popup appears behind the panel and after one or more seconds it moves to the correct place. Could be a problem with hover detection? Anyway, this is in Mint 19.2 as the 21.2 VM starts up very slowly (loaded over LAN) and weighs kinda heavy on this old notebook so I rarely fire it up.

[fredcw]

Download and upload speed applet seems to be working fine with this fix (mint 21.2)

[ghost]

There may have been some improvement in the area between 19.2 (Cinnamon 4.2) and 21.2 (Cinnamon 5.8). Since the 19.x line became irrelevant for most people my test results shouldn't matter much if anything. Hopefully it will keep working correctly on all newer Cinnamon versions.

Let's see Gr3q's results with the Weather Applet.

[Gr3q]

Ok, it's changed now. The PR fixes most of the issues with positioning at the cost of that if a panel has open menus it will automatically be shown.

Download and upload speed must be a special case, and I can take a look at that as well, but the fix probably will be in the applet's code.

[ghost]

The PR fixes most of the issues with positioning at the cost of that if a panel has open menus it will automatically be shown.

I'll wait for things to settle before testing this fix in Mint 19.2. It feels kinda hacky though to force the panel to be shown even if it wasn't intended to. Now, I don't personally know of any applet that would display a popup menu out of the blue without user interaction with the panel, but nevertheless in theory it could exist. Which means this fix might defeat its purpose. Admittedly it could be treated as a corner case, but too often corner cases are being easily dismissed only for later on to create problems.

As for 'download and upload speed', that applet displays the popup menu as a tooltip replacement upon hovering of applet's icon. So in theory the panel should be visible at the time and there would be no need for a fix. Practically though the visible state of the panel seems to be determined much later, at which point the popup menu had already been displayed at the wrong position. It's weird. As if panel visibility is only being detected correctly upon a mouse click (on an applet icon) but not on hover.

Because of this annoying issue I had implemented a switch in the settings for a mod of this applet, that would allow the display of a regular tooltip instead of the popup menu, and which seems to work properly. I wonder how the position of a tooltip can be detected so accurately in all situations, while the popup menu's can not.

[Gr3q]

The positioning problem comes up when a popup menu resizes after its open.

before the panel reported it's not visible but refused to hide If a menu was open, and when the menu repositioned itself on the next resize according to the state the panel reported.

[ghost]

before the panel reported it's not visible but refused to hide If a menu was open [...]

So practically all that needs done for things to work correctly in all possible scenarios is to make sure the panel always returns its real visible state when queried (use Gtk.Widget.get_visible() or Gtk.Widget.is_visible() ?).

[fredcw]

@Gr3q What wrong with my fix, it has the advantage of being a one liner?

[ghost]

Here's what I've been thinking.
In popupMenu.js > _calculatePosition() there's a check commented as:
// remove visible panels from workable area to avoid overlapping them
Within this check there's this line:
if (!panel.getIsVisible()) continue;
Seems to me this could - and most likely does - provide erroneous results due to the elusive to me this._shouldShow in panel.js.
As such I thought panel.js > getIsVisible() could be better off returning this.actor.is_visible() instead of this._shouldShow.

I believe this would provide instant accurate visibility state of the currently queried panel. What do you guys think...?

Scratch that, it seems it always returns true. Darn!

[ghost]

Download and upload speed must be a special case, and I can take a look at that as well, but the fix probably will be in the applet's code.

And so it seems. I have modified appletGui.js > open() as follows and that appears to have fixed the issue: No, it doesn't. This is getting ridiculous. 😡

	open: function(){
		this.menu._calculatePosition(); // <== added this
		this.menu.open();
	},

[Gr3q]

@Gr3q What wrong with my fix, it has the advantage of being a one liner?

Your fix is ok, I simply missed that comment.

I pushed the previous fix to another branch in case we need that behaviour. Also added one more line so panels will recheck it on popupmenu open.

And so it seems. I have modified appletGui.js > open() as follows and that appears to have fixed the issue: No, it doesn't. This is getting ridiculous. rage

open calls _calculatePosition internally. With the changes in this PR now it really shouldn't be under the panel.

[fredcw]

@Gr3q Nice. Shall I create a separate PR for the menu applet or should it be included in this one? It's basically just deleting the 3 lines in _onOpenStateChanged():

if (this.forceShowPanel) {
    this.panel.peekPanel();
}

and replacing them with the 3 lines in the comment above in _updateKeybinding()

Probably better to create a separate PR I'm guessing.

[Gr3q]

I think another PR would be better if it still has a timing issue with positioning when opened.

[fredcw]

@Gr3q This commit needs a better description. Also:

Also added one more line so panels will recheck it on popupmenu open.

This doesn't seem to make any difference.

I can add this commit to my PR if it's easier.