Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popup menus and MouseDown/MouseUp problem #3116

Closed
artscoop opened this issue May 3, 2014 · 17 comments
Closed

Popup menus and MouseDown/MouseUp problem #3116

artscoop opened this issue May 3, 2014 · 17 comments

Comments

@artscoop
Copy link

artscoop commented May 3, 2014

I've discovered a bug, revealed by three menu applets (including the Cinnamon menu). I can reproduce this everytime. (Cinnamon 2.2, Ubuntu 14.04)

  • I click on the menu icon.
  • While the menu is not yet visible, I move quickly to some position, approximately the expected position of a shortcut in the menu (I have about 300ms to do that)
  • When the menu gets visible, the icon under the pointer is considered clicked : the app opens and the menu closes immediately, even though I have never clicked that icon

This has to be related to some behaviour on popups, and the bug can only be reproduced on popups taking enough time to appear (more than 250ms is enough)

@collinss
Copy link
Member

collinss commented May 3, 2014

This is a regression introduced by #2779. I already mentioned the issue, but I haven't heard any word about it since. I did look into a potential fix, but it's not a trivial matter (unless the change is reverted, which I think is unlikely). The problem is that Cinnamon and Muffin don't handle the button release event until after all of the callbacks for the button press are handled (including opening the menu). It is only then that the mouse position is polled, which means that by then you've had ample opportunity to move your mouse to somewhere else (eg over an item in the now-open menu), so it thinks you released the mouse button over the menu item, even though you actually did so over the applet button. I've noticed that this problem happens to me most often when I'm in a hurry, and can least afford the wasted time of accidentally activating the screen-saver when I'm trying to shut down.

@artscoop
Copy link
Author

artscoop commented May 3, 2014

This makes perfect sense. Obviously this behaviour is likely to introduce bugs. I understand the idea of waiting for callbacks to be run before registering a "mouserelease" (at the wrong place), then what is needed is a complete mousepressed+mousereleased (a full click) event... Hope it can be done without too much hassle.

@mtwebster
Copy link
Member

We're aware of this and will likely do something about it before 2.2 is final...

Personally I'm trying to speed up the popup opening process - this really isn't a problem any time except the first time you open a particular popup. a great deal of calculation goes on to determine what the popup should look like (things like, where to position the pointy arrow thing on the outside of the popup) - this only occurs on the first open. I'd like to have that precalculated, but it's a challenge.

For instance, gtk popups (like context menus) behave the same way, but they're so fast to display it's generally not an issue (actually considered a feature by some).

Worst case we can 'eat' the first mouse up so it's ignored in these cases.

@ghost
Copy link

ghost commented May 3, 2014

mmm, and what about change also of all popup menu to mouse pressed instead mouse release?

@ManIVIctorious
Copy link

One little question: would it be possible to load the applet after button click and only display it after button release or wouldn't that be possible/to hard to implement?

@ManIVIctorious
Copy link

Sorry, just saw that this has been asked 3 posts before...

@collinss
Copy link
Member

collinss commented May 3, 2014

@lestcape, The idea with the current method is that you can press the mouse button, move the mouse over the item you want, and then release the button to activate the item (so the whole process is accomplished with a single 'click'). If possible, this behavior should be preserved. I think @mtwebster's idea of 'eating' the first button release event would be better than changing the behavior of the menu items (this would fix other things as well such as how the menu gets closed if the mouse up is registered over an empty space).

@ghost
Copy link

ghost commented May 3, 2014

@collinss this did not start here... see: https://github.com/lestcape/Configurable-Menu/issues/50

And you have right, but i do not think only on the menu, fix the cinnamon menu it's relative easy, the problem it's that, this happend for all popup menu, and we need a general solution, blocking the event take time and resources.... Change the event not, with this only you can notice that all react more faster on cinnamon. This is the same than before when the event change, but now for all actions.

@ghost
Copy link

ghost commented May 3, 2014

When @Garibaldo have the idea of change the event also i comment about this. See the origin:
#2597

@ghost
Copy link

ghost commented May 3, 2014

@ManIVIctorious to load the applet, the applet need to be visible first, this is the only thing that cause a little delay now on cinnamon menu, because this can not occurs when the applet it's not visible.

@ghost
Copy link

ghost commented May 3, 2014

Another easy thing can be store the last actor that receives a mouse pressed using global and on mouse release see if the actor it's the current actor that receives a mouse release also commented by me on the same thread #2597.

@ScipioAfricanus
Copy link

"...what is needed is a complete mousepressed+mousereleased (a full click) event"
I can't really believe you guys are suggesting going back to mouse press+release in order to open the menu. That would be a step backwards, literally a regression, in my opinion. Cinnamon menu and popups are more responsive now, after that change that eliminated the need of mouse releasing to activate events. That small bug you guys are complaining is by no means reason enough for reverting that important improvement in snappiness Cinnamon has gained in the last few months. So, please, don't even consider returning to the previous behavior. That would be a shame.

@ghost
Copy link

ghost commented May 3, 2014

@ScipioAfricanus, Take it easy, do not all have your expertise. Anyway here is not decided anything. This will be decided in the IRC by the developers of cinnamon. Any opinion is not important, also if it's worng?

@artscoop
Copy link
Author

artscoop commented May 3, 2014

@ScipioAfricanus I would agree with what you say if only this worked perfecty. But I consider that having a ghost click registered at the wrong position is a bug.

@ghost
Copy link

ghost commented May 3, 2014

@artscoop at less this is not a bug any more on Configurable Menu... You can test the last version....

@ghost
Copy link

ghost commented May 3, 2014

On applet actor pressed set press = true
On applet actor release set press = false
On all popup menu inside the applet menu only allow mouse release if !press and on mouse release do also press = false

When the menu applet close do press = false
on the menu actor do press = false on release

This was all... And without change the behavior of any thing...

@artscoop
Copy link
Author

artscoop commented May 3, 2014

@lestcape And this worked flawlessly. Yesterday I could still reproduce the bug with the reduced delay, but I had to do it on purpose. Now, even trying to reproduce the bug does not work anymore, and the menu has become also quicker to appear with the keyboard shortcut. Excellent.

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

No branches or pull requests

5 participants