Fix mpris slot name #19
Conversation
|
@mihaiplesa we should probably run this through QA, wdyt? @jayschwa thanks for the pr! Would you have any test steps to share? So we could verify that the change does what it's supposed to and doesn't break anything? |
|
The easiest way to test this is to use a GNOME-based Linux desktop environment (e.g. Ubuntu 22.xx) and play a video on YouTube. Media controls should appear in GNOME's notification area (accessible by clicking date-time area at top of desktop). Chrome, Firefox, and APT-installed Brave will all show the media controls. Snap-installed Brave will not. After applying this patch, re-test and observe the media controls are now present. I have been running my full-time browser with this patch since I created the PR and have not observed any regressions. |
|
Thanks @jayschwa. That sounds pretty straightforward. We'll merge the other open PR first, once we're done testing it, and then move on to this one. |
|
Okay. I'll rebase this PR after #18 is merged. |
|
#18 is merged |
MPRIS enables desktop environments (e.g. GNOME) to provide media player controls for media playing in the browser. Brave uses DBus name `org.mpris.MediaPlayer2.chromium` whereas Snap sets an AppArmor rule for `org.mpris.MediaPlayer2.brave` by default. Setting the name parameter explicitly for the slot makes Snap generate a compatible AppArmor rule. Fixes brave/brave-browser#16187
|
That looks good to me, but I wonder: should we fix the dbus name instead and identify as |
I have rebased and am now running with the new build as my main browser.
That would probably be a cleaner fix, yes. However, I don't think there is any conflict or harm if it remains "chromium". I can play videos in both browsers at the same time and I don't notice any sort of problem w.r.t. the OS media controls. |
There was a problem hiding this comment.
That would probably be a cleaner fix, yes. However, I don't think there is any conflict or harm if it remains "chromium". I can play videos in both browsers at the same time and I don't notice any sort of problem w.r.t. the OS media controls.
Thanks for testing this.
This sounds like a good fix to me.
Resolves brave/brave-browser#16187
MPRIS enables desktop environments (e.g. GNOME) to provide media player
controls for media playing in the browser.
Brave uses DBus name
org.mpris.MediaPlayer2.chromiumwhereas Snap setsan AppArmor rule for
org.mpris.MediaPlayer2.braveby default. Settingthe name parameter explicitly for the slot makes Snap generate a
compatible AppArmor rule.