fix: reverse playlist persists across video navigation (fixes #3759, #3818, #3883)#3946
Open
jiangyj545 wants to merge 1 commit into
Open
fix: reverse playlist persists across video navigation (fixes #3759, #3818, #3883)#3946jiangyj545 wants to merge 1 commit into
jiangyj545 wants to merge 1 commit into
Conversation
Collaborator
|
Try the latest release first? #3643 should've already fixed this |
wahajahmed010
left a comment
There was a problem hiding this comment.
Review: Reverse playlist SPA fix
Potential issues:
-
Stale setTimeout pileup —
setTimeouthas no cleanup. If the user navigates between videos quickly, multiple 1.5s delays stack up. TheImprovedTube.playlistReversed === trueguard helps but stale callbacks from earlier navigations could run when the user is on a different video. Consider storing & clearing the timeout ID. -
#3643 overlap — Hyperspeed1313 noted #3643 may already fix this. PR references 3 issues — worth clarifying if they persist after #3643.
-
Magic number — 1500ms works empirically but could be a named constant with rationale.
What looks good:
- Minimal single-file change
- Guard prevents double-reverse
- Real SPA race condition addressed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
(running playlistReverseUpdate() twice with 1.5s delay)
Fix for #3759, #3818, #3883 — Reverse playlist reverts on video navigation
### Problem When "Reverse Playlist" is active and the user navigates to the next video in the playlist, YouTube's SPA navigation triggers `yt-page-data-updated`. The extension calls `playlistReverseUpdate()`, but at this point `ytd_watch.data` may not be populated yet, so the function returns early without applying the reverse. The playlist then renders in its original order.- Only retries when
- Does not add an interval or loop — just one additional attempt
- The existing
File
Change
Added 1.5s delayed retry of
- 66/69 unit tests pass (3 pre-existing failures unrelated to this change)
- JS-only change, no CSS or HTML modifications
Solution
Add a delayed retry (1.5s) in
playlistReverse(). After the initialplaylistReverseUpdate()call, asetTimeoutfires once more to catch the case where the first attempt returned early because playlist data wasn't ready.This is minimal and safe:
playlistReversedis stilltrueisCurrentlyReversed === shouldBeReversedguard inplaylistReverseUpdate()prevents double-reversing if the first call succeededChanges
playlist.jsplaylistReverseUpdate()inplaylistReverse()Testing