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

Gnome 45 #20

Closed
L30Bola opened this issue Oct 10, 2023 · 15 comments · Fixed by #21
Closed

Gnome 45 #20

L30Bola opened this issue Oct 10, 2023 · 15 comments · Fixed by #21
Assignees
Labels
help wanted Extra attention is needed

Comments

@L30Bola
Copy link

L30Bola commented Oct 10, 2023

Can you please add support for Gnome 45? Gnome Shell 45 has a breaking change (another one, right?) for Extensions: https://blogs.gnome.org/shell-dev/2023/09/02/extensions-in-gnome-45/

@lucasresck lucasresck self-assigned this Oct 10, 2023
@lucasresck lucasresck added the help wanted Extra attention is needed label Oct 10, 2023
@lucasresck
Copy link
Owner

Oh, my 😮 Is it reasonable to make a change that will literally break all extensions?

I'm not sure whether it will be possible to make the extension work with GNOME 45 and, when possible, how much work it will require. Let's keep this issue active.

Thank you for the update! Please, consider taking a look at extension.js to check how the extension works. Maybe it's easy to solve it.

@Goldpaw
Copy link

Goldpaw commented Oct 14, 2023

I tried looking into it myself, with my fairly limited (read: none) knowledge of the gnome desktop. I followed the gnome guide to migrate extensions but quickly ran into an error that told me WindowSwitcherPopup now was a read-only method and couldn't be replaced.

Is it even possible to do this in Gnome 45? o.O

@Goldpaw
Copy link

Goldpaw commented Oct 14, 2023

There are quite a lot of changes in Gnome 45, btw. They have made an official guide for migrating extensions, but it doesn't help with the issue of certain things now being read-only and no longer replacable.

https://gjs.guide/extensions/upgrading/gnome-shell-45.html

@ld-web
Copy link

ld-web commented Oct 22, 2023

Hi everyone, I tried to fork and work on this, this is my first time contributing on Gnome extensions, so I had to read quite a lot of documentation to (kind of) understand how to use things, and didn't come to a working solution (yet).

However, I wanted to share the progress, just in case. Of course, if it's completely out of topic or doesn't help resolving the problem, don't mind it.

I tried to work around what @Goldpaw said, indeed, WindowSwitcherPopup is now a const, so I thought, maybe by trying to alter the prototype chain of the registered class, it could work. Or by replacing the implementation of the _finish method. But I'm not sure about how to do it properly.

I first tried to keep the classes defined in the extension, extending the altTab classes.
Then I switched to object initializers with the __proto__ property set to the altTab classes.

Finally, I removed the __proto__ property and just tried to directly use AltTab.WindowSwitcherPopup.prototype._finish.bind(this)()...But now I have a weird result, where Alt+Tab is very laggy, and doesn't work...which makes me think the problem may be the pointer module I decided to isolate in src ?

Everything is on my fork, on the gnome-45 branch.

Again, sorry if everything is completely wrong 😅

Cheers !

PS : btw, don't mind the metadata.json file, it was just for testing with another ID.

@lbltavares
Copy link

lbltavares commented Oct 22, 2023

Hey guys, I'm also new here 👋

I think I've got it working for WindowSwitcherPopup (still working on AppSwitcherPopup)

I came across another Alt+Tab related extension that overrides the _finish method using InjectionManager: Leleat/AltTab-Mod

So, I used his code as a base and simplified it until I reached this: my fork

Here's the main part:

        this._injectionManager = new InjectionManager();

        ...

        this._injectionManager.overrideMethod(
            AltTab.WindowSwitcherPopup.prototype,
            '_finish',
            (originalMethod) => {
                return function (timestamp) {
                    const [x, y] = global.get_pointer();
                    vdevice.notify_absolute_motion(global.get_current_time(), x, y);
                    Main.activateWindow(this._items[this._selectedIndex].window);
                    originalMethod.call(  
                        this,
                        timestamp
                    );
                };
            }
        );

For some reason it's not working for AppSwitcherPopup, but I'm currently looking into it

@ld-web
Copy link

ld-web commented Oct 22, 2023

Oh if I had found this InjectionManager before...thanks @lbltavares, gonna try this too to override the _finish method

@lbltavares
Copy link

lbltavares commented Oct 22, 2023

Got it working on AppSwitcherPopup!
I'll create the PR in a second

Oh if I had found this InjectionManager before...thanks @lbltavares, gonna try this too to override the _finish method

Don't even get me started! I spent the entire day on this poorly documented thing 😅 If it weren't for other similar extensions, I would have given up already

@ld-web
Copy link

ld-web commented Oct 22, 2023

Dang ! Got it working too with the InjectionManager I think ! I'll commit anyway, if you want we can review and exchange about it to see if versions match ? We can go with your PR of course. Thanks

@lbltavares lbltavares mentioned this issue Oct 22, 2023
@lbltavares
Copy link

Sure! Feel free to suggest anything, here it is: #21

It's still very hacky, so I'll appreciate your review!

@ld-web
Copy link

ld-web commented Oct 22, 2023

Okay I didn't want to put too much review since we went different ways, so I opened mine : #22

Let's discuss and see what we can do (or not hahaha) ! Cheers

@lbltavares
Copy link

lbltavares commented Oct 24, 2023

@ld-web Thanks for the suggestion! It indeed feels lighter having movePointer and vdevice outside the class. I merged your branch into mine :)
I also sometimes use alt+esc to switch windows directly and I noticed the bug was still there. So I added a fix for this in my PR. Additionally, I removed some redundant code in the injector callbacks (since we're calling originalMethod, there's no need to replicate it's logic inside the callback).

ps: I just realized we are 3 Lucas's working on this project 😄

@ld-web
Copy link

ld-web commented Oct 24, 2023

Nice ! I didn't notice for Alt+Esc, nice catch 💯
Let's wait for @lucasresck feedback then

PS : Hahaha uniting Lucas' forces ! 💪🏻

@lucasresck
Copy link
Owner

Hey, guys! Thank you very much for this amazing discussion! I'm loving this haha

I'll review the pull request soon, thank you so much!

@Goldpaw
Copy link

Goldpaw commented Oct 26, 2023

Hey, guys! Thank you very much for this amazing discussion! I'm loving this haha

The glory of open source. Windows and mac users definitely haven't got this kind of community! ;)

@lucasresck lucasresck linked a pull request Oct 26, 2023 that will close this issue
@lucasresck lucasresck mentioned this issue Oct 26, 2023
@lucasresck
Copy link
Owner

I am happy to announce the extension now supports GNOME 45! Changes are still in the develop branch but it will be included in main and released to GNOME Extensions soon!

I'll close this issue, but feel free to share more information if necessary.

Thank you for all work on this, specially @ld-web and @lbltavares for coming up with the solutions. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants