From ae5348ffa3b264743e93cec766feb14c5f284fa3 Mon Sep 17 00:00:00 2001 From: Brandon T Date: Tue, 12 Dec 2023 15:13:58 -0500 Subject: [PATCH] Fix Playlist Managed popup showing when it shouldn't Fix Playlist not pausing the web-page video on iPad when playing (so users hear double audio) Fix Playlist not continuing to the next video/audio in Picture-In-Picture Signed-off-by: Brandon T --- ...tListViewController+TableViewDataSource.swift | 2 +- .../Controllers/PlaylistListViewController.swift | 2 +- .../Controllers/PlaylistViewController.swift | 8 +++++++- .../PlaylistCarplayManager.swift | 7 +++---- .../Scripts/Paged/PlaylistScript.js | 16 ++++++++-------- 5 files changed, 20 insertions(+), 15 deletions(-) diff --git a/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController+TableViewDataSource.swift b/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController+TableViewDataSource.swift index 802dbab8daa..89ca1d62737 100644 --- a/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController+TableViewDataSource.swift +++ b/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController+TableViewDataSource.swift @@ -178,7 +178,7 @@ extension PlaylistListViewController: UITableViewDataSource { header.onAddPlaylist = { [unowned self] in guard let sharedFolderUrl = folder.sharedFolderUrl else { return } - if PlayListDownloadType(rawValue: Preferences.Playlist.autoDownloadVideo.value) != nil { + if PlayListDownloadType(rawValue: Preferences.Playlist.autoDownloadVideo.value) != .off { let controller = PopupViewController(rootView: PlaylistFolderSharingManagementView(onAddToPlaylistPressed: { [unowned self] in self.dismiss(animated: true) diff --git a/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController.swift b/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController.swift index 876c2b85d6e..794c15548df 100644 --- a/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController.swift +++ b/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistListViewController.swift @@ -637,7 +637,7 @@ extension PlaylistListViewController { return } - playerView.stop() + playerView.pause() playerView.bringSubviewToFront(activityIndicator) activityIndicator.startAnimating() activityIndicator.isHidden = false diff --git a/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistViewController.swift b/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistViewController.swift index 400a7cc7241..3f33415fb24 100644 --- a/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistViewController.swift +++ b/Sources/Brave/Frontend/Browser/Playlist/Controllers/PlaylistViewController.swift @@ -581,6 +581,7 @@ extension PlaylistViewController: PlaylistViewControllerDelegate { stop(playerView) // Cancel all loading. + PlaylistManager.shared.playbackTask?.cancel() PlaylistManager.shared.playbackTask = nil } @@ -864,7 +865,12 @@ extension PlaylistViewController: VideoViewDelegate { } func load(_ videoView: VideoView, asset: AVURLAsset, autoPlayEnabled: Bool) async throws /*`MediaPlaybackError`*/ { - self.clear() + // Task will be nil if the playback has stopped, but not paused + // If it is paused, and we're loading another track, don't bother clearing the player + // as this will break PIP + if PlaylistManager.shared.playbackTask == nil { + self.clear() + } let isNewItem = try await player.load(asset: asset) diff --git a/Sources/Brave/Frontend/Browser/Playlist/Managers & Cache/PlaylistCarplayManager.swift b/Sources/Brave/Frontend/Browser/Playlist/Managers & Cache/PlaylistCarplayManager.swift index 0fd90896ccb..54aa844419c 100644 --- a/Sources/Brave/Frontend/Browser/Playlist/Managers & Cache/PlaylistCarplayManager.swift +++ b/Sources/Brave/Frontend/Browser/Playlist/Managers & Cache/PlaylistCarplayManager.swift @@ -108,12 +108,11 @@ public class PlaylistCarplayManager: NSObject { func getPlaylistController(tab: Tab?, initialItem: PlaylistInfo?, initialItemPlaybackOffset: Double) -> PlaylistViewController { - // If background playback is enabled, tabs will continue to play media + // If background playback is enabled (on iPhone), tabs will continue to play media // Even if another controller is presented and even when PIP is enabled in playlist. // Therefore we need to stop the page/tab from playing when using playlist. - if Preferences.General.mediaAutoBackgrounding.value { - tab?.stopMediaPlayback() - } + // On iPad, media will continue to play with or without the background play setting. + tab?.stopMediaPlayback() // If there is no media player, create one, // pass it to the play-list controller diff --git a/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Paged/PlaylistScript.js b/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Paged/PlaylistScript.js index d172dc9422f..ae429ce9a31 100644 --- a/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Paged/PlaylistScript.js +++ b/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Paged/PlaylistScript.js @@ -164,6 +164,14 @@ window.__firefox__.includeOnce("Playlist", function($) { style.visibility !== 'hidden'; } + function getAllVideoElements() { + return [...document.querySelectorAll('video')].reverse(); + } + + function getAllAudioElements() { + return [...document.querySelectorAll('audio')].reverse(); + } + function setupLongPress() { Object.defineProperty(window.__firefox__, '$', { enumerable: false, @@ -220,14 +228,6 @@ window.__firefox__.includeOnce("Playlist", function($) { // MARK: --------------------------------------- function setupDetector() { - function getAllVideoElements() { - return [...document.querySelectorAll('video')].reverse(); - } - - function getAllAudioElements() { - return [...document.querySelectorAll('audio')].reverse(); - } - function requestWhenIdleShim(fn) { var start = Date.now() return setTimeout(function () {