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

MPRIS & global media hotkeys broken in Gnome in 4.7.0-pre builds #3583

Closed
3 tasks done
fastcat opened this issue Jun 11, 2019 · 33 comments · Fixed by #3587
Closed
3 tasks done

MPRIS & global media hotkeys broken in Gnome in 4.7.0-pre builds #3583

fastcat opened this issue Jun 11, 2019 · 33 comments · Fixed by #3587

Comments

@fastcat
Copy link

fastcat commented Jun 11, 2019

Checklist:

OS: Linux -- Debian Buster

GPMDP Version: CI build of 8f86ba7 (also happens with some earlier 4.7.0-pre builds)

Issue Descriptions:
The media player interface that used to integrate with Gnome no longer works in the current CI builds. It works fine in the last 4.6.x release build. Gnome doesn't think any media player app is running at all, and as such shows no panel indicator, nor do global media hotkeys work.

The merge of #3513 is immediately suspect in the commit history, but I have not bisected to prove that theory. Given that #3510 claims that a similar issue on MacOS was fixed via the Electron update, I suppose that may be suspect as being related on Linux?

Steps to Reproduce:

  1. Install GPMDP on a Gnome system
  2. Start playing music
  3. Look in the system menu for a media player indicator -- nothing
  4. Try using global media hotkeys bound via the gnome keyboard settings -- they show an indicator that nothing is playing

See also:

@jostrander
Copy link
Collaborator

Can you confirm if this is only related to YouTube music for you? I just fetched from master and tested and it appears that GPM works fine. IPC is showing the media keys firing to the outer process, leading me to believe that the YTM integration is entirely broken.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

I don't use YouTube Music at all, I've only tried with Google Play Music, and it happens 100% there for me.

@jostrander
Copy link
Collaborator

What version of gnome-shell are you running? dpkg -l | grep gnome-shell

Everything related to dbus has been changed since 4.6.x, so there's a lot to triage here.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

Debian Buster is using gnome-shell 3.30.2

More specific version info: https://packages.debian.org/buster/gnome-shell

@jostrander
Copy link
Collaborator

If you press ctrl+shift+i in the GPMDP window, do you have any errors in the console?

Also type openGPMDevTools() and hit enter, and check that window as well.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

That's not working to bring up the developer tools here :(

Is that perhaps disabled in the CI .deb packages?

@jostrander
Copy link
Collaborator

Ah, yeah it is disabled in the packaged versions. Try ctrl+shift+g and type 'DEV_MODE' then let it restart. Repeat the steps above.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

That did the trick for getting access to the developer consoles.

At startup, the only thing in the main (ctrl-shift-i) console is this warning:

/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/react/lib/lowPriorityWarning.js:38 Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in  React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs
printWarning @ /usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/react/lib/lowPriorityWarning.js:38

However in the openGPMDevTools console, there is this set of warnings & errors:

chrome-extension://pkedcjkdefgpdelpbcmbmeomcjbeemfm/cast_sender.js:1 Failed to load resource: net::ERR_NOT_IMPLEMENTED
chrome-extension://enhhojjnijigcajfphajepfemndkmdlo/cast_sender.js:1 Failed to load resource: net::ERR_NOT_IMPLEMENTED
[Deprecation] Styling master document from stylesheets defined in HTML Imports is deprecated, and is planned to be removed in M67, around May 2018. Please refer to https://goo.gl/EGXzpw for possible migration paths.
listen.js:3937 paper-toolbar is deprecated. Please use app-layout instead!
ready @ listen.js:3937
listen.js:3937 paper-toolbar is deprecated. Please use app-layout instead!
ready @ listen.js:3937
listen.js:3937 paper-toolbar is deprecated. Please use app-layout instead!
ready @ listen.js:3937
/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/node-mdns-easy/dist/browser.js:96 Uncaught TypeError: Cannot read property 'substring' of undefined
    at _class._normalizeService (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/node-mdns-easy/dist/browser.js:96)
    at _class.serviceUp (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/node-mdns-easy/dist/browser.js:74)
    at module.exports.emit (events.js:182)
    at module.exports.internal.onMessage (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/mdns-js/lib/browser.js:108)
    at module.exports.emit (events.js:182)
    at Socket.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/mdns-js/lib/networking.js:144)
    at Socket.emit (events.js:182)
    at UDP.onMessage [as onmessage] (dgram.js:673)

The last one repeats regularly.

When I start playback, no new messages appear in either console.

I tried installing playerctl as a "third party" interface to see what's going on with MPRIS, and it thinks the only MPRIS player running is Chrome (:man_facepalming:).

I also went mucking about with d-feet to browse the session bus, and I can't find any signs of GPMDP at all in there, whether related to media or not.

App startup on stdout prints this, dunno if its related:

Starting Sentry
error: Failed to load bonjour with error: {}
Bonjour is required to use Chromecast Support or to enable ZeroConf for the PlaybackAPI
On linux you need to install "avahi"
11 Jun 18:31:53 - uncaughtException: a860ad77c4a744b1af10b17fe0877833

Also I tried running the app under strace -f, and it was pretty easy to find all the dbus interactions, and nothing mpris-y shows up in the dbus messages back & forth.

Looking in the "Sources" tab in the dev tools ... it looks almost as if something is going wrong and it's not even loading the mprisService.js file out of the asar -- it doesn't appear in the list of loaded source files. On the other hand, I don't see lots of other stuff I'm pretty sure is actually loaded and working (like anything from src/main/), so I'm probably doing something wrong there.

@jostrander
Copy link
Collaborator

It appears that the chromecast functionality may be breaking the rest of the integrations for you. Try installing avahi to see if that resolves your problem. If it does we can probably implement a fix to ignore that error if avahi isn't installed.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

I ... do have avahi installed?

$ apt list --installed \*avahi\*
Listing... Done
avahi-daemon/testing,unstable,now 0.7-4+b1 amd64 [installed]
avahi-discover/testing,testing,unstable,unstable,now 0.7-4 all [installed]
avahi-utils/testing,unstable,now 0.7-4+b1 amd64 [installed]
libavahi-client3/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
libavahi-common-data/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
libavahi-common3/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
libavahi-core7/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
libavahi-glib1/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
libavahi-gobject0/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
libavahi-ui-gtk3-0/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]
python-avahi/testing,unstable,now 0.7-4+b1 amd64 [installed,automatic]

Do you know which bit of avahi I may be missing that it needs?

@jostrander
Copy link
Collaborator

jostrander commented Jun 11, 2019

libavahi-compat-libdnssd1, its technically an extension of avahi that provides an mDNS api.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

Aah, yep -- that should probably be added to the package depends then?

But ... while that fixes the error loading bonjour at startup, I don't have an MPRIS instance in DBus yet.

I do still have the ERR_NOT_IMPLEMENTED bits in the dev console from the casting bits, if that's relevant.

I also still have the 11 Jun 19:01:50 - uncaughtException: 763e6aa2094c4253b84a438f73da0236 message on the terminal when starting the app.

@fastcat
Copy link
Author

fastcat commented Jun 11, 2019

OK, I did some UTSL & figured out how to launch dev mode from the CLI, and now I have what is probably a much more useful error trace for you :)

error: Uncaught Exception. Error: Cannot find module 'xmlbuilder'
    at Module._resolveFilename (internal/modules/cjs/loader.js:602:15)
    at Function.Module._resolveFilename (/usr/share/google-play-music-desktop-player/resources/electron.asar/common/reset-search-paths.js:35:12)
    at Function.Module._load (internal/modules/cjs/loader.js:528:25)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/xml2js/lib/builder.js:7:13)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/xml2js/lib/builder.js:127:4)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/xml2js/lib/builder.js:129:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/xml2js/lib/xml2js.js:10:13)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/xml2js/lib/xml2js.js:37:4)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/xml2js/lib/xml2js.js:39:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/dbus-next/lib/service/name.js:6:16)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/dbus-next/lib/service/name.js:171:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/dbus-next/lib/bus.js:5:14)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/dbus-next/lib/bus.js:351:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/dbus-next/index.js:2:20)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/node_modules/dbus-next/index.js:144:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/main/features/linux/mediaKeysDBus.js:11:17)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/main/features/linux/mediaKeysDBus.js:49:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/main/features/linux/index.js:11:1)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/main/features/linux/index.js:14:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/main/features/index.js:13:1)
    at Object.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/main/features/index.js:14:3)
    at Module._compile (internal/modules/cjs/loader.js:711:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:722:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:559:12)
    at Function.Module._load (internal/modules/cjs/loader.js:551:3)
    at Module.require (internal/modules/cjs/loader.js:658:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at App.<anonymous> (/usr/share/google-play-music-desktop-player/resources/app.asar/build/index.js:187:5)
    at App.emit (events.js:187:15)

@jostrander
Copy link
Collaborator

Alright, yeah, that's what I needed. It appears something was missed in the CI build for linux, as that should have been installed as a dependency of dbus-next but wasn't.

@jostrander
Copy link
Collaborator

jostrander commented Jun 12, 2019

I updated the dbus dependencies on a branch, you can try using this deb:

https://3300-40008106-gh.circle-artifacts.com/0/home/circleci/project/dist/installers/debian/google-play-music-desktop-player_4.6.1_amd64.deb

Edit: this is also not working. Works when building locally, fails on CI.

@MarshallOfSound
Copy link
Owner

@jostrander Are you using npm or yarn. You might need to update the yarn lockfile

@jostrander
Copy link
Collaborator

yarn exclusively. Just checked on CI and xmlbuilder is definitely there at build time.

@jostrander
Copy link
Collaborator

It's definitely missing from the built deb:

asar l app.asar | grep xmlbuild shows xml2js but not xmlbuilder

I'm not sure where it would be stripping that out.

@jostrander
Copy link
Collaborator

I think I was able to isolate this down to the packager update in this commit: 23d9799. From 8 -> 13 electron-packager stopped relying on the package manager to prune and started using galactus.

It looks like you maintain galactus @MarshallOfSound. Any ideas on why it would be nuking xmlbuilder?

@MarshallOfSound
Copy link
Owner

@jostrander Not without a repro on my machine, it's got quite extensive use and tests so I'll be pretty annoyed if we've hit an edge case 😆

I'll ssh into a CI build at some point and take a look DEBUG=* should at least tell us everything that is happening

@jostrander
Copy link
Collaborator

At a cursory glance, playing with flora-colossus directly, this is what I'm getting for xml2js:

{ depType: 2,
  path: '/home/jostrander/data/Code/other/Google-Play-Music-Desktop-Player-UNOFFICIAL-/node_modules/xml2js',
  name: 'xml2js' }
{ depType: 1,
  path: '/home/jostrander/data/Code/other/Google-Play-Music-Desktop-Player-UNOFFICIAL-/node_modules/xml2js/node_modules/xmlbuilder',
  name: 'xmlbuilder' }

where depType 2 is optional, and 1 is dev.

@fastcat
Copy link
Author

fastcat commented Jun 12, 2019

Is that suggesting that the root of the issue is that dbus-next is listed as an optional dependency of GPMDP? Seems a little odd that wouldn't blow up until two layers in, since dbus-next lists xml2js as required, and xml2js lists xmlbuilder as required.

@jostrander
Copy link
Collaborator

Not at all, the underlying problem was definitely that the packager is deleting things under node_modules a bit too aggressively. The underlying problem has been fixed but its 3 dependencies deep so it will take some time to get everything bumped and working.

@jostrander
Copy link
Collaborator

Actually, I take that back, a rebuild will more than likely pull in the updated dep and cause it to work, I'll test it out.

@rameezv
Copy link

rameezv commented Jun 12, 2019

I'm having the exact same issue with Cinnamon 3.6.7-8, let me know if there's any information I can provide for more data points or whatever

EDIT: Confirmed building locally via yarn run make:deb:64 fixes the issue

@jostrander
Copy link
Collaborator

https://3322-40008106-gh.circle-artifacts.com/0/home/circleci/project/dist/installers/debian/google-play-music-desktop-player_4.6.1_amd64.deb

Here is a better "4.7.0" release for debian, this should fix the exception at start up. I get MPRIS and media keys in GPM with this. However I have another fix for YTM that I need to publish in order to get that working.

@fastcat
Copy link
Author

fastcat commented Jun 12, 2019

Here is a better "4.7.0" release for debian

I'm not getting the require errors any more when starting with --dev on the CLI, but I'm also not getting the MPRIS stuff working, and I can't see the integrations .js files loaded in either developer tools window yet. Nothing new/interesting in the developer consoles either.

However d-feet does see the org.mpris.MediaPlayer2.google_play_music_desktop_player going and playerctl can see & control the app, so my problem at this point may be with Gnome more than GPMDP.

Thanks!

@fastcat
Copy link
Author

fastcat commented Jun 12, 2019

OK, debugged some more and I think I've figured out what's going wrong with Gnome media keys:

  1. https://github.com/GNOME/gnome-settings-daemon/tree/master/plugins/media-keys
    Note especially:

Every time your application is focused, you should call GrabMediaPlayerKeys() again, so that gnome-settings-daemon knows which one was last used. This allows switching between a movie player and a music player, for example, and have the buttons control the last used application.

  1. https://github.com/MarshallOfSound/Google-Play-Music-Desktop-Player-UNOFFICIAL-/blob/master/src/main/features/linux/mediaKeysDBus.js
    Only does the GrabMediaPlayerKeys once at startup
  2. Chrome windows are all over my desktop, and so it is stealing the media key focus a lot, and GPMDP never takes it back

If I launch GPMDP and am careful not to focus a Chrome window again, the global Gnome media keys do work. But eventually after focusing Chrome windows (it doesn't happen immediately), it seems to take over the media key focus, and GPMDP never takes it back.


Edit: to be clear, the new build fixes this issue ~95%, it's just some fiddly corner cases that seem to be left, so re-Thanks :D

@jostrander
Copy link
Collaborator

Yep, that was actually never implemented to spec unfortunately. It can definitely be fixed. For now you might try removing the GPM chrome extension because that is probably what is stealing your keys, (if its installed)

@fastcat
Copy link
Author

fastcat commented Jun 12, 2019

For now you might try removing the GPM chrome extension because that is probably what is stealing your keys

Good call. That did at least make it not register an MPRIS instance (or anyways it did after restarting chrome -- disabling / uninstalling without restart was not sufficient). I'm gonna assume that if its MPRIS instance is gone, it will not be grabbing Gnome media keys either. Either way I wasn't using that extension on the desktop anyways (only on a chromebook, but got installed because sync), and things are working now :)

@fastcat
Copy link
Author

fastcat commented Jun 13, 2019

FYI I'm still seeing cases where chrome steals media keys from GPMDP and won't "give them back", but I've not traced down what triggers this, and so I can't say it's actually a GPMDP bug at this stage. I'll open a new ticket if I find anything useful.

@jostrander
Copy link
Collaborator

I can't reproduce with the latest build, what extensions do you have in gnome and chrome?

@fastcat
Copy link
Author

fastcat commented Jun 13, 2019

I have ... quite a few ... but none that obviously should be doing media key stuff. Loading a youtube page does not trigger chrome to steal.

My current best guess is that it's either Google Calendar event notifications, or the Hangouts App, which is doing it, but I've not proven this yet.

Whatever it is, it does correlate with chrome's mpris instance showing up in d-feet. no it doesn't ... loading youtube makes this appear, but doesn't cause chrome to eat the media keys ... yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants