Closed Bug 1458383 Opened 3 years ago Closed 3 years ago

Pinned tabs and blocked autoplay


(Core :: Audio/Video: Playback, enhancement, P2)




Tracking Status
firefox62 --- fixed


(Reporter: cpearce, Assigned: cpearce)




(4 files)

Should pinned tabs be able to autoplay?

Our current default behaviour (before block-autoplay) is for tabs loaded in the background to have playback delayed until the user focuses the tab. So if a page tries to play before the page has been foregrounded, we'll only actually start to play the media when user brings the tab to the foreground.

This is different for pinned tabs, which are allowed to play upon session restore without being foregrounded first.

This was to enable pinned mail/chat tabs being able to play sounds when showing notifications for new messages etc.

Rationale/previous decision:

Unfortunately some users pin tabs to enable quick re-access upon session restore, and some users pin tabs for sites that like to autoplay media, but the users don't want them to autoplay, for example YouTube tabs:
bug 1449022, and bug 1378172 comment 4.

Also note that many of these media sites we expect will be whitelisted to allow autoplay, further defeating some users' expectations. IMO, the whitelist should be consulted *after* we've checked whether the page has been in the foreground to avoid that issue.
I'd overlooked that the block-autoplay UX spec specifies that pinned tabs should no longer be able to autoplay, but should instead show an audio tab indicator:

Having discussed this with Cindy, we agreed this is a good behaviour. Coupled with delaying playback start, this ensures that pinned tabs, even if for whitelisted origins, will not be a nuisance. If a tab is trying to play sounds to cause a notification, the audio tab indicator will show which brings attention to the tab, including if it's pinned.
Resolving, as we've decided to not have any special handling for pinned tabs. So nothing to do here.
Closed: 3 years ago
Resolution: --- → WORKSFORME
(In reply to Chris Pearce (:cpearce) from comment #2)
> So nothing to do here.

This is incorrect.

With media.autoplay.enabled=false and media.autoplay.enabled.user-gestures-needed=true we're still autoplay in pinned tabs in whitelisted origins upon session restore. We need to partially backout Bug 1347791 (I think just patch part 4, commit 49b533231388, to stop the block status being session restored), and also we need to change nsPIDOMWindowOuter::mMediaSuspend so that it's initialized by default to SUSPENDED_BLOCK when gesture activation is required.
Resolution: WORKSFORME → ---
Comment on attachment 8974252 [details]
Bug 1458383 - Don't dispatch mediaBlockStop notification.
Attachment #8974252 - Flags: review?(amarchesini) → review+
Comment on attachment 8974253 [details]
Bug 1458383 - Assume tabs delay playback start of media.
Attachment #8974253 - Flags: review?(amarchesini) → review+
Duplicate of this bug: 1449022
Comment on attachment 8974250 [details]
Bug 1458383 - Don't session restore browser.mediaBlocked.

I trust that you can resolve the issue below without the need to have me check this again ;-)

::: browser/base/content/tabbrowser.js
(Diff revision 2)
> -            };
> -          break;
>          case "userTypedValue":
>          case "userTypedClear":
> -        case "mediaBlocked":
> -          getter = () => SessionStore.getLazyTabValue(aTab, name);

You're removing too much here! Please keep `getter = () => SessionStore.getLazyTabValue(aTab, name);`, otherwise urlbar content restoration will be broken.
Attachment #8974250 - Flags: review?(mdeboer) → review+
Comment on attachment 8974251 [details]
Bug 1458383 - Remove browser mediaBlocked attribute.

Cool! Nice cleanup.
Attachment #8974251 - Flags: review?(mdeboer) → review+
Pushed by
Don't session restore browser.mediaBlocked. r=mikedeboer
Remove browser mediaBlocked attribute. r=mikedeboer
Don't dispatch mediaBlockStop notification. r=baku
Assume tabs delay playback start of media. r=baku
Assignee: nobody → cpearce
Hi Chris. I saw that here are some automated tests here. Does this require manual testing in these circumstances?
Flags: qe-verify?
Flags: needinfo?(cpearce)
I don't think we need any more manual QA here. Thanks!
Flags: needinfo?(cpearce)
Flags: qe-verify?
You need to log in before you can comment on or make changes to this bug.