Skip to content

Commit

Permalink
Feature: Change markdown engine to markdown it
Browse files Browse the repository at this point in the history
This commit removes the old evilstreak markdownjs engine.

- Adds specs to WhiteLister and changes it to stop using globals
    (Fixes large memory leak)
- Fixes edge cases around bbcode handling
- Removes mdtest which is no longer valid (to be replaced with
    CommonMark)
- Updates MiniRacer to correct minor unmanaged memory leak
- Fixes plugin specs
  • Loading branch information
SamSaffron committed Jul 17, 2017
1 parent 9e03fae commit d0c5205
Show file tree
Hide file tree
Showing 123 changed files with 1,153 additions and 7,844 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ GEM
mime-types (2.99.3)
mini_mime (0.1.3)
mini_portile2 (2.2.0)
mini_racer (0.1.9)
mini_racer (0.1.10)
libv8 (~> 5.3)
minitest (5.10.2)
mocha (1.2.1)
Expand Down
28 changes: 14 additions & 14 deletions app/assets/javascripts/markdown-it-bundle.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
//= require markdown-it.js
//= require ./pretty-text/engines/markdown-it/helpers
//= require ./pretty-text/engines/markdown-it/mentions
//= require ./pretty-text/engines/markdown-it/quotes
//= require ./pretty-text/engines/markdown-it/emoji
//= require ./pretty-text/engines/markdown-it/onebox
//= require ./pretty-text/engines/markdown-it/bbcode-block
//= require ./pretty-text/engines/markdown-it/bbcode-inline
//= require ./pretty-text/engines/markdown-it/code
//= require ./pretty-text/engines/markdown-it/category-hashtag
//= require ./pretty-text/engines/markdown-it/censored
//= require ./pretty-text/engines/markdown-it/table
//= require ./pretty-text/engines/markdown-it/paragraph
//= require ./pretty-text/engines/markdown-it/newline
//= require ./pretty-text/engines/markdown-it/html_img
//= require ./pretty-text/engines/discourse-markdown/helpers
//= require ./pretty-text/engines/discourse-markdown/mentions
//= require ./pretty-text/engines/discourse-markdown/quotes
//= require ./pretty-text/engines/discourse-markdown/emoji
//= require ./pretty-text/engines/discourse-markdown/onebox
//= require ./pretty-text/engines/discourse-markdown/bbcode-block
//= require ./pretty-text/engines/discourse-markdown/bbcode-inline
//= require ./pretty-text/engines/discourse-markdown/code
//= require ./pretty-text/engines/discourse-markdown/category-hashtag
//= require ./pretty-text/engines/discourse-markdown/censored
//= require ./pretty-text/engines/discourse-markdown/table
//= require ./pretty-text/engines/discourse-markdown/paragraph
//= require ./pretty-text/engines/discourse-markdown/newline
//= require ./pretty-text/engines/discourse-markdown/html_img
3 changes: 0 additions & 3 deletions app/assets/javascripts/pretty-text-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@
//= require ./pretty-text/censored-words
//= require ./pretty-text/emoji/data
//= require ./pretty-text/emoji
//= require ./pretty-text/engines/discourse-markdown
//= require ./pretty-text/engines/discourse-markdown-it
//= require_tree ./pretty-text/engines/discourse-markdown
//= require xss.min
//= require better_markdown.js
//= require ./pretty-text/xss
//= require ./pretty-text/white-lister
//= require ./pretty-text/sanitizer
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { default as WhiteLister, whiteListFeature } from 'pretty-text/white-lister';
import { default as WhiteLister } from 'pretty-text/white-lister';
import { sanitize } from 'pretty-text/sanitizer';
import guid from 'pretty-text/guid';

Expand All @@ -10,10 +10,10 @@ function deprecate(feature, name){
};
}

function createHelper(featureName, opts, optionCallbacks, pluginCallbacks, getOptions) {
function createHelper(featureName, opts, optionCallbacks, pluginCallbacks, getOptions, whiteListed) {
let helper = {};
helper.markdownIt = true;
helper.whiteList = info => whiteListFeature(featureName, info);
helper.whiteList = info => whiteListed.push([featureName, info]);
helper.registerInline = deprecate(featureName,'registerInline');
helper.replaceBlock = deprecate(featureName,'replaceBlock');
helper.addPreProcessor = deprecate(featureName,'addPreProcessor');
Expand Down Expand Up @@ -151,7 +151,7 @@ export function setup(opts, siteSettings, state) {
}

// we got to require this late cause bundle is not loaded in pretty-text
Helpers = Helpers || requirejs('pretty-text/engines/markdown-it/helpers');
Helpers = Helpers || requirejs('pretty-text/engines/discourse-markdown/helpers');

opts.markdownIt = true;

Expand All @@ -165,6 +165,7 @@ export function setup(opts, siteSettings, state) {

const check = /discourse-markdown\/|markdown-it\//;
let features = [];
let whiteListed = [];

Object.keys(require._eak_seen).forEach(entry => {
if (check.test(entry)) {
Expand All @@ -173,7 +174,7 @@ export function setup(opts, siteSettings, state) {

const featureName = entry.split('/').reverse()[0];
features.push(featureName);
module.setup(createHelper(featureName, opts, optionCallbacks, pluginCallbacks, getOptions));
module.setup(createHelper(featureName, opts, optionCallbacks, pluginCallbacks, getOptions, whiteListed));
}
}
});
Expand Down Expand Up @@ -227,10 +228,16 @@ export function setup(opts, siteSettings, state) {
opts.markdownIt = true;
opts.setup = true;

if (!opts.discourse.sanitizer) {
if (!opts.discourse.sanitizer || !opts.sanitizer) {
const whiteLister = new WhiteLister(opts.discourse);

whiteListed.forEach(([feature, info]) => {
whiteLister.whiteListFeature(feature, info);
});

opts.sanitizer = opts.discourse.sanitizer = (!!opts.discourse.sanitize) ? a=>sanitize(a, whiteLister) : a=>a;
}

}

export function cook(raw, opts) {
Expand Down
Loading

4 comments on commit d0c5205

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/discourse-yuml-add-yuml-diagrams-to-discourse-posts/30448/13

@fsalvi
Copy link

@fsalvi fsalvi commented on d0c5205 Jul 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this modification, it results (at least in my setup with nginx reverse proxy and web.socketed template) in "Discourse.MarkdownItURL" using a http url when the website is used in https. So, browsers send alerts/warning about mixed content...

@SamSaffron
Copy link
Member Author

@SamSaffron SamSaffron commented on d0c5205 Jul 20, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fsalvi
Copy link

@fsalvi fsalvi commented on d0c5205 Jul 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it possible to have discourse working correctly in http and https at the same time ?
It's the only element which causes a problem... (earlier versions were ok)

Please sign in to comment.