p91paul / middleclickclose

Gnome shell extension for closing apps in overview with a middle click
GNU General Public License v2.0
81 stars 17 forks source link

Breaks window selection from overview by keyboard #18

Closed bbo2adwuff closed 1 year ago

bbo2adwuff commented 4 years ago

When middleclickclose (branch 3.38) is enabled I cannot select window from Overview with the keyboard anymore.

Steps to reproduce

  1. Hit Super.
  2. Navigate to window with arrow keys.
  3. Hit Enter

When I disable middleclickclose then it works again.

(I also reported it here https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3259)

p91paul commented 4 years ago

Fixed in latest master branch, uploading it to e.g.o. Thanks for the bug report.

bbo2adwuff commented 4 years ago

Wow thanks for fixing it so quick :rocket: The bug report on gitlab.gnome.org is closed already since 2 days ;)

Thanks!

p91paul commented 4 years ago

The fix for this brought worse issues (#20). I need to fix my fix, meanwhile I unreleased v18 on extensions.gnome.org.

gitbobbed commented 3 years ago

Hey Paul,

Has there been any progress in fixing this bug? Were you able to fix the fix?

Also, thanks for the great work. It's been hard to adapt to GNOME without your extension, but I had to because being able to select a window with the keyboard while in overview is super important for me.

Best,

Daniel

bbo2adwuff commented 3 years ago

Now in GNOME 40 at least graphically it seems that I can navigate with the arrow keys to certain windows in the overview.

Try following steps:

  1. Hit Super.
  2. Navigate to window with arrow keys.
  3. Hit Enter

1 and 2 work, only the Enter in steps is not working.

I don't know if that helps or if that was the case already before that 2 works!?

bbo2adwuff commented 3 years ago

I really don't have any clue about Gnome extensions, but wouldn't the following logic work?

You use if button == middleclick here: https://github.com/p91paul/middleclickclose/blob/7c1653bf00da0bc28296ce921cc79ccf4d91e6d4/middleclickclose%40paolo.tranquilli.gmail.com/extension.js#L75

What about an extra line if button == enter?

I mean it's so simple that you probably thought about it already!?

p91paul commented 3 years ago

@bbo2adwuff IIRC that handles mouse events only, so I guess it's a no :disappointed:

vjxyz commented 3 years ago

I also have this issue.. can't select a window by pressing Enter from overview if I have the extension installed 😢

pesader commented 1 year ago

Hi everyone! I did some digging while trying to fix this and found out a few things.

The cause

This extension overrides the _activate() function making it empty.

WindowPreview.prototype._activate = () => {};

As a result, when it is called by the function that handles the Enter key press nothing happens. Below is the code from GNOME Shell that handles that:

  vfunc_key_press_event(keyEvent) {
      let symbol = keyEvent.keyval;
      let isEnter = symbol == Clutter.KEY_Return || symbol == Clutter.KEY_KP_Enter;
      if (isEnter) {
          this._activate(); // this is overriden as empty, so nothing happens
          return true;
      }

      return super.vfunc_key_press_event(keyEvent);
  }

Attempt 1

To fix this, I tried overriding the vfunc_key_press_event(keyEvent) itself to use the _oldActivate() function that this extension stores upon being enabled. This way, it would call the original _activate, not the one overridden as empty.

// save the original key press handling function
this._oldVfuncKeyPressEvent = WindowPreview.prototype.vfunc_key_press_event;

// override the key press handling function
WindowPreview.prototype.vfunc_key_press_event = function(keyEvent) {
    let symbol = keyEvent.keyval;
    let isEnter = symbol == Clutter.KEY_Return || symbol == Clutter.KEY_KP_Enter;
    if (isEnter) {
    init._oldActivate.apply(this);
    return true;
    }

    // if it wasn't the Enter key, use the original function
    return this._oldVfuncKeyPressEvent(keyEvent);
}

But that change made the shell crash whenever the activities view was shown with one window or more. Anyways, you can test it yourself on this branch.

Attempt 2

I then realized there would be no need to override the _activate() function if we could simply disconnect the 'clicked' signal handler. So here's what I did:

// override _addWindowClone to add my event handler
Workspace.Workspace.prototype._addWindowClone = function(metaWindow) {
    let clone = init._oldAddWindowClone.apply(this, [metaWindow]);

    // remove default 'clicked' signal handler
    let id = GObject.signal_handler_find(
        clone.get_actions()[0],
        {signalId: 'clicked'}
    )
    clone.get_actions()[0].disconnect(id);

    // add custom 'clicked' signal handler
    clone.get_actions()[0].connect('clicked', onClicked.bind(clone));

    return clone;
}

But that also lead to the same crashing on overview 🙃 I have a branch with these changes here.

Conclusion

I think I'm on to something but I don't know what I'm doing, so I just hope maybe this is something @p91paul will able to build from 😅

p91paul commented 1 year ago

attempt 2 looks promising. Can you retrieve which error happens exactly from journalctl logs?

pesader commented 1 year ago

Can you retrieve which error happens exactly from journalctl logs?

Of course! Which command should I run to do that?

p91paul commented 1 year ago

The ideal way is to run journalctl -f, leave it running, reproduce the issue, then kill journalctl with ctrl+c and copy the output text. If the crash makes you unable to use a terminal afterwards, but you manage to restore the system to a working state without rebooting, journalctl -b gives the logs from boot, but make sure to only get the last few minutes because it will be huge. If you are forced to reboot, journalctl also has options to get logs from previous boot.

pesader commented 1 year ago

Hey, sorry for disappearing (back to school!).

I got the logs and it seems that this is what we were looking for:

Mar 06 12:55:44 silverblue gnome-shell[45939]: JS ERROR: ReferenceError: GObject is not defined
                                               enable/Workspace.Workspace.prototype._addWindowClone@/var/home/pedro/.local/share/gnome-shell/extensions/middleclickclose@paolo.tranquilli.gmail.com/extension.js:81:13
                                               _init@resource:///org/gnome/shell/ui/workspace.js:1091:22
                                               Workspace@resource:///org/gnome/shell/ui/workspace.js:1029:1
                                               _updateWorkspaces@resource:///org/gnome/shell/ui/workspacesView.js:446:29
                                               _init@resource:///org/gnome/shell/ui/workspacesView.js:108:14
                                               WorkspacesViewBase@resource:///org/gnome/shell/ui/workspacesView.js:27:4
                                               WorkspacesView@resource:///org/gnome/shell/ui/workspacesView.js:86:1
                                               _updateWorkspacesViews@resource:///org/gnome/shell/ui/workspacesView.js:1034:24
                                               prepareToEnterOverview@resource:///org/gnome/shell/ui/workspacesView.js:993:14
                                               prepareToEnterOverview@resource:///org/gnome/shell/ui/overviewControls.js:720:33
                                               prepareToEnterOverview@resource:///org/gnome/shell/ui/overview.js:82:24
                                               _animateVisible@resource:///org/gnome/shell/ui/overview.js:568:24
                                               show@resource:///org/gnome/shell/ui/overview.js:549:14
                                               toggle@resource:///org/gnome/shell/ui/overview.js:659:18
                                               _init/<@resource:///org/gnome/shell/ui/overviewControls.js:448:31

You can checkout the rest of the log file here if you want to too!

p91paul commented 1 year ago

you reference GObject in your code for attempt 2, but you probably didn't import it. At the top of the file you see a series of imports, you need to check where that GObject is defined and import it (if you copied the code from somewhere else, they'll probably have such an import statement

pesader commented 1 year ago

You reference GObject in your code for attempt 2, but you probably didn't import it.

Yep, that was the problem. It's working now, so I'll make a PR.

If you copied the code from somewhere else, they'll probably have such an import statement

I wrote it myself! 🤓 (That's probably why there was such a silly mistake in it)

Thanks for the help!

p91paul commented 1 year ago

Nice job, thanks!

p91paul commented 1 year ago

Fixed by @pesader via #41, I submitted it to extensions.gnome.org and I expect it to be approved shortly. Thanks!