Differential D20724

Keep player around when emptied but the website tells us it's actually just paused
ClosedPublic
Actions

Authored by broulik on Apr 22 2019, 8:39 AM.

Details

Reviewers
fvogt
Group Reviewers
Plasma
Commits
R856:eb583434a561: Keep player around when emptied but the website tells us it's actually just…
Summary

Requires "Enhanced media controls" to be enabled in extension settings to work

BUG: 402324

Test Plan

Paused soundcloud, no longer had the player disappear and switch to one on a different tab

Diff Detail

Repository
R856 Plasma Browser Integration
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Apr 22 2019, 8:39 AM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 22 2019, 8:39 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Apr 22 2019, 8:39 AM
fvogt added inline comments.Apr 22 2019, 8:48 AM
extension/content-script.js
373

Doesn't player.paused work here?

broulik added inline comments.Apr 22 2019, 8:52 AM
extension/content-script.js
373

No. This case here is so when you empty the player, e.g. use ajax page nav to navigate away, we stop controlling the player. It is always paused when emptied, so this would just noop.
With media session api we give the website some control of providing a playback status we communicate to the user regardless of their implementation detail.
In theory they could even show media controls without ever using a HTML5 audio/video object (which we don't support as of now) purely using media sessions

fvogt added inline comments.Apr 22 2019, 9:02 AM
extension/content-script.js
373

Ok, so this is even more of a hack than I thought. Can you add a && player == activePlayer check there?

broulik updated this revision to Diff 56711.Apr 22 2019, 9:22 AM
  • Add active player check
fvogt accepted this revision.Apr 22 2019, 9:25 AM
This revision is now accepted and ready to land.Apr 22 2019, 9:25 AM
Closed by commit R856:eb583434a561: Keep player around when emptied but the website tells us it's actually just… (authored by broulik). · Explain WhyApr 22 2019, 9:29 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mextension/content-script.js (10 lines)

Diff 56711

View Options

extension/content-script.js

Loading...