-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: handle page restoration from bfcache in Safari #7683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
robwalch
merged 3 commits into
video-dev:master
from
christriants:fix/reattach-media-pageshow
Jan 16, 2026
+31
−0
Merged
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,6 +120,10 @@ export default class BufferController extends Logger implements ComponentAPI { | |
| [null, null], | ||
| [null, null], | ||
| ]; | ||
| // Handler for Safari/WebKit MediaSource workaround | ||
| private safariSourceCloseHandler: (() => void) | null = null; | ||
| // Flag indicating whether this browser needs MediaSource close recovery from bfcache (Safari/WebKit) | ||
| private needsMediaSourceCloseRecovery: boolean = false; | ||
|
|
||
| constructor(hls: Hls, fragmentTracker: FragmentTracker) { | ||
| super('buffer-controller', hls.logger); | ||
|
|
@@ -138,6 +142,15 @@ export default class BufferController extends Logger implements ComponentAPI { | |
| } | ||
|
|
||
| public destroy() { | ||
| // Clean up Safari/WebKit workaround listener before destroying | ||
| if (this.mediaSource && this.safariSourceCloseHandler) { | ||
| this.mediaSource.removeEventListener( | ||
| 'sourceclose', | ||
| this.safariSourceCloseHandler, | ||
| ); | ||
| this.safariSourceCloseHandler = null; | ||
| } | ||
|
|
||
| this.unregisterListeners(); | ||
| this.details = null; | ||
| this.lastMpegAudioChunk = this.blockedAudioAppend = null; | ||
|
|
@@ -279,6 +292,7 @@ export default class BufferController extends Logger implements ComponentAPI { | |
| data: MediaAttachingData, | ||
| ) { | ||
| const media = (this.media = data.media); | ||
| this.needsMediaSourceCloseRecovery = this.detectMediaSourceCloseIssue(); | ||
| this.transferData = this.overrides = undefined; | ||
| const MediaSource = getMediaSource(this.appendSource); | ||
| if (MediaSource) { | ||
|
|
@@ -329,6 +343,12 @@ export default class BufferController extends Logger implements ComponentAPI { | |
| ms.addEventListener('startstreaming', this._onStartStreaming); | ||
| ms.addEventListener('endstreaming', this._onEndStreaming); | ||
| } | ||
| // Safari/WebKit workaround: Add additional listener for recovery | ||
| if (this.needsMediaSourceCloseRecovery) { | ||
| const handler = this._handleSafariMediaSourceClose.bind(this); | ||
| this.safariSourceCloseHandler = handler; | ||
|
||
| ms.addEventListener('sourceclose', handler); | ||
| } | ||
| } | ||
|
|
||
| private attachTransferred() { | ||
|
|
@@ -516,6 +536,14 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe | |
| mediaSource.removeEventListener('endstreaming', this._onEndStreaming); | ||
| } | ||
|
|
||
| // Remove Safari/WebKit workaround listener if present | ||
| if (this.safariSourceCloseHandler) { | ||
| mediaSource.removeEventListener( | ||
| 'sourceclose', | ||
| this.safariSourceCloseHandler, | ||
| ); | ||
| this.safariSourceCloseHandler = null; | ||
| } | ||
| this.mediaSource = null; | ||
| this._objectUrl = null; | ||
| } | ||
|
|
@@ -1935,6 +1963,31 @@ transfer tracks: ${stringify(transferredTracks, (key, value) => (key === 'initSe | |
| }); | ||
| track.listeners.length = 0; | ||
| } | ||
|
|
||
| private detectMediaSourceCloseIssue(): boolean { | ||
| const ua = navigator.userAgent.toLowerCase(); | ||
| const isSafari = /safari/.test(ua) && !/chrome/.test(ua); | ||
| const isWebKit = /webkit/.test(ua); | ||
| return isSafari || isWebKit; | ||
| } | ||
|
|
||
| /** | ||
| * Handles MediaSource 'sourceclose' event on Safari/WebKit. | ||
| * Safari/WebKit have a bug where MediaSource becomes invalid after bfcache restoration. | ||
| * When the user navigates back, the MediaSource is in a 'closed' state and cannot be used. | ||
| * The 'sourceclose' event fires to notify of this, but by then sourceBuffers are already | ||
| * cleaned up (length = 0). | ||
| * If sourceclose fires while media is still attached, we trigger recovery via | ||
| * recoverMediaError() to reattach media. | ||
| */ | ||
| private _handleSafariMediaSourceClose = () => { | ||
| const { media, mediaSource } = this; | ||
| if (!media || !mediaSource) { | ||
| return; | ||
| } | ||
| this.warn('MediaSource closed while media attached - triggering recovery'); | ||
| this.hls.recoverMediaError(); | ||
| }; | ||
| } | ||
|
|
||
| function removeSourceChildren(node: HTMLElement) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't mean to suggest that this should be behind a user-agent check. I'd prefer to use existing listeners, or only add new event listeners (like "pageshow") as needed to detect that the source closure resulted from backgrounding the page.
We already have a listener for 'sourceclose':
_onMediaSourceClose>video.error, hls error, or loading state?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sorry for the misunderstanding! I think we have two solid options here.
sourcecloselistener /_onMediaSourceClosehandler. On Safari, the MediaSource is closed when returning to the page via bfcache, so thesourcecloseevent fires immediately when we land back on the page. When this occurs, there's also a media error ("Load was aborted"), but I don't think that error is specific to this issue.If media is still attached when
sourceclosefires, we can trigger recovery. My only concern is understanding all scenarios wheresourceclosemay fire. Currently, the event also fires during media detachment, but we don't catch it because we remove the listener before the event is captured.Something like this works:
My concern is: what happens if we start catching the
sourcecloseevent during media detachment in the future? I think we would try to trigger a recovery? But then again,this.mediawould benull, so the condition here probably protects us from this.pageshowlistener from earlier to specifically account for bfcache restoration:This is more explicit about handling bfcache restoration. On other browsers, the MediaSource
readyStateremainsopenwhen restoring from bfcache, so this check would prevent unnecessary recovery attempts. However, we would be adding a global window listener to all browsers and instances.I pushed up a change for option 1, but happy to continue discussing.