Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Small Miscellaneous Backend Changes #1009

Merged
merged 6 commits into from Aug 23, 2015

Conversation

Projects
None yet
2 participants
Contributor

harding commented Aug 18, 2015

This PR contains five commits that each make a small change to the way the site is built. There are no changes that affect text or layout.

1: Treat SVG files as binaries when diffing

This commit adds a .gitattributes file that treats SVG files as binaries rather than text files. I don't think this will affect GitHub previews, but I've used it myself and found it makes reviewing diffs much easier. You can see the difference by running the following commands on the master branch and this branch (commit 42b10b1 was the most recent time we changed any SVG files):

git diff 42b10b1^..42b10b1
git diff --stat 42b10b1^..42b10b1

While implementing this, I remembered that SVG files can contain Javascript code---which is a reason for actually treating them like text. However, I've never read SVG diffs before and I don't whether anyone is ever going to read the raw SVGs I create, so this commit also includes a Makefile rule that terminates builds if any SVG contains <script.

I don't know how secure that is, but it's better than what existed before.

2: Add JQuery UI images

When I added JQuery UI to the site for the glossary search box, I neglected to add the icons that came with it. This had no effect on the search box which doesn't use the icons, but it does affect some newer elements I'd like to add on future pages.

This commit adds the icons and modifies the JQuery UI CSS rules to point to the directory containing them. I've checked and the icons are only loaded when used, so except for a few extra bytes in the CSS file, this shouldn't affect page load times.

3: {% comment %} out HTML comments

The _include/references.md file contains HTML comments that get loaded onto every page that uses it---roughly 100 pages currently. That's silly since the rest of the content in this file never gets rendered at all, so it leaves a bunch of random-seeming comments in the middle of the HTML output.

This commit wraps those HTML comments in Liquid comments so they don't get rendered.

This also restores a link to the secg.org website that we had to temporarily replace last year when the site went down.

4: Ensure all pages in en/ have "lang: en" set

The site templates make extensive use of the the {{page.lang}} variable, so if this variable isn't set, lots of things break. For example, the link href="/{{page.lang}}/download" becomes href="//download".

HTML Proofer sees the link above as an off-site link, so it doesn't check it and no error output is produced.

Since the only place (currently) that a page can be created without a language being set is the en/ hierarchy, this commit adds a Makefile rule that requires every page in that directory declare lang: en. The four RSS files in that hierarchy don't need the language to be set, but I added it anyway to keep the rule simple.

5: Add an end_of_page hook for Javascript

When adding custom JS to a page or calling certain functions, it's best that they run after all of the HTML and CSS for the page has been loaded so that users can start using the page while they the JS is loaded or run.

Right now, we load some JS at the end of the Content section, before the Footer section is loaded. This adds a hook called from the YAML header that allows placing the JS at the very end of the page. Several pages in the dev docs section are modified to use this hook.

Contributor

harding commented Aug 18, 2015

Opps, forgot:

@saivann saivann and 1 other commented on an outdated diff Aug 19, 2015

@@ -264,3 +266,11 @@ check-for-subheading-anchors:
$S grep -r -i --include \*.html -L 'Note: this file exempt from check-for-subheading-anchors check' _site/ \
| xargs grep '<h[23456]' \
| grep -v '<h[23456][^>]* id=' | eval $(ERROR_ON_OUTPUT)
+
+check-for-javascript-in-svgs:
+## Security check: don't allow any SVGs that contain Javascript.
+ $S find _site/ -name '*.svg' | xargs grep '<script' | eval $(ERROR_ON_OUTPUT)
+
+check-for-english-in-en-dir:
+## All pages must have page.lang set to work properly with the site templates
+ $S grep -rl -- '---' en/ | xargs grep -L '^[^#]*lang: en' | eval $(ERROR_ON_OUTPUT)
@saivann

saivann Aug 19, 2015

Contributor

@harding How about a more sensitive match?

$S grep -rl -- '---' en/ | xargs grep -L '^ *lang: *en' | eval $(ERROR_ON_OUTPUT)
@harding

harding Aug 19, 2015

Contributor

Oh, duh, that is obviously much better. I don't know what I was thinking. I'll update, thanks!

Contributor

saivann commented Aug 19, 2015

@harding Another little change I may suggest is to add -type f to all find commands in the Makefile, so we don't accidentally match directories at some point.

Regarding the end_of_page hook, in case you prefer this idea, an alternative way of doing it, which would prevent repeating each <script> tag on each page, would be to set a scripts-devel-doc: true declaration for each of the devel-doc pages, and load these scripts conditionally in _includes/base/footer-js.html. The updateToc javascript function can stop gracefully if there is no toc on the page by just adding if (!document.getElementById('toc')) return; to the function.

But that's all just suggestions! :)

LGTM up to dddb1d2, Thanks!

Contributor

harding commented Aug 19, 2015

@saivann re: find -type f -- great idea! I'll do that.

Re: end_of_page hook: for now I think I like the flexibility of having the hook. In the future---hopefully not too far away---I plan to do some re-arranging of the docs to split the Bitcoin Core parts off from the more general parts, and I think the extra flexibility will come in handy there.

Contributor

harding commented Aug 20, 2015

Commit 2c0e9a0 updates the Makefile tests as suggested by @saivann

In the absence of critical feedback, this pull will be merged Sunday.

Contributor

saivann commented Aug 21, 2015

Thanks!

harding added some commits Aug 18, 2015

Add gitAttributes file to treat SVGs as binary (not text)
Also prevent SVGs from containing Javascript
Contributor

harding commented Aug 22, 2015

Rebased to fix expected merge conflict because of JQuery file renaming. Diff below (sorry, combined diffs aren't quite syntax highlighted properly):

diff --cc _layouts/glossary_entry.html
index 8436a56,2958726..0000000
--- a/_layouts/glossary_entry.html
+++ b/_layouts/glossary_entry.html
@@@ -9,6 -9,10 +9,10 @@@ breadcrumbs
    - dev docs
    - glossary
    - GLOSSARY_ENTRY_TITLE
+ end_of_page: |
 -  <script src="/js/jquery-1.11.2.min.js"></script>
 -  <script src="/js/jquery-ui.min.js"></script>
++  <script src="/js/jquery/jquery-1.11.2.min.js"></script>
++  <script src="/js/jquery/jquery-ui.min.js"></script>
+   <script src="/js/devsearch.js"></script>
  ---
  <link rel="stylesheet" href="/css/jquery-ui.min.css">

diff --cc en/developer-documentation.md
index 6825ea6,6597f29..0000000
--- a/en/developer-documentation.md
+++ b/en/developer-documentation.md
@@@ -9,6 -9,10 +9,10 @@@ title: "Developer Documentation - Bitco
  breadcrumbs:
    - bitcoin
    - Developer Documentation
+ end_of_page: |
 -  <script src="/js/jquery-1.11.2.min.js"></script>
 -  <script src="/js/jquery-ui.min.js"></script>
++  <script src="/js/jquery/jquery-1.11.2.min.js"></script>
++  <script src="/js/jquery/jquery-ui.min.js"></script>
+   <script src="/js/devsearch.js"></script>
  ---
  <link rel="stylesheet" href="/css/jquery-ui.min.css">

diff --cc en/developer-examples.md
index 8825d39,fa9ea39..0000000
--- a/en/developer-examples.md
+++ b/en/developer-examples.md
@@@ -10,6 -10,11 +10,11 @@@ breadcrumbs
    - bitcoin
    - dev docs
    - Examples
+ end_of_page: |
 -  <script src="/js/jquery-1.11.2.min.js"></script>
 -  <script src="/js/jquery-ui.min.js"></script>
++  <script src="/js/jquery/jquery-1.11.2.min.js"></script>
++  <script src="/js/jquery/jquery-ui.min.js"></script>
+   <script src="/js/devsearch.js"></script>
+   <script>updateToc();</script>
  ---
  <link rel="stylesheet" href="/css/jquery-ui.min.css">

diff --cc en/developer-glossary.html
index 2f119f4,bbdf30a..0000000
--- a/en/developer-glossary.html
+++ b/en/developer-glossary.html
@@@ -9,6 -9,10 +9,10 @@@ breadcrumbs
    - bitcoin
    - dev docs
    - Glossary
+ end_of_page: |
 -  <script src="/js/jquery-1.11.2.min.js"></script>
 -  <script src="/js/jquery-ui.min.js"></script>
++  <script src="/js/jquery/jquery-1.11.2.min.js"></script>
++  <script src="/js/jquery/jquery-ui.min.js"></script>
+   <script src="/js/devsearch.js"></script>
  ---
  <link rel="stylesheet" href="/css/jquery-ui.min.css">

diff --cc en/developer-guide.md
index 5f837a2,369ea97..0000000
--- a/en/developer-guide.md
+++ b/en/developer-guide.md
@@@ -10,6 -10,11 +10,11 @@@ breadcrumbs
    - bitcoin
    - dev docs
    - Guide
+ end_of_page: |
 -  <script src="/js/jquery-1.11.2.min.js"></script>
 -  <script src="/js/jquery-ui.min.js"></script>
++  <script src="/js/jquery/jquery-1.11.2.min.js"></script>
++  <script src="/js/jquery/jquery-ui.min.js"></script>
+   <script src="/js/devsearch.js"></script>
+   <script>updateToc();</script>
  ---
  <link rel="stylesheet" href="/css/jquery-ui.min.css">

diff --cc en/developer-reference.md
index e10a98f,9357d3e..0000000
--- a/en/developer-reference.md
+++ b/en/developer-reference.md
@@@ -10,6 -10,11 +10,11 @@@ breadcrumbs
    - bitcoin
    - dev docs
    - Reference
+ end_of_page: |
 -  <script src="/js/jquery-1.11.2.min.js"></script>
 -  <script src="/js/jquery-ui.min.js"></script>
++  <script src="/js/jquery/jquery-1.11.2.min.js"></script>
++  <script src="/js/jquery/jquery-ui.min.js"></script>
+   <script src="/js/devsearch.js"></script>
+   <script>updateToc();</script>
  ---
  <link rel="stylesheet" href="/css/jquery-ui.min.css">

@harding harding merged commit 0c0d979 into bitcoin-dot-org:master Aug 23, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

harding added a commit that referenced this pull request Aug 23, 2015

Merge pulls #1009 and #1012
- 1009: Small Miscellaneous Backend Changes
- 1012: Create Developer Reference RPC page for gettxoutproof and verifytxoutproof
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment