firefox-ios icon indicating copy to clipboard operation
firefox-ios copied to clipboard

Background audio plays after tab is closed in private browsing

Open data-sync-user opened this issue 1 year ago • 1 comments

Description

Background audio from a tab continues playing even when tab is closed in private browsing mode

Prerequisites:

  • App is in private browsing mode

Steps to reproduce:

  1. Open youtube.com (or any video streaming site)
  2. Play a video
  3. Open another tab and select it
  4. Close tab playing video

Expected results:

  • Background audio from video is no longer audible

Actual results:

  • Background audio from video is still audible

Notes:

  • Issue seems to only happens with videos playing in private browsing
  • Navigating to any website on another tab seems to resolve the issue

Device / build info:

  • Device: iPad Pro (M4) (iOS 17.5) (simulator)
  • Build: v133 (main branch)

┆Issue is synchronized with this Jira Bug

data-sync-user avatar Oct 18 '24 21:10 data-sync-user

➤ Matt Lichtenstein commented:

Also tested on iPhone 16 Pro (iOS 18.1)

App store v131.3:

  • Background audio stops once tab playing audio is closed

Nightly (46586)

  • Background audio stops once tab playing audio is switched away from

Have not yet tested on iPad - totally possible that this is iPad only

data-sync-user avatar Oct 18 '24 21:10 data-sync-user

➤ Matt Lichtenstein commented:

Tested on iPad Air (iOS 17.6.1)

App store v131.3:

  • Background audio stops once tab playing audio is closed

Nightly (46632)

  • Background audio continues to play after tab is closed

data-sync-user avatar Oct 21 '24 15:10 data-sync-user

➤ Andrei Bodea commented:

Closing as duplicate of https://github.com/mozilla-mobile/firefox-ios/issues/22089 ( https://github.com/mozilla-mobile/firefox-ios/issues/22089|smart-link )

data-sync-user avatar Oct 29 '24 15:10 data-sync-user

➤ Matt Reagan commented:

Reopening. This is still replicating on main AFAICS. This ticket is specific to audio continuing to play after a private tab is closed, but the ticket linked as the Duplicate appears to be for tabs that are still open.

data-sync-user avatar Nov 14 '24 23:11 data-sync-user

➤ Matt Reagan commented:

I can currently replicate this on main on an iPad Air 5th gen (attached video, make sure to enable audio). I seem to be able to replicate most easily via:

  1. Launch Firefox with no other tabs open (except single default homepage)
  2. Open a website with audio/video (in the existing homepage tab)
  3. Close the tab

Result: the audio continues playing.

!audiobug.mov|width=444,height=640,alt="audiobug.mov"!

data-sync-user avatar Nov 15 '24 02:11 data-sync-user

➤ Matt Reagan commented:

Note: I’ve confirmed this is also reproducible in current app store release (v132.1). Commit that introduced the bug is https://github.com/mozilla-mobile/firefox-ios/commit/d75728073f0fe31c9698298eb47bb2eea2513373 ( https://github.com/mozilla-mobile/firefox-ios/commit/d75728073f0fe31c9698298eb47bb2eea2513373|smart-link ) . The actual Tab appears to be retained by the backupCloseTab.

data-sync-user avatar Nov 15 '24 22:11 data-sync-user

➤ Matt Reagan commented:

A couple notes from initial investigation:

  • The audio/video privacy issue (and leak of the WKWebView) can be fixed by removing the !selectedTab.isFxHomeTab line (3699) added in https://github.com/mozilla-mobile/firefox-ios/commit/d75728073f0fe31c9698298eb47bb2eea2513373 ( https://github.com/mozilla-mobile/firefox-ios/commit/d75728073f0fe31c9698298eb47bb2eea2513373|smart-link )

    • Reverting this however appears to potentially introduce another issue where the homepage VC goes blank
  • There may be a 2nd leak (possibly a different bug) of the Tab itself which looks like it’s still not being deallocated; being retained by backupCloseTab

    • This leak can be avoided temporarily by commenting out the backupCloseTab update in L:626 of LegacyTabManager

data-sync-user avatar Nov 18 '24 16:11 data-sync-user

➤ Matt Reagan commented:

Updating priority based on initial team discussion.

data-sync-user avatar Nov 18 '24 19:11 data-sync-user

➤ Matt Reagan commented:

I have a PR up with an initial fix; Orla Mitchell should probably have final say in how this is addressed since I don’t have much context on the original commit or the blank homepage bugs (which regress when the line that introduced the leak is reverted; the open PR does appear to fix both of these problems but I know that we’ve had a lot of issues in this area previously, so it is quite brittle).

data-sync-user avatar Nov 19 '24 17:11 data-sync-user

➤ Matt Reagan commented:

Per team discussion, fix has been merged to main for now. Andrei Bodea if it’s possible to get QA to help regression test it would be very helpful. Please ping me with any questions. Summary: the fix should correct this audio/video problem, however it also touches code that is run whenever tabs are switched. So any scenarios involving tab switching (especially switching to homepage) would benefit from regression testing. Thank you.

data-sync-user avatar Nov 19 '24 21:11 data-sync-user

➤ Matt Reagan commented:

Marking fix version as 134 until we confirm 133 backport.

data-sync-user avatar Nov 19 '24 21:11 data-sync-user

➤ Matt Reagan commented:

It sounds like we’ll be merging the v133 backport for this. Going to update fix field. (cc Donal Meehan)

data-sync-user avatar Nov 20 '24 23:11 data-sync-user

➤ Diana Andreea Barladeanu commented:

Verified as fixed on v133 (47866), with iPhone 16+(18.1).

data-sync-user avatar Nov 21 '24 09:11 data-sync-user

➤ Matt Reagan commented:

{quote}Verified as fixed on v133 (47866), with iPhone 16+(18.1).{quote}

Diana Andreea Barladeanu Can you also please be sure to test this with iPad? The audio/video will only continue playing on iPad devices (even though the leak also impacts iPhones, the media playback stops on those devices).

data-sync-user avatar Nov 21 '24 20:11 data-sync-user

➤ Diana Andreea Barladeanu commented:

Matt Reagan The fix was validated on multiple ipads, but apparently I forgot to mention them. I've edited the comment. Thanks!

data-sync-user avatar Nov 21 '24 21:11 data-sync-user