Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fixes saving a .torrent file (follow up of #8246) #8446

Merged
merged 1 commit into from
Apr 23, 2017

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Apr 23, 2017

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

NOTE:
There was some rebase conflict and a.href was duplicated. This PR removes it.

Resolves #8146

Auditors: @bsclifton @bbondy

Test Plan:

Resolves brave#8146

Auditors: @bsclifton @bbondy

Test Plan:
- Visit https://webtorrent.io/free-torrents/
- Click "Big Buck Bunny (torrent file)"
- Click "Save Torrent File..."
- File is saved
@NejcZdovc NejcZdovc added this to the 0.15.0 milestone Apr 23, 2017
@NejcZdovc NejcZdovc requested review from bbondy and bsclifton April 23, 2017 06:37
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this out; works great 😄 ++

@bsclifton bsclifton merged commit db888a5 into brave:master Apr 23, 2017
@NejcZdovc NejcZdovc self-assigned this Apr 23, 2017
@feross
Copy link
Contributor

feross commented Apr 29, 2017

Why ok instead of true? Seems like an odd choice. I believe true is more standard. Just noticed that @diracdeltas accidentally changed ok to true and this stopped working.

Here: https://github.com/brave/browser-laptop/pull/8325/files#diff-edbab0c194a1bdf5a47e7c89d5f74ef3R86

My bad for not catching this before it was merged. Will send a PR to fix.

feross added a commit that referenced this pull request Apr 29, 2017
This feature broke again after PR #8325. My fault for not catching it
during review. This PR fixes the code that is now on `master`.

Addresses these comments:

-
#8446 (comment)
-
#8146 (comment)
52

It is better to add the '?download=true' querystring param with
`url.parse` and `url.format` since blindly tacking on a
'?download=true' can lead to invalid URLs.

A couple examples:

magnet:?a=b&c=d?download=true should use &download=true instead
https://example.com/file.torrent#ix=1?download=true should not add a
query param after a hash

Also, this names the downloaded file correctly, which didn't seem to be
working either after PR #8325.
@feross feross mentioned this pull request Apr 29, 2017
4 tasks
@feross
Copy link
Contributor

feross commented Apr 29, 2017

PR sent: #8567

@diracdeltas
Copy link
Member

Just noticed that @diracdeltas accidentally changed ok to true and this stopped working.

Oops, my bad! I missed that while rebasing.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants