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

Various Updates #237

Merged
merged 5 commits into from
Apr 1, 2019
Merged

Various Updates #237

merged 5 commits into from
Apr 1, 2019

Conversation

ozobi
Copy link
Contributor

@ozobi ozobi commented Feb 7, 2019

@matalo33 matalo33 added the enhancement Improvements to existing features label Feb 7, 2019
@coliff coliff mentioned this pull request Mar 9, 2019
@blackmichael
Copy link

@matcornic is there any plan to merge this soon? Or provide feedback for changes? I believe this jQuery upgrade will resolve an XSS concern I currently have with my hugo site.

layouts/partials/header.html Outdated Show resolved Hide resolved
static/css/theme.css Show resolved Hide resolved
static/js/search.js Show resolved Hide resolved
static/js/search.js Show resolved Hide resolved
@matalo33 matalo33 added this to the v2.3.0 milestone Mar 15, 2019
@matalo33 matalo33 self-assigned this Mar 15, 2019
fixed indent
fixed indent
@coliff
Copy link
Contributor

coliff commented Apr 1, 2019

This is a great PR. I hope to see this merged soon.
FYI; Lunr 2.3.6 is available now if you want to update that in this PR (https://github.com/olivernn/lunr.js/blob/master/CHANGELOG.md)

@matalo33 matalo33 merged commit 18212e6 into matcornic:master Apr 1, 2019
@matalo33
Copy link
Contributor

matalo33 commented Apr 1, 2019

This was held up because I wanted to quickly validate checksums of the javascript, however this has proven much more difficult than expected because a couple of the files come from dynamic generators. Here's what I found:

  • modernizer - md5 differs and LARGE amount of changes ⛔️
  • highlight - md5 differs and LAEGE amount of changes ⛔️
  • lunr - md5 mismatch, but diff is trivial 🆗
  • clipboard.min.js - md5 match 🆗
  • jquery-3.3.1.min.js - md5 match 🆗
  • featherlight.min.js - md5 match 🆗

We don't really have a method for validating these files so I'll continue and merge for now. Maybe in the future dynamically generated JS shouldn't get supplied in PRs?

On the other hand, if only maintainers could update dynamic JS files why should anyone trust that we haven't tampered with the JS? The end user should be able to md5 validate the files against upstream. Will give this some thought.

In the meantime, thanks again for your extensive contribution! Apologies it took so long to merge.

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

Successfully merging this pull request may close these issues.

4 participants