JasonLG1979 / gnome-shell-extension-mpris-indicator-button

A full featured MPRIS indicator button extension for GNOME Shell 3.38+
https://extensions.gnome.org/extension/1379/mpris-indicator-button/
GNU General Public License v3.0
159 stars 21 forks source link

Chromium/Chrome 75 exposes a broken MPRIS interface. #10

Open JasonLG1979 opened 5 years ago

JasonLG1979 commented 5 years ago

As the title says Chromium/Chrome 75 exposes a broken interface. The only thing that currently works is metadata (limited to artist and title) and playback status. All other properties and methods seem to be nonfunctional. (rendering the controls in the extension also nonfunctional) On top of that once the interface shows up it never goes away even after navigating away from a supported sites like YouTube or PlayMusic.

Screenshot from 2019-06-05 17-54-32

Until it actually works I think the best thing to do is blacklist Chromium/Chrome for now.

JasonLG1979 commented 5 years ago

On further investigation it seems that disabling the enable-media-session-service flag stops Chromium/Chrome from putting up the broken interface. So I don't think I will be blacklisting Chromium/Chrome after all.

I think this is just a case of prematurely enabling a feature before it's fully baked. So my advice is to disable the enable-media-session-service flag until it actually works.

coxackie commented 5 years ago

That may be so for YouTube or PlayMusic, but the with the spotify PWA (https://open.spotify.com/), one gets good MPRIS support (play/pause/back/forward) with your extension. The gnome shell notification area, for some reason, does not show the controls...

JasonLG1979 commented 5 years ago

That may be so for YouTube or PlayMusic, but the with the spotify PWA (https://open.spotify.com/), one gets good MPRIS support (play/pause/back/forward) with your extension. The gnome shell notification area, for some reason, does not show the controls...

The logs show "JS ERROR: Exception in callback for signal: changed: TypeError: this._player.trackArtists.join is not a function"

My guess would be that they are sending the artist metadata as a string and not an array of strings as it should be according to spec...

My extension is quite a bit more forgiving of types when it comes to parsing the metadata: https://github.com/JasonLG1979/gnome-shell-extension-mpris-indicator-button/blob/master/mprisindicatorbutton%40JasonLG1979.github.io/dbus.js#L960-L963

I tend to assume that all players are broken in at least one way or another, because with very few exceptions they are...

coxackie commented 5 years ago

Hmmm, I see. I guess there is no point trying to contact Spotify to fix this, right? It appears like they give zero feedback to Linux-related inquiries.

JasonLG1979 commented 5 years ago

It's not a Spotify bug. It's a Chrome bug. Spotify is not creating the MPRIS interface when you use their web player on Chrome, Chrome is.

JasonLG1979 commented 5 years ago

You can try to file a bug report with Chromium/Chrome. But I doubt anyone will listen. As I said it's a case of prematurely enabling a feature. They have to know it doesn't work I would think?

coxackie commented 5 years ago

Ah - ok. I could, but maybe they would be more interested in hearing from you, as a dev, regarding this? Or I could file in chromium and mention this thread?

JasonLG1979 commented 5 years ago

Ah - ok. I could, but maybe they would be more interested in hearing from you, as a dev, regarding this? Or I could file in chromium and mention this thread?

There intention (along with GNOME and other Linux desktop devs) is to use MPRIS for media key bindings. On the surface that may seem reasonable but it has one major flaw. There is no way in MPRIS to tell who to give the keys to in the event of more than one player. MPRIS lacks any sort of "focus" or "last user interaction time" properties. You'd need a platform/desktop agnostic way to tell who is the focused player and/or who was the last player the user interacted with to tell who to give the keys too. Adding the porps to the MPRIS spec is a possibility, but I don't trust players to implement them properly. I have the feeling that devs who choose to use MPRIS as media keys will be inundated with bug reports for players that they can't do anything about and the experience as a whole will degrade.

terencode commented 4 years ago

This focus thing is a big flaw I experience all the time. Has it been reported somewhere?

JasonLG1979 commented 4 years ago

This focus thing is a big flaw I experience all the time. Has it been reported somewhere?

I mentioned it in a bug report for key binding back in 3.20 it was I think. I not so politely told them it was a dumb ass idea. (which got me locked out of the thread,lol!!!) They don't care. It's just there way of moving more functionality and responsibility out of GNOME Shell so that when someone else complains they can just say "That's not our bug".

JasonLG1979 commented 4 years ago

They'll just figure out some kind of sideband GLib app only way to work around it. What really needs to happen if it hasn't already is a desktop agnostic mediakeys binding DBus interface.

terencode commented 4 years ago

Ok I'll try to look around and ask if something like that is in development, thanks.

JasonLG1979 commented 4 years ago

GNOME Shell sucks at keeping track of focused apps. Separate instances of an app are treated as the same app by default. That's why I wrote the AppWrapper class. Otherwise you run into edge cases like if you have 2 instances of VLC and you try to raise one of them. Who knows which one it will actually raise if you call Shell.App.activate(). I brought that up in a bug report not long ago also and they shot it down. they are much more interested in what's easy and fits in there idea of how things should be then what actually works correctly and how things actually are.

ferdnyc commented 4 years ago

@JasonLG1979 There's actually now an open bug report about the focus issue in the MPRIS repo, and an related MR proposing to address it in the protocol by adding a last-focused timestamp to MPRIS messages.

However, that MR has already gotten some pushback. ...From you. Oh. Never mind! :wink:

JasonLG1979 commented 4 years ago

However, that MR has already gotten some pushback. ...From you. Oh. Never mind!

It's not that I'm totally opposed to a "focused" prop I just think that it's a mistake and asking for trouble to leave it up to players to provide a timestamp. As I stated I think a Boolean is a better idea. The client should decide how they want to sort the players.

As old as the MPRIS spec is most player implementations are still incomplete or broken sometimes in the most brain dead ways. Adding one more fairly critical thing for players to screw up is IMHO not a good idea. Not to mention that the time in the MR is in Epoch time (eww)...

fcastilloec commented 4 years ago

I'm trying to run electron with --disable-media-session-service with app.commandLine.appendSwitch('disable-media-session-service') but it's not doing anything.

Does anybody have some documentation on the enable-media-session-service flag? I've also tried with chrome/chromium in the command line and it doesn't change the flag. Any ideas?

JasonLG1979 commented 4 years ago

My bad it's --disable-features=MediaSessionService

An easy way to play with flags is to go to chrome://flags/ change the flag you want, restart Chrome and then go to chrome://version/ and see what the command line arg is.

meden commented 4 years ago

My bad it's --disable-features=MediaSessionService

In my setup (Vivaldi 3.0 / Chromium 81.0.4044.142) it is actually --disable-features=HardwareMediaKeyHandling (the flag is #hardware-media-key-handling).

I needed this to prevent the browser to grab the media keys (even if there is no player active) and make them not functional with Spotify. Hope can help.

fcastilloec commented 2 years ago

It seems that the #hardware-media-key-handling flag has been removed on newer Chrome versions. It can still be enabled under expired flags, but it will permanently be removed in the very near future. I completely understand that this is technically not this extension issue, but having a way to ignore all chromium players would be great! Electron apps won't be affected as long as they register under their own name with DBus

JasonLG1979 commented 2 years ago

It seems that the #hardware-media-key-handling flag has been removed on newer Chrome versions. It can still be enabled under expired flags, but it will permanently be removed in the very near future. I completely understand that this is technically not this extension issue, but having a way to ignore all chromium players would be great! Electron apps won't be affected as long as they register under their own name with DBus

Settings in the extension are a no go so there's no real way to make everyone happy. Some may want Chrome to show up in the extension so a blanket ban is also not ideal. The Chrome devs really should just fix their shit.

fcastilloec commented 2 years ago

Settings in the extension are a no go so there's no real way to make everyone happy. Some may want Chrome to show up in the extension so a blanket ban is also not ideal. The Chrome devs really should just fix their shit.

I totally agree with you, Chrome devs should just fix this. Nonetheless, it's unrealistic that it will ever happen. I definitely understand that some people might want Chrome to show up and others don't. So, the only practical solution is to let people decide. Instead of settings, a config file can be read to ignore programs (not just chromium); similar in concept to a gitignore file. Only advanced users can make use of this and there's no need to create settings. Thoughts?

JasonLG1979 commented 2 years ago

I totally agree with you, Chrome devs should just fix this. Nonetheless, it's unrealistic that it will ever happen. I definitely understand that some people might want Chrome to show up and others don't. So, the only practical solution is to let people decide. Instead of settings, a config file can be read to ignore programs (not just chromium); similar in concept to a gitignore file. Only advanced users can make use of this and there's no need to create settings. Thoughts?

To my knowledge config files are not really a thing for extensions. The conical way is to have a setting UI and associated gsettings. I'm not a fan at all of settings or a config file for what should be (and currently is) a very simple (in operation) extension. That's just a doorway for feature creep or at least endless feature requests. I consider this extension to be feature complete.

ferdnyc commented 2 years ago

There's another flag that still is present in Chrome 101, #system-keyboard-lock:

Experimental system keyboard lock

Enables websites to use the keyboard.lock() API to intercept system keyboard shortcuts and have the events routed directly to the website when in fullscreen mode. – Mac, Windows, Linux, Chrome OS, Fuchsia

So, it may well be that they've gotten the message that MPRIS is not the way to go with this, and are planning to let media players directly grab media controls (along with any other keystrokes they want to treat specially)... but only when fullscreened.

Which seems reasonable, and handily ducks the focus issue entirely.