-
Notifications
You must be signed in to change notification settings - Fork 452
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
Fix for using selected_files from previous downloads #8413
Conversation
06f6af9
to
4045f93
Compare
validate |
// Disable for all errors, except metainfo errors (timeouts) | ||
setDisabled(response.error.message !== "metainfo error"); |
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.
This is not enough to revert #8403, it reintroduces #8360 for slow loading sites. For example, entering https://www.wish.com/
and clicking Download
, leads to:
I appreciate your cause of "removing any feedback that something is happening", but this is not sufficient. Please either fix your fix, undo your unfix, or add a spinner or loadbar? I'm O.K. with anything but breaking it again.
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.
OK, I've removed my fix.
4045f93
to
9000c47
Compare
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.
Reflecting on my earlier request for changes:
- We used to have a good magnet link experience and a bad HTTP(S) experience.
- In Fixed exception on bad torrent URL #8403, that flipped around. As you pointed out, the magnet link experience is now bad.
I think the only way out of this cycle, is to make some edits to the core endpoint.
Or just disable the "Download" button while fetching from HTTP(s). An easy fix. |
I'm all for an easy fix. The "add torrent" endpoint does a lot of stuff though, so I think we should be careful about covering our bases. I'm a bit wary of this code now, because my own "easy fix" ignored magnets while improving http. For example, I know the endpoint follows HTTP 300 redirects as well. So, could we get a situation where a magnet link is allowed to be added without metainfo, but a magnet link from a HTTP 300 redirect does have to wait? 🤷♂️ |
I'll leave this one to you then. |
Fixes #8381.
Also fixes:
SelectRemotePath
.An issue with theSaveAs
dialog only being shown after the metadata has been downloaded, removing any feedback that something is happening. Only happened when coming fromAddTorrent
(introduced in Fixed exception on bad torrent URL #8403).