Gnome45 version

[Batwam]

See initial commits for gnome45 compatibility update in relation to #77.

I have split this into multiple independant commits with the priority being to make as little change to the underlying code structure in the hope that the gnome45 variant is limited to some line swaps which are hopefully independent from any requires code change.

Note that for this purpose, I have included 2 initial commits which should ideally be cherry-picked and applied to the current main. They are compatible with older versions but I'm relying on them to make the gnome45 version work.

Everything after these 2 commits is gnome 45 specific and shouldn't be merged with main. (and yes, putting settings back into the pref.js function sets it back to what it was a few days ago...). That's primarily because I couldn't figure out how to define settings globally so if you can find a way, it may not be required.

Regarding the binary file, I didn't update it (on purpose) and can confirm that this isn't causing any issue so I'm still hopeful that this gnome45 for could be kept in sync with main with just these few commits added for compatibility.

At this point, the extension loads, appears in the panel with an icon, menu, and preference settings window. What I can't get to work is the import of the buildLabel var/function which means that the label text isn't loaded. For the time being, I have overwritten it with a static text for testing purposes. It's arguably a bit of a dealbreaker but at least the rest appears to work (symbolic option, label position, menu,...). Would you mind looking to see how the buildLabel could be imported? I just can't quite figure out what the syntax should be...

One other thing I noticed is that the album cover seems to require reloading the extension to appear. Essentially, if I start the player (only tried with rhythmbox), it shows the app icon but not the album. if I disable it and re-enbable it, the album will appear. It could be a gnome-nightly issue or a rhythmbox signal issue, not sure...

[Moon-0xff]

I don't have an environment with GNOME 45 installed right now, but I will test it when I can.

Would you mind looking to see how the buildLabel could be imported? I just can't quite figure out what the syntax should be...

Perhaps the var buildLabel = part of the function declaration shouldn't be kept https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#standard_import

[Batwam]

I installed gnome-boxes as flatpak and installed gnome-nightly from it. The main challenge is that it's running wayland so I'm having to log out constantly to reload gnome-shell... but it's not too bad.

Based on further investigation, the import seems to work fine as the code does go into buildLabel and getSettings. It seems to crash when trying to access the setting values...

[Batwam]

Turns out there was an undefined REFRESH_RATE variable left which was crashing the code within label.js getSettings(). It worked as soon as I removed it (the variable wasn't use anyway).

I have split the change, (together with deleting 2 duplicate variable definitions) in 782f4a8. This should probably go into main too for "future compatibility" reason.

It's pulling the label just fine now. I noticed that gjs has become more sentitive to this sort of coding errors with undefined variables. Another example was this error here. Almost as if use strict is enabled by default...

Still have this issue with the album cover only loading when I turn the extension on/off... could be a rhythmbox issue if it's not responding fast enough... I'll try to install Ubuntu 23.10 to check.

everything else looks fine as far as I can tell:

[Batwam]

ok, the covers seem to work fine on Ubuntu 23.10 using G4Music. Not sure why it wasn't working with gnome-nightly/rhythmbox.

One thing I did notice however is that the menu items are no longer aligned. It used to look like this

Now it looks like this as Ornament is set to HIDDEN rather than NONE by default.

This appears to have been done as a recent change in gnome which changed the default from NONE to HIDDEN to remove the left padding. We can restore the original look by setting the style back to NONE which should still work in the gnome42 version:

[Batwam]

Would you consider merging the non-gnome45 specific commits into main to minimise the changes? These commits should be minor and could be tested now and included in the upcoming release.

We have the first two commits of this PR as well as 782f4a8 and this last one which sets the Ornament to what it already is in gnome <45.

[Moon-0xff]

Current main is in great shape to release a new version and I don't want to delay it.

[Batwam]

Ok, no worries. We don't necessarily have to merge now but I was also asking whether you are open to make changes to main which aren't strictly required but help minimise code changes between the gnome42 and gnome45+ versions.

We still have some time to get the gnome45 version out but Mantic Minotaur is releasing at the end of next month so it would be good to have the gnome version out mid-Oct.

I don't have much to change as I think that what we have in this PR works. There may be a way to avoid putting settings back into the pref.js and define it at Global level but I can't figure out how with the new method.

The import method I'm pretty confident is correct. I did notice as I split the imports that we currently seem to be importing libraries which aren't being used but that's probably not a deal breaker.

For getSettings(), the lookupUrl method works. There may be other ways to do it but I doubt that would significantly simplify the code at it's just a one liner currently.

The album cover issue is either a gnome-nightly or rhythmbox specific issue. It would be worth testing this more but the issue doesn't seem to be directly related to the changes in this PR and might have more to do with the issue we we're already experienced with Spotify. Anyway, I'll take the action to test with rhythmbox on Ubuntu 23.10 to see how that looks.

[Moon-0xff]

I was also asking whether you are open to make changes to main which aren't strictly required but help minimise code changes between the gnome42 and gnome45+ versions.

Some of them are to be included because they are fixes of coding errors (that didn't matter because it seems that gjs in 43,44 is more lenient).

The others might not be necessary or beneficial depending on the strategy we will use to handle both versions.

We still have some time to get the gnome45 version out but Mantic Minotaur is releasing at the end of next month so it would be good to have the gnome version out mid-Oct.

Sounds good.

There may be a way to avoid putting settings back into the pref.js and define it at Global level but I can't figure out how with the new method.

Rules don't allow it.
And I just noticed that current main is also holding settings as global so the new version I submitted to ego will get rejected (oops!).

I don't have much to change as I think that what we have in this PR works. There may be a way to avoid putting settings back into the pref.js and define it at Global level but I can't figure out how with the new method.

The import method I'm pretty confident is correct. I did notice as I split the imports that we currently seem to be importing libraries which aren't being used but that's probably not a deal breaker.

For getSettings(), the lookupUrl method works. There may be other ways to do it but I doubt that would significantly simplify the code at it's just a one liner currently.

👍

The album cover issue is either a gnome-nightly or rhythmbox specific issue. It would be worth testing this more but the issue doesn't seem to be directly related to the changes in this PR and might have more to do with the issue we we're already experienced with Spotify. Anyway, I'll take the action to test with rhythmbox on Ubuntu 23.10 to see how that looks.

If it works on Ubuntu/G4Music I don't how it could be related to our extension, but gjs being less lenient and crashing in some places that didn't before could mean that there's broken functionality deep into the code.

[Moon-0xff]

Certainly we will need to test the initial 45 version a lot more before releasing it.

[Batwam]

Ok, I guess you can cherry-pick the first 2 commits of this PR then?

For info, I'm in discussions with the Just Perfection guy who is suggesting some changes so I'll probably end up starting this gnome 45 port from scratch to keep things clean but at least we know that it can be made to work.

[Batwam]

PR closed pending main code update to support gnome45 transition