From 9ac45035ff0e24a1e1d5afd0038f56e6fc6403dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Serta=C3=A7=20=C3=96zercan?= <852750+sozercan@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:21:03 -0700 Subject: [PATCH 1/2] Integrate upstream #408 playback reliability (hand-merged onto the fork) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-pick of upstream sozercan/kaset dfdb4ba ("fix: improve YouTube playback reliability"), hand-resolved against the fork's 175-commit divergence and reconciled with the fork's own #19 (⌘R refresh + network auto-retry). Taken from #408 (the reliability core): - Loading-timeout + media-error recovery in YouTubePlayerService (finishPlaybackLoadingIfReady / deferMediaErrorIfNeeded, ready-media based), replacing the fork's duration>0 loading-clear proxy. Kept the fork's #374 conditional-assignment perf optimization and ad-skip/live-edge fields. - Runtime liveness (currentMediaIsLive) for URL launches / SPA drift; adopted in the seek-to-end guard. - ScrollForwardingWebView (only forwards wheel events off the extracted watch doc, not off consent/sign-in/CAPTCHA interstitials). - Generalized interstitial handling (isTrustedIntermediaryURL + revealInterstitialScript) in the watch coordinator; kept the fork's clearPlaybackLoadingForInterstitial() call. - YouTubeItemParser thumbnail helpers; new document-generation/loading-timeout/ media-error test suites. Kept the fork's version (superseded upstream's incidental UI changes): - YouTubePlayerBar, YouTubeShortsView, YouTubeVideoWindowController — the fork deliberately reworked these (overflow menu, shorts rail, PiP always-on-top). - Subscribe strings retranslated to the free-subscribe sense ("Iscriviti"), matching the app's abbonati(paid)/iscriviti(free) terminology. Notes: - Dropped upstream's YouTubeVideoWindowFrameAutosaveState/FullscreenIntent refactor (fork handles fullscreen-frame persistence inline + keeps PiP posture) and its extraction-script test harness (fork extracts differently). - Scoped LocalizationCatalogParityTests to the fork's shipped locale subset. - Pre-existing test-target failures on main (AppLocalizationTests bundle loading, SettingsManager/ListenBrainz, storyboard flake) are unchanged; no playback/reliability test regressed. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 8 + Sources/Kaset/Resources/Localizable.xcstrings | 8 +- .../Resources/it.lproj/Localizable.strings | 8 +- .../Parsers/YouTube/YouTubeItemParser.swift | 35 ++- .../WebPlaybackDocumentGeneration.swift | 33 +++ .../Player/YouTubePlayerService+Seeking.swift | 13 +- .../Player/YouTubePlayerService.swift | 183 ++++++++++-- .../YouTube/ScrollForwardingWebView.swift | 32 ++ .../YouTube/YouTubeWatchSurfaceView.swift | 39 ++- .../YouTubeWatchWebView+Coordinator.swift | 144 +++++++-- ...uTubeWatchWebView+DocumentGeneration.swift | 183 ++++++++++++ .../YouTube/YouTubeWatchWebView+Scripts.swift | 67 ++++- .../Views/YouTube/YouTubeWatchWebView.swift | 111 +------ Tests/KasetTests/AppLocalizationTests.swift | 2 +- ...itesManagerLegacyMigrationClaimTests.swift | 23 +- .../LocalizationCatalogParityTests.swift | 155 ++++++++++ .../WebPlaybackDocumentGenerationTests.swift | 81 +++++ .../YouTubeDocumentStartAudioGuardTests.swift | 251 ++++++++++++++++ ...uTubeInterstitialScriptContractTests.swift | 19 ++ Tests/KasetTests/YouTubeItemParserTests.swift | 110 +++++++ .../YouTubePlayerLoadingTimeoutTests.swift | 279 ++++++++++++++++++ .../YouTubePlayerMediaErrorTests.swift | 113 +++++++ .../YouTubePlayerRecoveryTests.swift | 2 + .../YouTubePlayerServiceTests.swift | 159 +++++++++- .../YouTubeWatchLiveStateScriptTests.swift | 27 ++ .../KasetTests/YouTubeWatchScriptTests.swift | 2 + .../YouTubeWatchSurfaceOwnershipTests.swift | 64 ++++ ...6-generation-scoped-web-playback-bridge.md | 28 +- 28 files changed, 1990 insertions(+), 189 deletions(-) create mode 100644 Sources/Kaset/Views/YouTube/ScrollForwardingWebView.swift create mode 100644 Tests/KasetTests/LocalizationCatalogParityTests.swift create mode 100644 Tests/KasetTests/YouTubeDocumentStartAudioGuardTests.swift create mode 100644 Tests/KasetTests/YouTubeInterstitialScriptContractTests.swift create mode 100644 Tests/KasetTests/YouTubePlayerLoadingTimeoutTests.swift create mode 100644 Tests/KasetTests/YouTubePlayerMediaErrorTests.swift create mode 100644 Tests/KasetTests/YouTubeWatchLiveStateScriptTests.swift create mode 100644 Tests/KasetTests/YouTubeWatchSurfaceOwnershipTests.swift diff --git a/AGENTS.md b/AGENTS.md index a7b05bc4..a3290dc8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,14 @@ Default local workflow is CLI-first: use the commands above for day-to-day verif > ⚠️ **SwiftFormat `--self insert` rule**: The project uses `--self insert` in `.swiftformat`. In static methods, call other static methods with `Self.methodName()` (not bare `methodName()`); in instance methods, use `self.property` explicitly. +## Localization + +> 🌐 **`Sources/Kaset/Resources/Localizable.xcstrings` is the localization source of truth.** Packaged builds compile the catalog directly, but SwiftPM/Xcode runtime builds use the checked-in `Sources/Kaset/Resources/*.lproj/Localizable.strings` mirrors. + +- When adding localization keys or changing translations, update the catalog first and update the corresponding checked-in `.lproj/Localizable.strings` files in the same change. Never update only one side. +- Run `swift test --skip KasetUITests --filter LocalizationCatalogParityTests` after localization changes. +- When adding a locale, also register its `.lproj` in `Package.swift`, add the `SettingsManager.ContentLanguage` case, and extend localization tests. + ## Debugging & Measurement > 🔬 **Measure before you fix — never guess at runtime behavior.** For any bug about *timing, lifecycle, or "why didn't this run/load/update"* (SwiftUI `.task`/state churn, cold-launch ordering, perceived latency), instrument the real code path and observe before changing anything. Reasoning about SwiftUI lifecycle or async ordering from the source alone is unreliable; a 10-line timestamped trace settles in one launch what hours of hypothesizing cannot. Add the trace → reproduce → read the evidence → fix the thing the data points at → re-measure to confirm → remove the instrumentation. diff --git a/Sources/Kaset/Resources/Localizable.xcstrings b/Sources/Kaset/Resources/Localizable.xcstrings index dc5d8e96..c6b81849 100644 --- a/Sources/Kaset/Resources/Localizable.xcstrings +++ b/Sources/Kaset/Resources/Localizable.xcstrings @@ -101724,7 +101724,7 @@ "it" : { "stringUnit" : { "state" : "translated", - "value" : "Abbonati a %@" + "value" : "Iscriviti a %@" } }, "ja" : { @@ -101909,7 +101909,7 @@ "it" : { "stringUnit" : { "state" : "translated", - "value" : "Abbonati ai podcast su YouTube Music per vederli qui." + "value" : "Iscriviti ai podcast su YouTube Music per vederli qui." } }, "ja" : { @@ -102279,7 +102279,7 @@ "it" : { "stringUnit" : { "state" : "translated", - "value" : "Errore nell'abbonamento" + "value" : "Errore di iscrizione" } }, "ja" : { @@ -102464,7 +102464,7 @@ "it" : { "stringUnit" : { "state" : "translated", - "value" : "Abbonamenti" + "value" : "Iscrizioni" } }, "ja" : { diff --git a/Sources/Kaset/Resources/it.lproj/Localizable.strings b/Sources/Kaset/Resources/it.lproj/Localizable.strings index 1f1bd3ae..6612d892 100644 --- a/Sources/Kaset/Resources/it.lproj/Localizable.strings +++ b/Sources/Kaset/Resources/it.lproj/Localizable.strings @@ -549,11 +549,11 @@ "Style" = "Stile"; "Submit" = "Invia"; "Subscribe" = "Iscriviti"; -"Subscribe %@" = "Abbonati a %@"; -"Subscribe to podcasts on YouTube Music to see them here." = "Abbonati ai podcast su YouTube Music per vederli qui."; +"Subscribe %@" = "Iscriviti a %@"; +"Subscribe to podcasts on YouTube Music to see them here." = "Iscriviti ai podcast su YouTube Music per vederli qui."; "Subscribed" = "Iscritto"; -"Subscription Error" = "Errore nell'abbonamento"; -"Subscriptions" = "Abbonamenti"; +"Subscription Error" = "Errore di iscrizione"; +"Subscriptions" = "Iscrizioni"; "Suggested" = "Suggerito"; "Suggested Removals" = "Rimozioni suggerite"; "Suggestions per insertion" = "Suggerimenti per inserimento"; diff --git a/Sources/Kaset/Services/API/Parsers/YouTube/YouTubeItemParser.swift b/Sources/Kaset/Services/API/Parsers/YouTube/YouTubeItemParser.swift index bf28290f..debbb11e 100644 --- a/Sources/Kaset/Services/API/Parsers/YouTube/YouTubeItemParser.swift +++ b/Sources/Kaset/Services/API/Parsers/YouTube/YouTubeItemParser.swift @@ -146,9 +146,7 @@ enum YouTubeItemParser { return nil } - let sources = ( - (lockup["thumbnailViewModel"] as? [String: Any])?["image"] as? [String: Any] - )?["sources"] as? [[String: Any]] + let sources = self.imageSources(fromThumbnailViewModel: lockup["thumbnailViewModel"]) return YouTubeVideo( videoId: videoId, @@ -355,16 +353,33 @@ enum YouTubeItemParser { /// wrap it in `collectionThumbnailViewModel.primaryThumbnail` (the stacked /// poster), so fall back to that path. static func thumbnailURL(fromLockup lockup: [String: Any]) -> URL? { - let contentImage = lockup["contentImage"] as? [String: Any] - let thumbnailViewModel = contentImage?["thumbnailViewModel"] as? [String: Any] - ?? ((contentImage?["collectionThumbnailViewModel"] as? [String: Any])?["primaryThumbnail"] - as? [String: Any])?["thumbnailViewModel"] as? [String: Any] - let sources = (thumbnailViewModel?["image"] as? [String: Any])?["sources"] - as? [[String: Any]] - guard let sources else { return nil } + guard let contentImage = lockup["contentImage"] as? [String: Any] else { return nil } + + if let sources = self.imageSources(fromThumbnailViewModel: contentImage["thumbnailViewModel"]) { + return self.bestSourceURL(from: sources) + } + + // Playlist and album lockups stack their poster behind a collection wrapper. + let collection = contentImage["collectionThumbnailViewModel"] as? [String: Any] + guard let sources = self.imageSources( + fromThumbnailViewModel: collection?["primaryThumbnail"] + ) else { + return nil + } return self.bestSourceURL(from: sources) } + /// Extracts `image.sources` from a `thumbnailViewModel`, tolerating the + /// singly-nested form InnerTube uses for Shorts and collection lockups. + static func imageSources(fromThumbnailViewModel value: Any?) -> [[String: Any]]? { + guard let container = value as? [String: Any] else { return nil } + if let sources = (container["image"] as? [String: Any])?["sources"] as? [[String: Any]] { + return sources + } + let nested = container["thumbnailViewModel"] as? [String: Any] + return (nested?["image"] as? [String: Any])?["sources"] as? [[String: Any]] + } + /// Picks the largest entry from an array of `{url, width, height}` sources, /// normalizing protocol-relative URLs. static func bestSourceURL(from sources: [[String: Any]]) -> URL? { diff --git a/Sources/Kaset/Services/Player/WebPlaybackDocumentGeneration.swift b/Sources/Kaset/Services/Player/WebPlaybackDocumentGeneration.swift index 2c6e224d..255da085 100644 --- a/Sources/Kaset/Services/Player/WebPlaybackDocumentGeneration.swift +++ b/Sources/Kaset/Services/Player/WebPlaybackDocumentGeneration.swift @@ -352,6 +352,14 @@ struct WebPlaybackDocumentGeneration: Equatable { return host == playbackHost.lowercased() || Self.trustedRedirectHosts.contains(host) } + // Google's "unusual traffic" challenge (`www.google.com/sorry/…`) is + // deliberately NOT trusted here. Allowing the URL alone does not make it + // reachable: the challenge is commonly served as HTTP 429, which + // `acceptsMainFrameResponse` rejects before commit, and clearing it posts + // `g-recaptcha-response` as a form body that `requestByBindingGeneration` + // cannot carry across a generation rebind. Supporting it needs both of + // those handled together. + static func isTrustedIntermediaryURL(_ url: URL?) -> Bool { guard let url, url.scheme?.lowercased() == "https", @@ -384,6 +392,31 @@ struct WebPlaybackDocumentGeneration: Equatable { && Self.isAllowedPlaybackNavigationURL(currentURL, playbackHost: playbackHost) } + /// A form submission from a committed trusted intermediary must keep the + /// original WebKit navigation so its HTTP body is preserved. GET + /// continuations can be rebound with a generation token, but WebKit omits + /// form bodies from the navigation-policy request used to reconstruct them. + static func shouldAllowTrustedIntermediaryFormSubmission( + _ request: URLRequest, + currentURL: URL?, + generation: UInt64, + playbackHost: String, + committedIntermediaryGeneration: UInt64? + ) -> Bool { + guard request.httpMethod?.uppercased() == "POST", + committedIntermediaryGeneration == generation, + self.isTrustedIntermediaryURL(currentURL), + self.isTrustedIntermediaryURL(request.url), + Self.generation(from: request.url) == nil + else { return false } + + let mainDocumentGeneration = Self.generation(from: request.mainDocumentURL) + guard mainDocumentGeneration == nil || mainDocumentGeneration == generation else { + return false + } + return Self.isAllowedPlaybackNavigationURL(request.url, playbackHost: playbackHost) + } + static func requestByBindingGeneration( _ request: URLRequest, generation: UInt64 diff --git a/Sources/Kaset/Services/Player/YouTubePlayerService+Seeking.swift b/Sources/Kaset/Services/Player/YouTubePlayerService+Seeking.swift index fd3b01af..ba0c72ab 100644 --- a/Sources/Kaset/Services/Player/YouTubePlayerService+Seeking.swift +++ b/Sources/Kaset/Services/Player/YouTubePlayerService+Seeking.swift @@ -36,10 +36,15 @@ extension YouTubePlayerService { guard time.isFinite else { return } self.invalidateExplicitStartTargetForUserSeek() let target = self.clampedSeekTarget(time) - // A live stream never "ends" when you catch up to the edge — seeking to - // the end of the DVR window just parks at the live head, so don't treat - // it as a completed watch (which would pause the stream). - if !self.isLive, self.duration > 0, target >= self.duration - Self.seekToEndThreshold { + // A live stream's duration is its DVR window, not a finish line, so + // seeking to the edge must not count as watching to the end. Runtime + // bridge state covers URL launches and SPA drift whose feed model did + // not carry liveness. + if self.currentVideo?.isLive != true, + !self.currentMediaIsLive, + self.duration > 0, + target >= self.duration - Self.seekToEndThreshold + { self.handleManualSeekToEnd() return } diff --git a/Sources/Kaset/Services/Player/YouTubePlayerService.swift b/Sources/Kaset/Services/Player/YouTubePlayerService.swift index f61d6ba2..2734e096 100644 --- a/Sources/Kaset/Services/Player/YouTubePlayerService.swift +++ b/Sources/Kaset/Services/Player/YouTubePlayerService.swift @@ -208,6 +208,13 @@ final class YouTubePlayerService { /// Whether the current video is waiting for the WebView to report playable media. private(set) var isPlaybackLoading = false + /// Runtime liveness from the active media element. This covers videos whose + /// feed model is incomplete, including URL launches and SPA drift. + @ObservationIgnored var currentMediaIsLive = false + + @ObservationIgnored private var playbackLoadingTimeoutTask: Task? + @ObservationIgnored private var playbackLoadingGeneration: UInt64 = 0 + /// SponsorBlock segments for the current video ([start, end] in seconds, by category). /// Updated by the WebView observer when segments are fetched from the SponsorBlock API. var sponsorSegments: [SponsorSegment] = [] @@ -483,6 +490,7 @@ final class YouTubePlayerService { private let webKitManager: WebKitManager let playbackController: any YouTubeWatchPlaybackControlling private var usesCookieFreePlaybackDataStore = false + private let playbackLoadingTimeout: Duration private let logger = DiagnosticsLogger.player /// Whether a playing video should pop out into the floating window when the @@ -494,11 +502,13 @@ final class YouTubePlayerService { init( webKitManager: WebKitManager = .shared, playbackController: (any YouTubeWatchPlaybackControlling)? = nil, - shouldPopOutOnNavigateAway: @escaping @MainActor () -> Bool = { SettingsManager.shared.popOutVideoOnNavigateAway } + shouldPopOutOnNavigateAway: @escaping @MainActor () -> Bool = { SettingsManager.shared.popOutVideoOnNavigateAway }, + playbackLoadingTimeout: Duration = .seconds(15) ) { self.webKitManager = webKitManager self.playbackController = playbackController ?? YouTubeWatchWebView.shared self.shouldPopOutOnNavigateAway = shouldPopOutOnNavigateAway + self.playbackLoadingTimeout = playbackLoadingTimeout } // MARK: - Commands @@ -532,7 +542,7 @@ final class YouTubePlayerService { seconds: normalizedStartAt ) } - self.isPlaybackLoading = true + self.beginPlaybackLoading() self.surfaceLocation = .inline // Create the WebView on demand; containers reparent it on appear. @@ -602,7 +612,7 @@ final class YouTubePlayerService { resumeAt: resumeAt ) self.isIdentityReloadInFlight = true - self.isPlaybackLoading = true + self.beginPlaybackLoading() } // MARK: - Refresh (⌘R) & network auto-retry @@ -725,7 +735,7 @@ final class YouTubePlayerService { self.autoplayRecoveryRequestGeneration &+= 1 self.isPlaying = false self.isIdentityReloadInFlight = false - self.isPlaybackLoading = false + self.finishPlaybackLoading() self.hasObservedPausedMedia = true return } @@ -738,14 +748,14 @@ final class YouTubePlayerService { self.userUpdatedPendingPausedIdentityReloadSeek = false self.isIdentityReloadInFlight = false self.isPlaying = false - self.isPlaybackLoading = false + self.finishPlaybackLoading() return } self.playbackController.reloadVideo(videoId: currentVideo.videoId, resumeAt: resumeAt) self.isPlaying = false self.isIdentityReloadInFlight = true - self.isPlaybackLoading = true + self.beginPlaybackLoading() } /// Leaves a failed watch-page navigation in an explicitly retryable paused @@ -770,9 +780,15 @@ final class YouTubePlayerService { self.userUpdatedPendingPausedIdentityReloadSeek = false self.isIdentityReloadInFlight = false self.desiredPlaybackIntent = .paused + // A navigation may have finished before its media became ready, leaving + // no tracked load for the controller to cancel. Keep late playing + // samples from re-authorizing that document or consuming the deferred + // reload before an explicit user resume. + self.isExplicitPauseIntentActive = true self.isAwaitingResumeConfirmation = false + self.hasObservedPausedMedia = true self.isPlaying = false - self.isPlaybackLoading = false + self.finishPlaybackLoading() } /// Toggles play/pause. @@ -874,7 +890,7 @@ final class YouTubePlayerService { self.desiredPlaybackIntent = .playing self.isExplicitPauseIntentActive = false self.isIdentityReloadInFlight = true - self.isPlaybackLoading = true + self.beginPlaybackLoading() return true } @@ -913,7 +929,7 @@ final class YouTubePlayerService { self.isIdentityReloadInFlight = false } self.isPlaying = false - self.isPlaybackLoading = false + self.finishPlaybackLoading() } /// Stops playback entirely and releases the surface. @@ -943,7 +959,7 @@ final class YouTubePlayerService { self.isExplicitPauseIntentActive = true self.isAwaitingResumeConfirmation = false self.isPlaying = false - self.isPlaybackLoading = false + self.finishPlaybackLoading() self.isIdentityReloadInFlight = false self.progress = 0 self.duration = 0 @@ -1079,7 +1095,7 @@ final class YouTubePlayerService { self.currentWatchConcluded = false self.isIdentityReloadInFlight = false self.resetPerVideoState() - self.isPlaybackLoading = true + self.beginPlaybackLoading() self.playbackController.prepare( webKitManager: self.webKitManager, playerService: self, @@ -1144,6 +1160,8 @@ final class YouTubePlayerService { self.activeCaptionLanguageCode = nil self.qualityLevels = [] self.currentQuality = nil + self.userPinnedQuality = nil + self.currentMediaIsLive = false self.storyboardSpec = nil self.sponsorSegments = [] self.dismissSponsorBlockSkipNotice() @@ -1208,7 +1226,12 @@ final class YouTubePlayerService { guard self.currentVideo?.videoId == videoId else { return } self.captionTracks = tracks - self.qualityLevels = await self.playbackController.availableQualityLevels() + let levels = await self.playbackController.availableQualityLevels() + // The player accepts "auto" even when it does not advertise it, and + // the menu needs that row to represent an unpinned quality. + self.qualityLevels = levels.isEmpty || levels.contains("auto") + ? levels + : levels + ["auto"] self.currentQuality = await self.playbackController.currentQualityLevel() self.activeCaptionLanguageCode = await self.playbackController.currentCaptionLanguageCode() @@ -1317,6 +1340,41 @@ final class YouTubePlayerService { /// source: the inline surface pauses in place (no pop-out window) and /// the restored watch view re-adopts it when the user comes back. private var pauseInPlaceOnDisappear = false +} + +extension YouTubePlayerService { + private func beginPlaybackLoading() { + self.playbackLoadingTimeoutTask?.cancel() + self.playbackLoadingGeneration &+= 1 + let loadingGeneration = self.playbackLoadingGeneration + self.isPlaybackLoading = true + + let timeout = self.playbackLoadingTimeout + let videoId = self.currentVideo?.videoId + self.playbackLoadingTimeoutTask = Task { [weak self] in + do { + try await Task.sleep(for: timeout) + } catch { + return + } + guard !Task.isCancelled, + let self, + self.playbackLoadingGeneration == loadingGeneration, + self.isPlaybackLoading, + self.currentVideo?.videoId == videoId + else { return } + if !self.playbackController.cancelPendingLoad() { + self.deferCurrentVideoReload() + } + } + } + + private func finishPlaybackLoading() { + self.playbackLoadingGeneration &+= 1 + self.isPlaybackLoading = false + self.playbackLoadingTimeoutTask?.cancel() + self.playbackLoadingTimeoutTask = nil + } /// Prepares the inline surface for a switch to the music source: /// pause in place, keep everything loaded for restore. @@ -1355,9 +1413,7 @@ final class YouTubePlayerService { self.stop() } } -} -extension YouTubePlayerService { // MARK: - Bridge Callbacks /// A `STATE_UPDATE` payload from the watch page observer script. @@ -1366,6 +1422,7 @@ extension YouTubePlayerService { let progress: Double let duration: Double var hasReadyMedia = false + var hasMediaError = false var videoId: String? var boundVideoId: String? var title: String? @@ -1431,6 +1488,8 @@ extension YouTubePlayerService { self.reopenConcludedWatchIfNeeded(update, effectiveIsPlaying: effectiveIsPlaying) self.refreshPlaybackMetadataIfNeeded(update, effectiveIsPlaying: effectiveIsPlaying) self.followPageDriftIfNeeded(update) + self.finishPlaybackLoadingIfReady(update) + self.deferMediaErrorIfNeeded(update) self.completeAutoplayTransitionIfNeeded(update) self.recordReadyContentProgressIfPossible(update) @@ -1554,15 +1613,60 @@ extension YouTubePlayerService { if self.isAtLiveEdge != update.isAtLiveEdge { self.isAtLiveEdge = update.isAtLiveEdge } - // Keep the loading spinner up until the video actually has media — the - // first STATE_UPDATE arrives while the