Add check for page's referrer before auto download.

[mloyd]

This prevents someone from clicking a Google search result leading to an
automatic download that likely is not necessary.

[pedrorijo91]

Thanks a lot for helping us solve this long lasting issue :) a few small comments. Still need to see if there's no problems when running this. Will try to test it ASAP

[pedrorijo91]

nitpick: can you extract the if condition into a method with a clear name please? :)

[mloyd]

Maybe I'm missing your point. But it already is in a method named triggerAutoDownload. Well, technically, it was in a method I named autoDownload but then renamed it to the former. If this is not what you meant, please elaborate.

  triggerAutoDownload: function() {
    var ref = document.referrer;
    var src = $('#auto-download-link').attr('href');

    if (ref && src && ref.indexOf(document.location.origin) === 0) {
      $('.downloading .hide').show();
      document.location = src;
    }
  },

[pedrorijo91]

maybe I was not clear, sorry :)

My suggestion was to move the ref && src && ref.indexOf(document.location.origin) === 0 into a method (for instance shouldAutoDownload(ref, src))

[mloyd]

I understand now. Thanks for clarifying. Change made.

[pedrorijo91]

I think this can be achieved with $('.download .hide').show()

[mloyd]

Changed per your instruction.

[pedrorijo91]

this seems weird. Not an js expert, but there's probably a better way to achieve what you are trying to do in a cleaner way. What are you achieving with this? Will try to see if there's another solution

[mloyd]

You're right. I just took the iframe markup and included that in the method. Setting the location is a cleaner way to achieve triggering the download.

[pedrorijo91]

sorry for taking so much time @mloyd

the code LGTM, but I wasn't able to find a way to test it. Can I ask you how did you do it?

[mloyd]

Thanks, @pedrorijo91 and sorry for my late reply.

I tested by temporarily changing the masthead view to include four hard-coded links:

<ol>
  <li><a href="http://127.0.0.1:5000/download/win">http://127.0.0.1:5000/download/win</a></li>
  <li><a href="http://127.0.0.1:5000/download/mac">http://127.0.0.1:5000/download/mac</a></li>
  <li><a href="http://localhost:5000/download/win">http://localhost:5000/download/win</a></li>
  <li><a href="http://localhost:5000/download/mac">http://localhost:5000/download/mac</a></li>
</ol>

Then, after firing up scripts/server and browsing to http://localhost:5000, I should be able to confirm by which link I clicked. Clicking links 1 and 2 should not trigger the download. But clicking links 3 and 4 should trigger the download.

[pedrorijo91]

@peff / @jnavila if any of you wants to give a look just let us know, otherwise I will merge it soon (today or tomorrow probably)