[iOS] - Fix playlist auto-download when adding an item #28594
Conversation
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Playlist.swift
Outdated
Show resolved
Hide resolved
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BVC+Playlist.swift
Outdated
Show resolved
Hide resolved
9b191a4 to
0992766
Compare
| self.updatePlaylistURLBar( | ||
| tab: self.tabManager.selectedTab, | ||
| state: .existingItem, | ||
| item: newItem | ||
| ) |
There was a problem hiding this comment.
After a lot of testing, it's not needed, so I removed it. My main concern was that when I open the video from the page, it'd open the incorrect video because the URL is different and because of threading differences (page might update the URL before the func runs), but I was wrong. It actually opens correctly and it deletes correctly.
| let newItem = (try? await mediaStreamer.loadMediaStreamingAsset(item)) ?? item | ||
| PlaylistManager.shared.autoDownload(item: newItem) |
There was a problem hiding this comment.
We should just not bother auto-downloading if loadMediaStreamingAsset fails since its not going to work anyways
| isInvisible: item.isInvisible | ||
| ) | ||
| item.pageSrc = tab.visibleURL?.absoluteString ?? item.pageSrc | ||
| item.pageTitle = tab.title ?? item.pageTitle |
There was a problem hiding this comment.
lastPlayedOffset was reset to 0 before change
There was a problem hiding this comment.
I had removed it because I didn't see it making any difference in practice, and I didn't want to modify the last played offset for a video which might have updated.
So I made a test, I created a video, then added it to playlist. Then I modified that video to something else on Youtube, and when the video in playlist updated, the offset was wrong. So there is good reason to set it to 0.0 here.
I reverted the change.
| else { | ||
| return | ||
| } | ||
| public func canAutoDownload(item: PlaylistInfo) -> Bool { |
There was a problem hiding this comment.
item is not used here, did you mean to also check the item's url scheme for blob here?
There was a problem hiding this comment.
I was going to remove this change, but decided to keep it lol. So I added the blob check which is a good idea.
0992766 to
e66671d
Compare
|
Chromium major version is behind target branch (135.0.7049.100 vs 136.0.7103.25). Please rebase. |
e66671d to
f2f68c7
Compare
|
Released in v1.79.75 |
Summary
Resolves brave/brave-browser#44652
Test Plan
Screenshots
itemhas asrcwithblob:https://m.youtube.com/....newItemhas asrcwithhttps://rr5---snand it downloadsnewItem:PlaylistManager.shared.autoDownload(item: newItem)