Merge pull request #1335 from GitbookIO/fix/1333

Fix #1333: fix invalid URLs for glossary annotations

6 files changed, 121 insertions, 22 deletions

diff --git a/lib/output/__tests__/website.js b/lib/output/__tests__/website.js
index 554c56c..501503a 100644
--- a/lib/output/__tests__/website.js
+++ b/lib/output/__tests__/website.js
@@ -1,3 +1,4 @@
+var fs = require('fs');
 var generateMock = require('../generateMock');
 var WebsiteGenerator = require('../website');
 
@@ -12,6 +13,54 @@ describe('WebsiteGenerator', function() {
         });
     });
 
+    describe('Glossary', function() {
+        var folder;
+
+        before(function() {
+            return generateMock(WebsiteGenerator, {
+                'README.md': 'Hello World',
+                'SUMMARY.md': '* [Deep](folder/page.md)',
+                'folder': {
+                    'page.md': 'Hello World'
+                },
+                'GLOSSARY.md': '# Glossary\n\n## Hello\n\nHello World'
+            })
+            .then(function(_folder) {
+                folder = _folder;
+            });
+        });
+
+        it('should generate a GLOSSARY.html', function() {
+            expect(folder).toHaveFile('GLOSSARY.html');
+        });
+
+        it('should correctly resolve glossary links in README', function() {
+            var html = fs.readFileSync(folder + '/index.html', 'utf8');
+            expect(html).toHaveDOMElement('.page-inner a[href="GLOSSARY.html#hello"]');
+        });
+
+        it('should correctly resolve glossary links in directory', function() {
+            var html = fs.readFileSync(folder + '/folder/page.html', 'utf8');
+            expect(html).toHaveDOMElement('.page-inner a[href="../GLOSSARY.html#hello"]');
+        });
+
+        it('should accept a custom glossary file', function() {
+            return generateMock(WebsiteGenerator, {
+                'README.md': 'Hello World',
+                'book.json': '{ "structure": { "glossary": "custom.md" } }',
+                'custom.md': '# Glossary\n\n## Hello\n\nHello World'
+            })
+            .then(function(folder) {
+                expect(folder).toHaveFile('custom.html');
+                expect(folder).toNotHaveFile('GLOSSARY.html');
+
+                var html = fs.readFileSync(folder + '/index.html', 'utf8');
+                expect(html).toHaveDOMElement('.page-inner a[href="custom.html#hello"]');
+            });
+        });
+    });
+
+
     it('should copy asset files', function() {
         return generateMock(WebsiteGenerator, {
             'README.md': 'Hello World',
diff --git a/lib/output/getModifiers.js b/lib/output/getModifiers.js
index e649df6..66fbc1a 100644
--- a/lib/output/getModifiers.js
+++ b/lib/output/getModifiers.js
@@ -4,6 +4,7 @@ var Api = require('../api');
 var Plugins = require('../plugins');
 var Promise = require('../utils/promise');
 var defaultBlocks = require('../constants/defaultBlocks');
+var fileToOutput = require('./helper/fileToOutput');
 
 var CODEBLOCK = 'code';
 
@@ -17,9 +18,13 @@ function getModifiers(output, page) {
     var book = output.getBook();
     var plugins = output.getPlugins();
     var glossary = book.getGlossary();
-    var entries = glossary.getEntries();
     var file = page.getFile();
 
+    // Glossary entries
+    var entries = glossary.getEntries();
+    var glossaryFile = glossary.getFile();
+    var glossaryFilename = fileToOutput(output, glossaryFile.getPath());
+
     // Current file path
     var currentFilePath = file.getPath();
 
@@ -34,18 +39,18 @@ function getModifiers(output, page) {
         // Normalize IDs on headings
         Modifiers.addHeadingId,
 
+        // Annotate text with glossary entries
+        Modifiers.annotateText.bind(null, entries, glossaryFilename),
+
+        // Resolve images
+        Modifiers.resolveImages.bind(null, currentFilePath),
+
         // Resolve links (.md -> .html)
         Modifiers.resolveLinks.bind(null,
             currentFilePath,
             resolveFileToURL.bind(null, output)
         ),
 
-        // Resolve images
-        Modifiers.resolveImages.bind(null, currentFilePath),
-
-        // Annotate text with glossary entries
-        Modifiers.annotateText.bind(null, entries),
-
         // Highlight code blocks using "code" block
         Modifiers.highlightCode.bind(null, function(lang, source) {
             return Promise(code.applyBlock({
diff --git a/lib/output/modifiers/__tests__/annotateText.js b/lib/output/modifiers/__tests__/annotateText.js
index 40b1e6c..67e7a10 100644
--- a/lib/output/modifiers/__tests__/annotateText.js
+++ b/lib/output/modifiers/__tests__/annotateText.js
@@ -12,7 +12,7 @@ describe('annotateText', function() {
     it('should annotate text', function() {
         var $ = cheerio.load('<p>This is a word, and multiple words</p>');
 
-        annotateText(entries, $);
+        annotateText(entries, 'GLOSSARY.md', $);
 
         var links = $('a');
         expect(links.length).toBe(2);
@@ -31,14 +31,14 @@ describe('annotateText', function() {
     it('should not annotate scripts', function() {
         var $ = cheerio.load('<script>This is a word, and multiple words</script>');
 
-        annotateText(entries, $);
+        annotateText(entries, 'GLOSSARY.md', $);
         expect($('a').length).toBe(0);
     });
 
     it('should not annotate when has class "no-glossary"', function() {
         var $ = cheerio.load('<p class="no-glossary">This is a word, and multiple words</p>');
 
-        annotateText(entries, $);
+        annotateText(entries, 'GLOSSARY.md', $);
         expect($('a').length).toBe(0);
     });
 });
diff --git a/lib/output/modifiers/annotateText.js b/lib/output/modifiers/annotateText.js
index d8443cf..2b4b439 100644
--- a/lib/output/modifiers/annotateText.js
+++ b/lib/output/modifiers/annotateText.js
@@ -62,9 +62,10 @@ function replaceText($, el, search, replace, text_only ) {
     Annotate text using a list of GlossaryEntry
 
     @param {List<GlossaryEntry>}
+    @param {String} glossaryFilePath
     @param {HTMLDom} $
 */
-function annotateText(entries, $) {
+function annotateText(entries, glossaryFilePath, $) {
     entries.forEach(function(entry) {
         var entryId = entry.getID();
         var name = entry.getName();
@@ -81,7 +82,7 @@ function annotateText(entries, $) {
             ) return;
 
             replaceText($, this, searchRegex, function(match) {
-                return '<a href="/GLOSSARY.md#' + entryId + '" '
+                return '<a href="/' + glossaryFilePath + '#' + entryId + '" '
                     + 'class="glossary-term" title="' + escape(description) + '">'
                     + match
                     + '</a>';
diff --git a/lib/parse/parsePagesList.js b/lib/parse/parsePagesList.js
index 8644fa1..1cf42f5 100644
--- a/lib/parse/parsePagesList.js
+++ b/lib/parse/parsePagesList.js
@@ -5,6 +5,25 @@ var Page = require('../models/page');
 var walkSummary = require('./walkSummary');
 var parsePage = require('./parsePage');
 
+
+/**
+    Parse a page from a path
+
+    @param {Book} book
+    @param {String} filePath
+    @return {Page}
+*/
+function parseFilePage(book, filePath) {
+    var fs = book.getContentFS();
+
+    return fs.statFile(filePath)
+    .then(function(file) {
+        var page = Page.createForFile(file);
+        return parsePage(book, page);
+    });
+}
+
+
 /**
     Parse all pages from a book as an OrderedMap
 
@@ -12,10 +31,11 @@ var parsePage = require('./parsePage');
     @return {Promise<OrderedMap<Page>>}
 */
 function parsePagesList(book) {
-    var fs = book.getContentFS();
     var summary = book.getSummary();
+    var glossary = book.getGlossary();
     var map = Immutable.OrderedMap();
 
+    // Parse pages from summary
     return timing.measure(
         'parse.listPages',
         walkSummary(summary, function(article) {
@@ -26,21 +46,32 @@ function parsePagesList(book) {
             // Is the page ignored?
             if (book.isContentFileIgnored(filepath)) return;
 
-            return fs.statFile(filepath)
-            .then(function(file) {
-                var page = Page.createForFile(file);
-                return parsePage(book, page);
-            })
+            return parseFilePage(book, filepath)
             .then(function(page) {
                 map = map.set(filepath, page);
             }, function() {
                 // file doesn't exist
             });
         })
-        .then(function() {
-            return map;
-        })
-    );
+    )
+
+    // Parse glossary
+    .then(function() {
+        var file = glossary.getFile();
+
+        if (!file.exists()) {
+            return;
+        }
+
+        return parseFilePage(book, file.getPath())
+        .then(function(page) {
+            map = map.set(file.getPath(), page);
+        });
+    })
+
+    .then(function() {
+        return map;
+    });
 }
 
 
diff --git a/testing/setup.js b/testing/setup.js
index b91a299..59fef77 100644
--- a/testing/setup.js
+++ b/testing/setup.js
@@ -2,6 +2,7 @@ var is = require('is');
 var path = require('path');
 var fs = require('fs');
 var expect = require('expect');
+var cheerio = require('cheerio');
 
 expect.extend({
     /**
@@ -43,6 +44,18 @@ expect.extend({
             'expected to be defined'
         );
         return this;
+    },
+
+    /**
+        Check that a dom element exists in HTML
+
+        @param {String} selector
+    */
+    toHaveDOMElement: function(selector) {
+        var $ = cheerio.load(this.actual);
+        var $el = $(selector);
+
+        expect.assert($el.length > 0, 'expected HTML to contains %s', selector);
     }
 });