Skip to content
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

Update to jQuery 3.3.1 #206

Closed
coliff opened this issue Dec 13, 2018 · 9 comments
Closed

Update to jQuery 3.3.1 #206

coliff opened this issue Dec 13, 2018 · 9 comments
Labels
enhancement Improvements to existing features

Comments

@coliff
Copy link
Contributor

coliff commented Dec 13, 2018

It'd be good to update to jQuery 3.3.1. jQuery 2.x is no longer supported and has some XSS issues (https://snyk.io/test/npm/jquery/2.2.4)

I've tested this theme with jQuery 3.3.1 and it works correctly.

@matalo33
Copy link
Contributor

Would you be able to submit a PR for this?

If you do, please give a URL to where you obtained jquery from. Preferably a link to a specific version, not just 'latest'. This is so I can validate the file hasn't been modified before merging 👍

@matalo33 matalo33 added enhancement Improvements to existing features help wanted labels Dec 13, 2018
@coliff
Copy link
Contributor Author

coliff commented Dec 13, 2018

Sure thing. I'll do that shortly. I was thinking of suggesting that we load jQuery from a CDN with a local fallback - like HTML5Boilerplate recommends.

It'd be like this:

  <script src="https://code.jquery.com/jquery-3.3.1.min.js" integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8=" crossorigin="anonymous"></script>
  <script>window.jQuery || document.write('<script src="js/jquery-3.3.1.min.js"><\/script>')</script>

REF: https://github.com/h5bp/html5-boilerplate/blob/6.1.0/dist/index.html

What do you think?

@matalo33 matalo33 reopened this Dec 20, 2018
@matalo33
Copy link
Contributor

I like the idea of using CDN but I remember having a conversation about this with @matcornic in the past... he wanted the site to remain functional even in an "internal" or "offline" network.

This option of trying CDN and then falling back could work. We should check whether there's a noticeable delay while an 'offline' client fails or times out reaching the CDN.

Also it would make sense to apply this change to all libraries loaded by the theme which are available via a CDN.

@coliff
Copy link
Contributor Author

coliff commented Dec 21, 2018

The option for using CDN first with a local fallback for jQuery is simple to implement and doesn't impact performance even when running without Internet access. You can test how it works by blocking access to the CDN with Chrome dev tools. There isn't such an option for the other JS scripts to detect if CDN is available if not fallback to locally hosted. I think doing this CDN-first approach just for jQuery is fine.

@matcornic
Copy link
Owner

@coliff @matalo33 yes I wanted the website to work offline. With CDN and fallback to local is great solution for both worlds

@coliff
Copy link
Contributor Author

coliff commented Dec 23, 2018

Great - thanks for confirming @matcornic - I'll open a PR shortly.

@blackmichael
Copy link

@coliff Is there any progress on this?

@coliff
Copy link
Contributor Author

coliff commented Mar 9, 2019

This is dealt with PR #237 which I hope will be merged.

@matalo33
Copy link
Contributor

matalo33 commented Apr 1, 2019

Closed by #237

@matalo33 matalo33 closed this as completed Apr 1, 2019
maxatome pushed a commit to maxatome/hugo-theme-learn that referenced this issue Jan 23, 2024
maxatome pushed a commit to maxatome/hugo-theme-learn that referenced this issue Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features
Projects
None yet
Development

No branches or pull requests

4 participants