Youtube extras - #450
Open
azfoo wants to merge 9 commits into
Open
Conversation
azfoo
force-pushed
the
youtube-extras
branch
4 times, most recently
from
April 27, 2026 17:11
493f60f to
a2a7a14
Compare
Collaborator
|
Hey, is this meant for v0.6.0? |
Collaborator
Author
|
In theory, yes. |
Collaborator
|
Would it be too bad if we left it for v0.6.1? I think we have enough on our plates if we are releasing tomorrow, especially because I am calling it a day. |
Collaborator
Author
FelipeDefensor
requested changes
May 14, 2026
Collaborator
There was a problem hiding this comment.
Some issues with this:
- Fullscreen doesn't work at all for me (maybe we talked about this and I forgot, sorry).
- Autoplay only works if I clicked play for a previous video in the same TiLiA session.
- icecream is not listed as a dependency, if I remember correcly, so
ichas a potential to crash. Replace with another logging mechnism. - If I try to load a well-formed Youtube URL with an invalid video ID, the video is not loaded, but I get no error message at all.
Also, I got this review comment from Claude, it seems to make sense, but I haven't been able to test it.
- display_error semantics changed for all callers. It's reached from PlayerTracker.on_error (line 62), which forwards every YT JS onError (invalid video ID, embed disabled by uploader, region-restricted,
etc.). A reload won't recover from any of those — but the user now gets an unsolicited "Would you like to try and refresh the page?" prompt on every JS error. Consider gating the refresh prompt on
recoverable errors only, or at minimum keep the error-surface call (so the failure still flows through tilia.errors) and offer reload as a secondary step.
closes #451
shows only videos related to the video's uploader, rather than videos from the user's watch history (no way to hide related videos/youtube branding etc. due to yt's policies) .
- youtube.html: fire backend.on_error when the IFrame API script fails
to load (no connection / DNS failure); tryNotify loop retries until
backend is initialized to avoid a race condition
- youtube.html: fix default onError branch — was using a regular string
so ${event.data} was never interpolated; changed to template literal
on_audio_time_change previously accessed self.svg_view, which falls back to creating a new SvgViewer when Get.SCORE_VIEWER has no replier. During test teardown this caused a crash because get_data is called on a partially-destroyed object. Guard the method to skip scrolling when no viewer is open instead.
Collaborator
|
Is this ready for review again? Is this for 0.7? |
Collaborator
Author
|
yes I think so |
FelipeDefensor
added a commit
that referenced
this pull request
Aug 27, 2026
Three corrections found while building a bundle for PR #450: - All paths handed to TiLiA must be absolute. dirs.setup_dirs chdirs into the tilia package outside ENVIRONMENT=prod, and boot() parses its arguments before that chdir -- so `tilia repro/x.tla` passes argparse validation and then fails to open, and a CLI `save repro/x.tla` writes inside tilia/ silently. - The launch command has to go through the project environment. A bare `tilia` resolves to nothing unless .venv is active; use `uv run tilia "$PWD/repro/<issue>.tla"` from the repo root, which is both absolute and portable. - `gh pr checkout <N>` does not belong in a copy-paste launch line. It aborts when the head branch is checked out in another worktree, its fast-forward fails once the local branch has diverged, and --force discards the reviewer's local edits. It is one-time setup plus a warning, not part of the repeatable command.
This was referenced Aug 27, 2026
The script-load failure handler reported through on_error, the path that shows a message and stops there. A connection failure is the one thing a page reload actually repairs, so the user was left with a dead player and no route back short of reopening the file. Route it to on_recoverable_error instead, which surfaces the same message and then offers the refresh. Verified reachable: reloading re-runs the script tag, and the loadFinished connection made during the first load re-issues loadVideo for the current video, so playback resumes once the connection is back. Error code 5 keeps its existing routing.
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
2 participants
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.
No description provided.