Git Product home page Git Product logo

Comments (39)

vkareh avatar vkareh commented on August 15, 2024 1

Summary time. This is my experience so far:

  • Marco and Compiz <Mod4> shortcuts work just fine, at all times, regardless of any other shortcut settings (assuming #59 has been applied).
  • mate-settings-daemon (which handles bindings for the MATE Keyboard Shortcuts app) <Mod4> shortcuts do not work whenever there is another application with a shortcut bound to the Super_L key.
  • An application's <Mod4> shortcuts do not work whenever there is another application's shortcut bound to the Super_L key.
  • Super_L shortcuts take precedence over <Mod4> shortcuts.

Apart from the first item above, this is no longer a brisk-menu issue. It's a MATE issue.

It seems that whatever higher entity is handling global keybindings in MATE has an issue with the Super_L/<Mod4> pair. Since all key grabs are done directly through XLib, it all points to mate-settings-daemon, unless there's yet another entity handling global keybindings, @flexiondotorg?

I would try to change mate-settings-daemon to handle shortcuts on KeyRelease, rather than on KeyPress to see if that would solve the problem, but I have no idea how to even start compiling and testing it without messing up my computer... VM time?

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024 1

Welp, it seems I was wrong... I recompiled mate-settings-daemon without the key grabs, and the behavior persists between brisk-menu and mate-menu. Seems like any program that attempts global key grabs, if one of them is using Super_L, all others using <Mod4> don't grab it anymore. I'm not aware of any other programs that do global key grabs, so I'm not testing with anything else at the moment.

  • <Mod4> key grabs work well as long as Super_L is not bound
  • When Super_L is bound, <Mod4> shortcuts don't even grab (it's not even a filtering logic issue - it just plain does not grab it)

I still refuse to blame XLib for this, so I'm going to focus on the different permutations of modifier masks set during the grab phase. Maybe something along the lines of "if the bound key is Super_L, remove/add Mod4 masks to the XGrabKey call", who knows... I'm just hoping that I don't have to go the route of coding "if any other program has a bound Super_L, and this one is Mod4, then change this and that when calling XGrabKey". Not sure if I'm explaining this right, I'm running out of ideas.

Research and tinkering continues...

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024 1

w00t! That was some seriously painful spelunking, but I submitted a pull request to the mate-settings-daemon project that fixes this issue for brisk menu.

From what I can see, any program that expects to register global keybindings that may have similar issues (mainly using the <Mod4> key), should implement a similar fix. I'll try to do the same for brisk-menu.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024 1

hmmm - it's probably not necessary for brisk-menu to implement this, as it seems low probability that someone has brisk menu running, mapped to a <Mod4> shortcut, and have a different app (most likely another menu) mapped to Super_L.

I'm saying this in part because this change would mean that brisk-menu needs to request event reports from the window hierarchy of every screen, which seems both unnecessary and very likely a bad idea. The MATE settings daemon, on the other hand, is already aware of all the windows by definition, so it's relatively benign to expect it to be aware of keybindings sent to application windows.

I think mate-desktop/mate-settings-daemon#179 should take care of this issue at a general level, and any other menu/launcher that feels this is necessary can make that call independently.
@flexiondotorg, @ikeydoherty - thoughts?

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

If other things are hijacking it then you can change your keyboard shortcut to not use Super_L.
Given your hijack issue its probably why it's not closing again after.

from brisk-menu.

the-mentor avatar the-mentor commented on August 15, 2024

@ikeydoherty thanks for the quick reply.
the problem is that brisk is hijacking the shortcuts of other programs.
if you want to reproduce the issue install "albert" launcher set the shortcut to super+space and then try to launch albert using the super+space key.
you'll see that it doesn't work.
if you use the right super key + space it works.
this seems to be caused by the latest update to brisk.

edit: i also use a bunch of other shortcuts that use the left super key that don't work.
for example i use left super + right to snap a window to the right and that doesn't work.
its a real bummer since this update made my existing workflow unusable.

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

Gotcha.

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

Is Brisk showing when you release the button or before?

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

Confirmed that the hungry bastid is eating it

from brisk-menu.

the-mentor avatar the-mentor commented on August 15, 2024

Brisk shows when i release the button.
also I'm using Solus MATE if that changes anything.
i have no idea what is bastid but i'm happy you confirmed whats causing the issue.

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

@ikeydoherty I tested the original pull request for Super support it was working correctly and not swallowing Super for other applications.

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

@vkareh Can you take a look?

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

Of course! Just tested the latest from master and I can experience the following behavior:

  • When the menu is closed, other Super bindings get swallowed sporadically (I can move certain windows, but not others, using Super+Left/Right/etc). Sometimes it works once or twice, then stops working until I open and close the menu.

  • When the menu is open, pressing on the Super_L key closes the menu, and other Super bindings (called within the same key-press) work as expected. I don't have a Super_R key, but I suspect it shouldn't ever be an issue, since combo shortcuts use <Mod4>+Something, whereas brisk-menu uses <Super_L> specifically.

  • Closing the menu seems to reset whatever state the keyboard binding is reading, which gives me the impression that the issue might be in the logic within the brisk_key_binder_filter function, or thereabouts...

I'll play with the key-binder code throughout the day and see what I can figure out. I'll report back as soon as I have something.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

Okay, here are some of my findings:

  • When doing a Super+something shortcut, the brisk-menu code sends the event (using XSendEvent) to a NULL window, the reasoning here was that by not setting a window, it will propagate the event up the window hierarchy until it found a window that was grabbing it, or disappear into the Γ¦ther... Seems like I was wrong, and setting the window to be default root window instead does the trick for shortcuts that involve windows (Super+Left/Right/etc). I can have a pull request ready for this at any point today, but...

  • Second, if I set Super-based shortcuts from the MATE shortcuts combination app, they don't work. However, if I set the same shortcuts from the Compiz configuration, they work just fine. I can test whether using Marco would make a difference in this (but I'm currently at work and messing around with my wm right now seems like a ripe time for a client to request an impromptu demo of something...). I will test with Marco later today and report back.

These two items seem related somehow - fixing the XSendEvent window seems to allow Compiz to finally allow shortcuts to happen on its windows. I don't understand enough about the interplay between MATE and Compiz/Marco to make a more educated guess than that, but they seem related to me...

Thoughts, @flexiondotorg ?

In fact, in testing mate-menu, which uses the exact same technique, I can get the same behavior of Compiz super-based shortcuts work well, but MATE super-based shortcuts don't... now I'm eager to test out the wm theory for both menu apps. I'll report back later today, hopefully

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

More findings:

  • I couldn't wait and went ahead with changing back and forth between Compiz and Marco - no luck. Regardless of the window manager, whenever brisk-menu (or mate-menu for that matter) is running (with the Super key), MATE shortcuts that use the Super key do not work. When using Compiz, Compiz shortcuts work just fine (once I apply my fix for brisk-menu, of course).

  • I'm not ready to blame MATE shortcuts yet, though - I want to run through a few more scenarios, but I can't guarantee that I won't clone the code and hack at it at some point ;)

  • I think it makes sense for me to send in a pull request with the fix, even if some shortcuts are still swallowed.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

okay, #59 fixes part of this issue by allowing the correct XEvent behavior during the keybinding filter. It's very much compatible with the way that the mate-settings-daemon does it (although I have issues with mate executing the bound command on KeyPress, rather than on KeyRelease, but whatever, it doesn't seem to interfere for now).

I think the way brisk-menu does the initial key grabbing during the binding phase is where the issue lies... will keep experimenting with different key-grabbing techniques. mate-settings-daemon has a grab_key_unsafe function that I'll have to explore in more detail. So far it seems promising.

@ikeydoherty - After applying #59, does the issue persist in Budgie? Or is it a MATE-only thing?

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

Budgie locks out the windows key from grabs at the compositor level so its impossible to test there

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

OK so - does this fix the original issue with Albert?

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

@ikeydoherty - it does not, as far as I know.

I think Albert needs to implement the fix for it to work. I'm curious as to why it used to work and now it doesn't. Maybe it has to do with Brisk checking for KeyRelease rather than KeyPress, but I haven't been able to get Brisk to work consistently with just the KeyPress event...

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

Yeah I ran into the same issues and then the Async KeyPress buggered up KeyRelease..

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

I can keep trying - since Brisk moved to Async key grabs, a lot has changed in terms of interoperability between X keybindings and Brisk. I will give it another shot and see where it leads.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

yeah, it doesn't work... all I can manage is to get brisk to show up once, after that the menu entry gets highlighted in the panel, but the menu itself doesn't open (if I squint the right way, it seems like it opens and closes in quick succession). I'm really out of ideas now - I've been trying to get this thing to work with just KeyPress for a few days now thinking it might help, but nothing...

Sorry, @the-mentor 😒

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

We could maybe hack it. Make Brisk add an idle/timeout callback to then toggle the menu and cancel rapid successions ?

from brisk-menu.

the-mentor avatar the-mentor commented on August 15, 2024

@vkareh don't be sorry i know you've spent a bunch of time on it and i really appreciate it!
I wonder how other menus do it.
Also if there was a way to change the hotkey that would also be good since i would probably chance it to something else since i use the Super_L key for more things just that menu.

Edit: I guess i really love my launcher and the Super_L + Spacebar key combo :D

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

@ikeydoherty that seems like a good idea - a debounce is far from a hack, even though Xlib would probably disagree.

@the-mentor, thanks for your patience and understanding. The only other menu in MATE that is by default bound to the Super key is the Advanced MATE Menu, and it has the exact same issue as Brisk.

To change the hotkey, open dconf and go to "com / solus-project / brisk-menu / hot-key" (or something along those lines, I'm on my phone and going by memory. I use Ubuntu, so it might be different for you). Also, I totally get the love for a specific hotkey, it's literally the reason I got suckered into this project in the first place :)

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

@ikeydoherty - just realized that part of the reason that it's necessary to check for both KeyPress and KeyRelease is because we're using the Super_L key - since Super is both a key and a modifier, we need to make sure that a user pressing Super did not intend to press Mod4 instead, therefore we check for both KeyPress and KeyRelease to make sure the user did not press another key in between (in which case it should be treated as Mod4, rather than Super)

In playing with this some more, if we do only KeyPress or only KeyRelease, Brisk gets triggered for both Super_L and <Mod4>, which means that the menu will open when I'm, say, snapping windows with the keyboard or other such action...

To clarify with an example, say I want to open a terminal, mapped to <Mod4>t in MATE - here's how it currently happens:

  1. KeyPress Super
  2. KeyPress t
  3. terminal opens (mate-settings-daemon only looks at KeyPress)
  4. KeyRelease t
  5. KeyRelease Super

Unless we check for both KeyPress and KeyRelease, Brisk would open at either 1 or 5, along with the terminal at 3. So in Brisk we check that nothing else was Pressed/Release in between the Super key: https://github.com/solus-project/brisk-menu/blob/master/src/lib/key-binder.c#L185-L192

None of this would be necessary with other key combinations, but to support the Super_L key, we're stuck doing it this way. I'm assuming using other Mod keys (Ctrl, Alt, Shift, etc) as triggers would have similar issues, but Super seems to be the favorite. Other key combinations do not have any of these issues... Also, MATE does not allow you to set a single modifier key as a shortcut - to support that would be a nightmare!

Does that make sense?

What this means is that we're currently stuck keeping the code as is, hope mate-settings-daemon gets their pull request in to fix swallowed keys within MATE, and let other launchers implement their own logic however they choose. Then some time in the next decade we'll finally replace X with something more modern 😝

from brisk-menu.

ikeydoherty avatar ikeydoherty commented on August 15, 2024

Alternatively we could have MSD provide the global key handlers for all MATE apps over dbus, similar to GSD. Otherwise, yes, this is the best path right now.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

MSD taking care of this would be lovely, still, they don't support single-modifier shortcuts :-/

Another alternative could be to have the different launchers/menus (Brisk, Albert, etc) provide an executable to just open its program window, so Brisk could have a brisk-menu-open program that triggers the menu being open from the panel, and then use MATE to create shortcuts for it - not very elegant, though...

I'll take a look at gnome and see how they do it... I'm already way too deep in this rabbit hole, what's a few more!

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

To recap:

  • The title of this issue ("Superkey shortcuts being hijacked/blocked") is currently an issue in MATE. It should be fixed (only for MATE shortcuts) by mate-desktop/mate-settings-daemon#179
  • The initial description (as it applies to Albert shortcuts) is currently non-fixable by Brisk

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

@the-mentor: albertlauncher/albert#479

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

Thank you such a detailed analysis of the issue. Sorry I've been quiet on this, real life and such. I'll take care of the PR in MATE.

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

I've tested mate-desktop/mate-settings-daemon#179 and release a patched version of mate-settings-daemon 1.18.1 to the Ubuntu 17.10 archive.

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

@vkareh: Please see ubuntu-mate/mate-dock-applet#91 and I'm sorting our some funding for all your hard work, it is very much appreciated.

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

@vkareh @ikeydoherty I think this can be closed, right? Patches landed in mate-settings-daemon, marco, mate-dock-applet, etc.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

@flexiondotorg - for MATE, this is resolved. However, the original report was for albertlauncher/albert. I submitted a fix for it a few months ago but it hasn't been merged...

from brisk-menu.

the-mentor avatar the-mentor commented on August 15, 2024

@flexiondotorg @vkareh @ikeydoherty the issues with MATE R_Super key shortcuts issues still exists in my Solus MATE install.
I'm not refering to the albertluncher issues i'm refering to the MATE desktop shortcuts.
Do you know if these fixes were pushed to Solus ?
My Solus is completely up to date (solus3).

Thank you very much for all your support !!

from brisk-menu.

flexiondotorg avatar flexiondotorg commented on August 15, 2024

@ikeydoherty @vkareh This issue has been resolved in Ubuntu MATE for many months. Any reason this issue can't be closed? The fact the Albert devs haven't merged a pull request is unfortunate but no reason to keep this issue open here.

from brisk-menu.

vkareh avatar vkareh commented on August 15, 2024

@flexiondotorg - Good question! I think the original issue referenced the Albert launcher. I sent a patch to the project, but it never got merged. To be honest, I don't even know if it's necessary anymore, we've pretty much addressed all the keybinding blocks in m-s-d and marco at this point.

from brisk-menu.

the-mentor avatar the-mentor commented on August 15, 2024

@flexiondotorg @vkareh @ikeydoherty i'm fine with the existing resolution.
feel free to close this and thank you very much for all the help !!

from brisk-menu.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.