Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
51 stars 10 forks source link

Chromium workaround is weird #8

Closed Moon-0xff closed 1 year ago

Moon-0xff commented 1 year ago

The last discussion of #5 was about the chromium workaround:

    if(!matchedEntries.length === 0)
        return matchedEntries[0][0]

    if (suspectAppName == "chromium" && matchedEntries.length === 0)
        matchedEntries = Gio.DesktopAppInfo.search("google-chrome")

    if(matchedEntries.length === 0)
        return null

It works. But why? It doesn't make logical sense. Also, it only works with google-chrome and other chromium-based browsers might need a workaround too.

Batwam commented 1 year ago

for info, I have tested Opera/Brave/Vivaldi in a VM. As expected, they all report as chromium and won't load the icons but the following (snipets of) code work. Brave had to be capitalised to work for some reason. The problem is obviously that since it's not searching through running apps, it will pick up the icon from the first app which is installed, regardless of whether it's the correct one or not... which is why I have loaded them in order of popularity: matchedEntries = Gio.DesktopAppInfo.search("Opera") matchedEntries = Gio.DesktopAppInfo.search("Brave") matchedEntries = Gio.DesktopAppInfo.search("Vivaldi")

if we could somehow find our the app hidden behind the mpris source or exclude non-running apps that would be a lot easier...

These were installed using debs (Opera/Vivaldi) when no flatpak was available. Brave was flatpak. I tried the snaps but for some reaons there is an issue with appArmor and they can't seem to be able to get access to mpris so the extension doesn't work for snap versions of these browsers (works for firefox though...).

Batwam commented 1 year ago

I notied when I was playing around with D-Feet that in the list of mpris sources, Chrome provides a cmd info which could be perhaps used to better identify the various chromium derivatives than the current hack if we can extract it? image

Batwam commented 1 year ago

ok, I think that i found an indirect solution as I was going through the mpris properties and spec.

While DesktopEntry is optional and seems to be unreliable, Identity is not. I have put this interface into the extension and reported both DesktopEntry (first) and Identity below, the chromium browsers can now be reliably segragated and we no longer need the hacky workaround!!!! ---------------------------------------------------DesktopEntry / Identity ----------------------------------------------------- Screenshot from 2023-02-11 16-46-08

I'm not going to do anything as we have 3 merge requests on the go and the last thing we want to do is waste time with merge conflicts but as soon as things are merged, I think that we should consider implementing setting the icon using Identity and removing the current guesswork based on address.

PS: Identity could also replace shortname in the menu and remove a few extra lines of code

Moon-0xff commented 1 year ago

27(merged) uses Identity to get the icon of the player (among other things) and removes the necessity for a workaround, therefore this issue is resolved.