Use Firefox profile folder names for browser cookies#89
Conversation
|
Quick re-check against current For Firefox, |
|
I reviewed this more closely. The PR is directionally correct and needed, but I think we should handle two cases before merging:
Overall, the PR is good and narrowly scoped. Separating the display name from the value passed to yt-dlp is the right fix, but these two edge cases would make the behavior more robust for existing users and custom Firefox profiles. |
e4cd62d to
7c19897
Compare
|
Updated this PR against current upstream/main and addressed both review points:\n\n- Firefox profile resolution now happens in the Rust cookie-arg path before yt-dlp receives --cookies-from-browser, so downloads/metadata/channel/polling flows no longer depend on opening Settings first.\n- \profiles.ini\ parsing now handles \IsRelative=0\ by preserving the full profile path, while relative Firefox paths still pass the real profile folder basename.\n\nAlso kept the Settings-side legacy display-name migration so the UI self-heals when users open Network settings. Added targeted Rust tests for relative paths, absolute paths, and legacy display-name resolution. |
|
Thanks for the update. I rechecked the latest version and the two previous concerns are addressed: Firefox profile normalization now happens in the backend cookie-arg path, and The scope looks good to me: the PR keeps the existing UI behavior, separates display name from the value passed to yt-dlp, and adds focused tests for relative paths, absolute paths, and legacy display-name migration. I verified locally with The only blocker I see now is that GitHub reports the PR as conflicting with |
Firefox profiles.ini separates the user-facing Name from the Path that yt-dlp needs. The previous patch fixed the Settings selector, but downloads started before opening Settings could still pass the legacy display name. The profile parser now lives in a shared Rust utility so browser detection and yt-dlp cookie args use the same value, including absolute paths from IsRelative=0 entries. Constraint: Firefox profiles can use Name=default-release with Path=Profiles/<real-folder>. Constraint: yt-dlp accepts a Firefox profile path for profiles outside the default root. Rejected: Frontend-only localStorage migration | download, metadata, channel, and polling flows can run before Settings opens. Confidence: high Scope-risk: narrow Directive: Keep display_name user-facing; only folder_name/cookie value should be passed to yt-dlp. Tested: bun run biome check --write . Tested: bun run tsc -b Tested: cargo test firefox_profile --lib Tested: cargo check Not-tested: Manual Firefox cookie download with an IsRelative=0 profile.
7c19897 to
00a7086
Compare
Summary
profiles.inito the actual profile directory basename used by yt-dlp.Nameas display text only.Why
Firefox
profiles.iniseparatesNamefromPath. yt-dlp browser cookies expect the profile directory name, so selecting a profile likedefault-releasecan fail when the actual folder isi879pxds.default-release.Reproducer
A Firefox profile entry can look like
Name=default-releasewithPath=Profiles/i879pxds.default-release. Passingdefault-releaseto yt-dlp misses the actual profile directory; passingi879pxds.default-releaseworks.Verification
bun run biome check --write src-tauri/src/commands/dependencies.rs src/components/settings/sections/NetworkSection.tsxbun run tsc -bcargo test firefox_profile_uses_the_real_directory_name --libcargo checkinsrc-tauriScreenshots
Not included; this is a value-mapping fix and the existing settings UI remains visually unchanged.