Make JavaScript work with LibreJS #1213

jonah · 2019-08-24T17:10:02Z

dawidpotocki commented

2019-08-24 17:10:02 +00:00

(Migrated from github.com)

Works. LibreJS is happy.

- [x] Works. LibreJS is happy.

Vincevrp (Migrated from github.com) reviewed 2019-08-24 17:10:02 +00:00

netlify[bot] commented

2019-08-24 17:10:57 +00:00

(Migrated from github.com)

Deploy preview for privacytools-io ready!

Built with commit 2a478ded0f

https://deploy-preview-1213--privacytools-io.netlify.com

Deploy preview for *privacytools-io* ready! Built with commit 2a478ded0ffab00899a0b3b6516143d57720ee73 https://deploy-preview-1213--privacytools-io.netlify.com

nitrohorse (Migrated from github.com) reviewed 2019-08-24 17:27:21 +00:00

assets/js/main.js

						
				@ -34,0 +44,4 @@

				  g.defer = true;

				  g.src = u + "matomo.js";

				  s.parentNode.insertBefore(g, s);

				})();

nitrohorse (Migrated from github.com) commented

2019-08-24 17:27:21 +00:00

Doesn't matter too much but could we make this a bit more readable?

// Matomo
var _paq = window._paq || [];
// tracker methods like "setCustomDimension" should be called before "trackPageView"
_paq.push(['trackPageView']);
_paq.push(['enableLinkTracking']);
(function() {
  var u = 'https://stats.privacytools.io/';
  _paq.push(['setTrackerUrl', u + 'matomo.php']);
  _paq.push(['setSiteId', '1']);
  var d = document;
  var g = d.createElement('script');
  var s = d.getElementsByTagName('script')[0];
  g.type = 'text/javascript';
  g.async = true;
  g.defer = true;
  g.src = u + 'matomo.js';
  s.parentNode.insertBefore(g, s);
})();

Doesn't matter too much but could we make this a bit more readable? ```javascript // Matomo var _paq = window._paq || []; // tracker methods like "setCustomDimension" should be called before "trackPageView" _paq.push(['trackPageView']); _paq.push(['enableLinkTracking']); (function() { var u = 'https://stats.privacytools.io/'; _paq.push(['setTrackerUrl', u + 'matomo.php']); _paq.push(['setSiteId', '1']); var d = document; var g = d.createElement('script'); var s = d.getElementsByTagName('script')[0]; g.type = 'text/javascript'; g.async = true; g.defer = true; g.src = u + 'matomo.js'; s.parentNode.insertBefore(g, s); })(); ```

nitrohorse (Migrated from github.com) reviewed 2019-08-24 17:30:49 +00:00

_includes/footer.html

						
				@ -77,0 +77,4 @@

				    </a>

				    <p class="mt-2">

				      <strong>No Ads, No Google Analytics, No Affiliates, No Cross-Site Requests.</strong>

				      <em>privacytools.io is a socially motivated website that provides information for protecting your data security and privacy. Never trust any company with your privacy, always encrypt.</em>

nitrohorse (Migrated from github.com) commented

2019-08-24 17:30:49 +00:00

Could we add a slight space here? <br />

Could we add a slight space here? `<br />`

nitrohorse (Migrated from github.com) reviewed 2019-08-24 17:31:21 +00:00

about/javascript/index.html

nitrohorse (Migrated from github.com) commented

2019-08-24 17:31:21 +00:00

I'm thinking for the creativecommons links we could use https://

I'm thinking for the creativecommons links we could use `https://`

nitrohorse commented

2019-08-24 17:31:46 +00:00

(Migrated from github.com)

Super cool!

nitrohorse (Migrated from github.com) reviewed 2019-08-24 17:47:59 +00:00

nitrohorse (Migrated from github.com) left a comment

LGTM!

nitrohorse (Migrated from github.com) approved these changes 2019-08-24 19:00:29 +00:00

nitrohorse (Migrated from github.com) left a comment

LGTM

Mikaela (Migrated from github.com) approved these changes 2019-08-24 20:33:14 +00:00

Mikaela (Migrated from github.com) left a comment

LGTM, I guess, but I would like to have @JonahAragon to check it

jonah approved these changes 2019-08-25 03:44:49 +00:00

jonah left a comment

Uhh... YOLO, right? I don't see why this shouldn't work.

jonah commented

2019-08-25 03:46:14 +00:00

Have you removed all the inline JS in this PR? We could probably update the CSP too.

Have you removed all the inline JS in this PR? We could probably update [the CSP](https://github.com/privacytoolsIO/privacytools.io/blob/master/nginx/010-headers.conf#L4) too.

dawidpotocki commented

2019-08-25 03:52:26 +00:00

(Migrated from github.com)

Have you removed all the inline JS in this PR? We could probably update the CSP too.

Well… I only found this in scripts.html and LibreJS does not inform me
about other stuff. So probably, yes.

> Have you removed all the inline JS in this PR? We could probably update [the CSP](https://github.com/privacytoolsIO/privacytools.io/blob/master/nginx/010-headers.conf#L4) too. Well… I only found this in `scripts.html` and LibreJS does not inform me about other stuff. So probably, yes.

This repo is archived. You cannot comment on pull requests.

No reviewers

Vincevrp

nitrohorse

Mikaela