Support .torrent files in Torrent Viewer #7351

feross · 2017-02-22T02:05:08Z

Test Plan

Visit https://codepen.io/ferossity/full/qaezaB/
Click the "torrent" link for any of the given files.
Verify the following:
1. The URL bar shows an http link that ends in ".torrent"
2. The torrent viewer loads
3. Clicking "Save Torrent File" causes a .torrent file to download.
4. Clicking "Start Download" causes the download to start.
5. Clicking "Save Torrent File" still causes a .torrent file to download.
6. Clicking on a video/audio file inside the torrent causes streaming to immediately start.

Also, confirm that magnet links continue to work:

Visit https://codepen.io/ferossity/full/qaezaB/
Click the "magnet" link for any of the given files.
Verify the following:
1. The URL bar shows a magnet link
2. The torrent viewer loads
3. Clicking "Copy Magnet Link" causes the magnet link to be copied to the clipboard.
4. Clicking "Start Download" causes the download to start.
5. Clicking "Copy Magnet Link" still causes the magnet link to be copied to the clipboard.
6. Clicking on a video/audio file inside the torrent causes streaming to immediately start.

Description

Behold, .torrent file support!

Fixes: #6671
Fixes: #7772

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).

feross · 2017-02-22T02:20:13Z

app/filtering.js

@@ -303,7 +303,7 @@ function registerForHeadersReceived (session, partition) {
        continue
      }
      if (results.responseHeaders) {
-        cb({responseHeaders: results.responseHeaders})
+        cb({ responseHeaders: results.responseHeaders, statusLine: results.statusLine })


This doesn't seem to actually redirect the browser to the given Location header. Instead, it loads about-error.html page. Any ideas?

luixxiul · 2017-02-25T08:51:00Z

Does this cover .magnet links as well? The example is: http://download.documentfoundation.org/libreoffice/stable/5.3.0/win/x86/LibreOffice_5.3.0_Win_x86.msi.magnet

feross · 2017-02-28T21:27:56Z

@luixxiul I've never seen a .magnet file before. That's a first for me. They're non-standard and it looks like they're only used by the Vuze torrent client. We could support them, but I think that the libreoffice team was probably expecting users to open the .magnet file and paste the link into a torrent app.

feross · 2017-03-18T00:13:40Z

This PR is ready for review! I have no idea who should review it, so I added a bunch of potential reviewers. Looking forward to finally landing this!

feross · 2017-03-18T00:15:19Z

app/extensions/torrent/webtorrent.html

    <link rel="localization" href="locales/{locale}/app.properties">
  </head>
  <body>
    <div id="appContainer" />
+    <script src='gen/webtorrentPage.entry.js'></script>


This let's us use the DOM node #appContainer right away, without waiting for DOM ready.

feross · 2017-03-18T00:16:33Z

app/browser/webtorrent.js

+ *                         are read.
+ * @return {boolean} True if the resource is a torrent file.
+ */
+function isTorrentFile (details) {


This .torrent file detection is pretty conservative. It looks for a torrent mimetype, and only uses the ".torrent" extension to indicate a torrent if the server sets the 'content-type' to 'application/octet-stream'.

feross · 2017-03-18T00:16:58Z

js/webtorrent/components/app.js

@@ -0,0 +1,35 @@
+const React = require('react')


Pulled this into its own component

feross · 2017-03-18T00:17:31Z

js/webtorrent/components/mediaViewer.js


    let content
-    if (torrent.serverURL == null) {
+    if (!file || !serverUrl) {


Fix race condition where media is briefly added to <iframe>. If there is no file object yet, but there is a serverUrl, then an iframe is added to the page, instead of a "Loading..." message.

feross · 2017-03-18T00:18:57Z

js/webtorrent/components/torrentFileList.js

-      return <a href={magnetURL}>{file.name}</a>
+      const suffix = /^https?:/.test(torrentId)
+        ? '#ix=' + ix
+        : '&ix=' + ix


For http torrent links, we use a hash symbol instead of a query param to indicate the file that is selected, since we don't want to add random query params to the URL, which could cause the server to 404 if it doesn't like it.

For magnet links, adding a query param is acceptable since unknown keys are ignored by all torrent clients.

feross · 2017-03-18T00:19:21Z

js/webtorrent/components/torrentViewer.js

+          l10nId='copyMagnetLink'
+          className='whiteButton copyMagnetLink'
+          onClick={() => dispatch('copyMagnetLink')}
+        />


Magnet links get a "Copy Magnet Link" button, instead of "Save Torrent File" which made no sense.

feross · 2017-03-18T00:20:11Z

js/webtorrent/entry.js

+  store.torrentIdProtocol = parsedUrl.protocol
+
+  // `ix` param can be set by query param or hash, e.g. ?ix=1 or #ix=1
+  if (parsedUrl.protocol === 'http:' || parsedUrl.protocol === 'https:') {


This is where you can see how torrent links and magnet links are handled differently.

feross · 2017-03-18T00:21:04Z

js/webtorrent/entry.js

+  // Clean up the client. Note: Since this does IPC, it's not guaranteed to send
+  // before the page is closed. But that's okay; webtorrent-remote expects regular
+  // heartbeats and assumes clients are dead after 30s without one. So basically,
+  // this is an optimization to destroy the client sooner.


Added a much more descriptive comment here. We're not relying on this IPC succeeding before the page is unloaded. (It only succeeds like 50% of the time.)

feross · 2017-03-18T00:28:04Z

js/webtorrent/entry.js

+  // update() call that happens on a 1 second interval.
+  torrent.on('infohash', update)
+  torrent.on('metadata', update)
+  torrent.on('done', update)


These handlers make the UI snappier

feross · 2017-03-18T00:28:36Z

package.json

@@ -123,7 +124,7 @@
    "tracking-protection": "1.1.x",
    "underscore": "1.8.3",
    "url-loader": "^0.5.7",
-    "webtorrent-remote": "^1.0.0"
+    "webtorrent-remote": "^2.0.0"


Lots of fixes went into the next version of webtorrent-remote to support this PR

feross · 2017-03-18T00:29:18Z

package.json

@@ -87,6 +87,7 @@
    "ad-block": "^2.0.0",
    "aphrodite": "^1.0.0",
    "async": "^2.0.1",
+    "clipboard-copy": "^1.0.0",


Published this to npm since it's reuseable and useful in other contexts. It's simpler than the other copy-to-clipboard solutions out there. Unfortunately, this is not just a simple one-liner.

If there is no file object yet, but there is a serverUrl, then an iframe is added to the page, instead of a "Loading..." message

Fixes: #7772

(All the previous changes assume this version)

Always show torrent title -- even if internationalization code runs after it's set.

feross · 2017-03-21T00:01:22Z

@bsclifton Rebased, and ready for review!

alexwykoff · 2017-03-24T02:26:57Z

This is beautiful beyond words.

Why does the save down-arrow icon link to localhost? What is the expected behavior? (currently does nothing)

Also, could we perhaps show per-file info and have controls to stop/start on contents?
(Likely out of scope, better for another issue&PR)

What do I do if I want to start a new torrent?

Other than these questions ^ it looks great. Well done @feross!

feross · 2017-03-30T22:56:10Z

This is beautiful beyond words.

Thanks :)

Why does the save down-arrow icon link to localhost? What is the expected behavior? (currently does nothing)

We serve the files in the torrent from a local HTTP server. That way when the browser makes a request for a specific file (or a part of a file, in the case of streaming) the torrent engine knows to prioritize downloading that part of the torrent first. It's unfortunate that we're exposing localhost in the link, but I think it's acceptable.

The expected behavior is that clicking it will trigger a file download for just that one file. In some cases, I notice a long delay before that dialog shows up. It's unclear why that's happening. I can look into it for a future PR.

Also, could we perhaps show per-file info and have controls to stop/start on contents?

Per-file info is doable. What do you mean by controls to stop/start on contents?

What do I do if I want to start a new torrent?

The torrent viewer works just like the PDF viewer that's also built into Brave. You start a new torrent by navigating to a magnet link or torrent file URL.

srirambv · 2017-03-31T07:13:04Z

Also, could we perhaps show per-file info and have controls to stop/start on contents?

What do you mean by controls to stop/start on contents?

I think its start/stop/exclude buttons for individual downloads. As there is no option to exclude specific files form the list. This is (not sure if it still there) a feature in utorrent where you can pause/cancel/exclude specific files even after torrent download is started. That would be a nice addition to the existing feature

alexwykoff · 2017-03-31T22:42:10Z

@feross I think there are a few things which probably should be managed before we push this out the door.

Support local .torrent files (pdf.js has the same problem)
Allow users to stop a torrent (or seeding)

For future consideration:

For compilations, the ability to selectively download
Seed ratios
Bandwidth rate capping
Creating and hosting new torrents based on local files
Tracking multiple torrents on the same page

I'm sure I'm missing some obvious things here, but as a first push, this is great. If we get the first two issues resolved, this is good to go.
Network recovery tests totally worked, resume after quit Brave worked.

Tested on OS X and Windows. Linux test underway.

bsclifton

@alexwykoff has done a comprehensive review; I reviewed the code and also tried the PR- works great 😄 Will go ahead and merge and we can address any opens items with a follow up issue

feross · 2017-04-13T03:00:28Z

@alexwykoff I opened an issue for your request:

Support local .torrent files Support local .torrent files #8272
Stopping a download is already possible by closing the tab or clicking the newly-added Stop button (Torrent viewer: Add 'Stop Download' button #7863)

Your other suggestions are interesting. My thoughts are inline.

For compilations, the ability to selectively download

Useful, but definitely a power user feature. We already support this feature in WebTorrent Desktop, though I believe that are some bugs in the implementation right now and extra files end up getting downloaded.

FWIW, files are already prioritized based on need. So, media that is being consumed via the built-in video player, or that is being downloaded via the 'save file' button will be prioritized before the rest of the files in the torrent.

So the primary benefit of this feature would be saving bandwidth/disk space by not downloading files you know you don't need.

Seed ratios

Does an average user know what a seed ratio is? WebTorrent Desktop doesn't even include a seed ratio and it hasn't generated too many complaints.

Bandwidth rate capping

Definitely useful in some situations, but would this be a client-wide cap? Does that mean we need a page for configuring the torrent viewer extension?

Creating and hosting new torrents based on local files

Does the PDF viewer support creating PDF files? I think this is out of scope.

Tracking multiple torrents on the same page

I know that most existing torrent apps show all the torrents in one big table. But I like the current model where torrents are treated just like any other webpage. One torrent per tab.

Thanks for the review and for landing this, @alexwykoff @bsclifton!

feross commented Feb 22, 2017

View reviewed changes

luixxiul added PR/work-in-progress ⚒ feature/torrent labels Feb 22, 2017

feross force-pushed the feross/torrent-files branch from 100d6d3 to 7ae3d03 Compare March 17, 2017 01:02

feross mentioned this pull request Mar 17, 2017

Fixed torrent viewer CSP rule blocks <iframe> content #7243

Closed

feross changed the title ~~WIP: Show .torrent files in Torrent Viewer~~ Show .torrent files in Torrent Viewer Mar 17, 2017

feross changed the title ~~Show .torrent files in Torrent Viewer~~ Support .torrent files in Torrent Viewer Mar 18, 2017

feross requested review from bbondy, bsclifton, luixxiul, srirambv and bridiver March 18, 2017 00:12

feross commented Mar 18, 2017

View reviewed changes

js/webtorrent/components/app.js

@@ -0,0 +1,35 @@

const React = require('react')

Copy link

Contributor Author

feross Mar 18, 2017

Choose a reason for hiding this comment

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

Pulled this into its own component

feross commented Mar 18, 2017

View reviewed changes

feross added PR/needs-QA-attention ☕ and removed PR/work-in-progress ⚒ labels Mar 18, 2017

feross mentioned this pull request Mar 18, 2017

Updated "Save Torrent File" button to "Copy Magnet Link" for magnet links #7772

Closed

feross added this to the 0.14.1 milestone Mar 18, 2017

feross added 10 commits March 20, 2017 16:57

Refactor: prep for next version of webtorrent-remote

9b5061d

Fix race condition where media is briefly added to <iframe>

2fefbf2

If there is no file object yet, but there is a serverUrl, then an iframe is added to the page, instead of a "Loading..." message

Add support for .torrent files

ef21ddc

Make renders happen more quickly (initial and important events)

b81798f

Fix bugs in ix query parsing

fad4acf

Change "Save Torrent File" button to "Copy Magnet Link" for magnet links

e770256

Fixes: #7772

Update webtorrent-remote to v2

6df2507

(All the previous changes assume this version)

only update document.title when it changes

fabc31b

Better title handling with internationalization code

b790137

Always show torrent title -- even if internationalization code runs after it's set.

standard

9c44d03

feross force-pushed the feross/torrent-files branch from bd2d0c9 to 9c44d03 Compare March 21, 2017 00:00

simplify initTorrent()

a03d8c0

feross mentioned this pull request Mar 23, 2017

Torrent viewer: Add 'Stop Download' button #7863

Merged

4 tasks

bsclifton requested a review from alexwykoff March 30, 2017 15:28

luixxiul removed their request for review April 1, 2017 17:44

bsclifton approved these changes Apr 3, 2017

View reviewed changes

bsclifton merged commit ef59f09 into master Apr 3, 2017

bsclifton deleted the feross/torrent-files branch April 3, 2017 01:16

bsclifton added release-notes/include QA/test-plan-specified and removed PR/needs-QA-attention ☕ labels Apr 3, 2017

luixxiul mentioned this pull request Apr 4, 2017

Added support for torrent link files #6671

Closed

feross mentioned this pull request Apr 13, 2017

Support local .torrent files #8272

Open

NejcZdovc mentioned this pull request Apr 23, 2017

Fixed download of torrent files #8452

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support .torrent files in Torrent Viewer #7351

Support .torrent files in Torrent Viewer #7351

feross commented Feb 22, 2017 •

edited by bsclifton

Loading

feross Feb 22, 2017

luixxiul commented Feb 25, 2017

feross commented Feb 28, 2017

feross commented Mar 18, 2017

feross Mar 18, 2017

feross Mar 18, 2017

feross Mar 18, 2017

feross Mar 18, 2017

feross Mar 18, 2017 •

edited

Loading

feross Mar 18, 2017

feross Mar 18, 2017 •

edited

Loading

feross Mar 18, 2017

feross Mar 18, 2017

feross Mar 18, 2017

feross Mar 18, 2017 •

edited

Loading

feross commented Mar 21, 2017

alexwykoff commented Mar 24, 2017

feross commented Mar 30, 2017

srirambv commented Mar 31, 2017

alexwykoff commented Mar 31, 2017

bsclifton left a comment

feross commented Apr 13, 2017

Support .torrent files in Torrent Viewer #7351

Support .torrent files in Torrent Viewer #7351

Conversation

feross commented Feb 22, 2017 • edited by bsclifton Loading

Test Plan

Description

Choose a reason for hiding this comment

luixxiul commented Feb 25, 2017

feross commented Feb 28, 2017

feross commented Mar 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feross Mar 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feross Mar 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feross Mar 18, 2017 • edited Loading

Choose a reason for hiding this comment

feross commented Mar 21, 2017

alexwykoff commented Mar 24, 2017

feross commented Mar 30, 2017

srirambv commented Mar 31, 2017

alexwykoff commented Mar 31, 2017

bsclifton left a comment

Choose a reason for hiding this comment

feross commented Apr 13, 2017

feross commented Feb 22, 2017 •

edited by bsclifton

Loading

feross Mar 18, 2017 •

edited

Loading

feross Mar 18, 2017 •

edited

Loading

feross Mar 18, 2017 •

edited

Loading