Skip to content

Conversation

@emmceemoore
Copy link
Contributor

These reflect the newest versions shown on the Bootstrap website.

I suspect folks would prefer to be on newer versions of these libraries. However, I can see how you might prefer to have users take these updates via settings updates instead.

Updated links were all sourced from here. The cdnjs website provides the SHA values using the 512 algorithm (instead of 384) which is maybe better?

Instead of the URLs coming from different CDNs they now all use Cloudflare (which never goes down, right? 😛).

TESTING:

  • I tried the template tags out with these changes and didn't notice any errors in the inspector.

@dyve
Copy link
Member

dyve commented Jul 18, 2020

Looking good, but we've always used the CDN's that are shown in the Introduction on getbootstrap.com (https://getbootstrap.com/docs/4.5/getting-started/introduction/). I see no reason to change that now.

Using the same links as the Introduction keeps us connected to the Bootstrap examples, and prevent discussion over which CDN is best.

@emmceemoore
Copy link
Contributor Author

Looking good, but we've always used the CDN's that are shown in the Introduction on getbootstrap.com (https://getbootstrap.com/docs/4.5/getting-started/introduction/). I see no reason to change that now.

Using the same links as the Introduction keeps us connected to the Bootstrap examples, and prevent discussion over which CDN is best.

Makes sense. I've updated the values to match what they show in the introduction docs.

It may be worth noting that I did not see a URL for the non-slim version of JQuery in the examples. However, I dropped the .slim bit from the slim version and computed the "integrity" value with an online SRI generator. (The value checks out in my browser.)

I also updated the corresponding test cases which now pass locally on my machine.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.046% when pulling db8b200 on emmceemoore:patch-2 into 609cea6 on zostera:main.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.046% when pulling db8b200 on emmceemoore:patch-2 into 609cea6 on zostera:main.

@dyve
Copy link
Member

dyve commented Jul 20, 2020

Thanks. I agree that the non-slim version is a bit of a pain.

The "official" versions of jQuery (with integrity values) can be found on https://code.jquery.com/jquery/. I used those in the past for the non-slim version if I remember correctly. For 3.3.1 that would be

<script src="https://code.jquery.com/jquery-3.3.1.min.js" integrity="sha256-FgpCb/KJQlLNfOu91ta32o/NMZxltwRo8QtmkMRdAu8=" crossorigin="anonymous"></script>

@emmceemoore
Copy link
Contributor Author

Good to know! Inspecting the page source I was able to see the integrity values.

It's interesting to see that the values use the SHA 256 algorithm. The code currently on the main branch has a "sha384" value.

@dyve
Copy link
Member

dyve commented Jul 20, 2020

You don't need to view the source code, if you click one of the versions you get a link with the security hash.
Schermafbeelding 2020-07-20 om 08 11 29

@dyve
Copy link
Member

dyve commented Jul 20, 2020

I may have used another CDN for jQuery before. I think Bootstrap also switched its preferred CDN's at one point. For now, let's stay as close as possible to what getbootstrap.com suggests.

@emmceemoore
Copy link
Contributor Author

You don't need to view the source code, if you click one of the versions you get a link with the security hash.

My bad! I looked at the first section of links initially and didn't get such a modal. I didn't realize the other ones might have this sort of modal. I ended up copying the "link address" and pasting into another system to get the "sha384" value. 😆

image

For now, let's stay as close as possible to what getbootstrap.com suggests.

I believe my proposed changes achieve this. If there's something I'm missing, please let me know. Thanks

image

@dyve
Copy link
Member

dyve commented Aug 2, 2020

Thanks!

@dyve dyve merged commit f1a0805 into zostera:main Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants