From 79dc68512fd7e77575ba1c35a31920bae5229c51 Mon Sep 17 00:00:00 2001 From: Jens Maier Date: Thu, 24 Jul 2014 17:34:13 +0200 Subject: [PATCH 1/5] FIX: dialects accept nested inline markup --- .../javascripts/discourse/dialects/dialect.js | 17 +++++++++-------- test/javascripts/lib/bbcode_test.js | 1 + 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/discourse/dialects/dialect.js b/app/assets/javascripts/discourse/dialects/dialect.js index 495d0fa6720..67f7cb8cc94 100644 --- a/app/assets/javascripts/discourse/dialects/dialect.js +++ b/app/assets/javascripts/discourse/dialects/dialect.js @@ -288,7 +288,7 @@ Discourse.Dialect = { this.registerInline(start, function(text, match, prev) { if (invalidBoundary(args, prev)) { return; } - var endPos = self.findEndPos(text, stop, args, startLength); + var endPos = self.findEndPos(text, start, stop, args, startLength); if (endPos === -1) { return; } var between = text.slice(startLength, endPos); @@ -304,13 +304,14 @@ Discourse.Dialect = { }); }, - findEndPos: function(text, stop, args, start) { - var endPos = text.indexOf(stop, start); - if (endPos === -1) { return -1; } - var after = text.charAt(endPos + stop.length); - if (after && after.indexOf(stop) === 0) { - return this.findEndPos(text, stop, args, endPos + stop.length + 1); - } + findEndPos: function(text, start, stop, args, offset) { + var endPos, nextStart; + do { + endPos = text.indexOf(stop, offset); + if (endPos === -1) { return -1; } + nextStart = text.indexOf(start, offset); + offset = endPos + stop.length; + } while (nextStart !== -1 && nextStart < endPos); return endPos; }, diff --git a/test/javascripts/lib/bbcode_test.js b/test/javascripts/lib/bbcode_test.js index 7d630a16b87..c269cff4537 100644 --- a/test/javascripts/lib/bbcode_test.js +++ b/test/javascripts/lib/bbcode_test.js @@ -22,6 +22,7 @@ test('basic bbcode', function() { "evil trout", "allows embedding of tags"); format("[EMAIL]eviltrout@mailinator.com[/EMAIL]", "eviltrout@mailinator.com", "supports upper case bbcode"); + format("[b]strong [b]stronger[/b][/b]", "strong stronger", "accepts nested bbcode tags"); }); test('invalid bbcode', function() { From b19ad1508684990c5008652648b81ad6f56d3453 Mon Sep 17 00:00:00 2001 From: Jens Maier Date: Thu, 24 Jul 2014 19:39:16 +0200 Subject: [PATCH 2/5] FIX: improve list bbcodes: ignore newlines resulting in unnecessary blank lines --- .../discourse/dialects/bbcode_dialect.js | 20 ++++++++++++++++--- test/javascripts/lib/bbcode_test.js | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/discourse/dialects/bbcode_dialect.js b/app/assets/javascripts/discourse/dialects/bbcode_dialect.js index fe27c9c0a3d..d04f675de68 100644 --- a/app/assets/javascripts/discourse/dialects/bbcode_dialect.js +++ b/app/assets/javascripts/discourse/dialects/bbcode_dialect.js @@ -55,6 +55,20 @@ function replaceBBCodeParamsRaw(tag, emitter) { }); } +/** + Filters an array of JSON-ML nodes, removing nodes that represent empty lines ("\n"). + + @method removeEmptyLines + @param {Array} [contents] Array of JSON-ML nodes +**/ +function removeEmptyLines(contents) { + var result = []; + for (var i=0; i < contents.length; i++) { + if (contents[i] !== "\n") { result.push(contents[i]); } + } + return result; +} + /** Creates a BBCode handler that accepts parameters. Passes them to the emitter. Processes the inside recursively so it can be nested. @@ -75,9 +89,9 @@ replaceBBCode('u', function(contents) { return ['span', {'class': 'bbcode-u'}].c replaceBBCode('s', function(contents) { return ['span', {'class': 'bbcode-s'}].concat(contents); }); Discourse.Markdown.whiteListTag('span', 'class', /^bbcode-[bius]$/); -replaceBBCode('ul', function(contents) { return ['ul'].concat(contents); }); -replaceBBCode('ol', function(contents) { return ['ol'].concat(contents); }); -replaceBBCode('li', function(contents) { return ['li'].concat(contents); }); +replaceBBCode('ul', function(contents) { return ['ul'].concat(removeEmptyLines(contents)); }); +replaceBBCode('ol', function(contents) { return ['ol'].concat(removeEmptyLines(contents)); }); +replaceBBCode('li', function(contents) { return ['li'].concat(removeEmptyLines(contents)); }); rawBBCode('img', function(contents) { return ['img', {href: contents}]; }); rawBBCode('email', function(contents) { return ['a', {href: "mailto:" + contents, 'data-bbcode': true}, contents]; }); diff --git a/test/javascripts/lib/bbcode_test.js b/test/javascripts/lib/bbcode_test.js index c269cff4537..f07c1274a21 100644 --- a/test/javascripts/lib/bbcode_test.js +++ b/test/javascripts/lib/bbcode_test.js @@ -45,6 +45,7 @@ test('spoiler', function() { test('lists', function() { format("[ul][li]option one[/li][/ul]", "", "creates an ul"); format("[ol][li]option one[/li][/ol]", "
  1. option one
", "creates an ol"); + format("[ul]\n[li]option one[/li]\n[li]option two[/li]\n[/ul]", "", "suppresses empty lines in lists"); }); test('tags with arguments', function() { From 9124cf0eafad46586a45dd8220f8dcc9094dad24 Mon Sep 17 00:00:00 2001 From: Jens Maier Date: Sun, 27 Jul 2014 15:57:40 +0200 Subject: [PATCH 3/5] FIX: auto-quote should not trigger when the first " is preceded by bbcode-like garbage. --- test/javascripts/lib/markdown_test.js | 1 + vendor/assets/javascripts/better_markdown.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/javascripts/lib/markdown_test.js b/test/javascripts/lib/markdown_test.js index 7a961900b71..7524aa68784 100644 --- a/test/javascripts/lib/markdown_test.js +++ b/test/javascripts/lib/markdown_test.js @@ -36,6 +36,7 @@ test("Auto quoting", function() { "it converts single line quotes to blockquotes"); cooked('"hello\nworld"', "

\"hello
world\"

", "It doesn't convert multi line quotes"); cooked('"hello "evil" trout"', '

"hello "evil" trout"

', "it doesn't format quotes in the middle of a line"); + cooked('["text"', '

["text"

', "it recognizes leading tag-like text"); }); test("Traditional Line Breaks", function() { diff --git a/vendor/assets/javascripts/better_markdown.js b/vendor/assets/javascripts/better_markdown.js index 732764d8cfd..017cb414da1 100644 --- a/vendor/assets/javascripts/better_markdown.js +++ b/vendor/assets/javascripts/better_markdown.js @@ -662,7 +662,7 @@ return [consumed, null, nodes]; } - var res = this.dialect.inline.__oneElement__.call(this, text.substr( consumed ), patterns ); + var res = this.dialect.inline.__oneElement__.call(this, text.substr( consumed ), patterns, [text.substr(0, consumed)]); consumed += res[ 0 ]; // Add any returned nodes. nodes.push.apply( nodes, res.slice( 1 ) ); From 479eb64a7691662ae415bf9b8faa112b5befa9b8 Mon Sep 17 00:00:00 2001 From: Jens Maier Date: Sun, 27 Jul 2014 16:07:47 +0200 Subject: [PATCH 4/5] FIX: rewrite replaceBlock logic to better handle mismatched nested quotes. --- .../discourse/dialects/code_dialect.js | 13 +- .../javascripts/discourse/dialects/dialect.js | 144 ++++++++---------- test/javascripts/lib/bbcode_test.js | 10 ++ test/javascripts/lib/markdown_test.js | 2 +- 4 files changed, 85 insertions(+), 84 deletions(-) diff --git a/app/assets/javascripts/discourse/dialects/code_dialect.js b/app/assets/javascripts/discourse/dialects/code_dialect.js index 8f0ff739a27..5bb5d6b8b00 100644 --- a/app/assets/javascripts/discourse/dialects/code_dialect.js +++ b/app/assets/javascripts/discourse/dialects/code_dialect.js @@ -10,6 +10,15 @@ var acceptableCodeClasses = "profile", "python", "r", "rib", "rsl", "ruby", "rust", "scala", "smalltalk", "sql", "tex", "text", "vala", "vbscript", "vhdl"]; +function flattenBlocks(blocks) { + var result = ""; + blocks.forEach(function(b) { + result += b; + if (b.trailing) { result += b.trailing; } + }); + return result; +} + Discourse.Dialect.replaceBlock({ start: /^`{3}([^\n\[\]]+)?\n?([\s\S]*)?/gm, stop: '```', @@ -19,7 +28,7 @@ Discourse.Dialect.replaceBlock({ if (matches[1] && acceptableCodeClasses.indexOf(matches[1]) !== -1) { klass = matches[1]; } - return ['p', ['pre', ['code', {'class': klass}, blockContents.join("\n") ]]]; + return ['p', ['pre', ['code', {'class': klass}, flattenBlocks(blockContents) ]]]; } }); @@ -50,6 +59,6 @@ Discourse.Dialect.replaceBlock({ skipIfTradtionalLinebreaks: true, emitter: function(blockContents) { - return ['p', ['pre', blockContents.join("\n")]]; + return ['p', ['pre', flattenBlocks(blockContents)]]; } }); diff --git a/app/assets/javascripts/discourse/dialects/dialect.js b/app/assets/javascripts/discourse/dialects/dialect.js index 67f7cb8cc94..94d81d1d0ed 100644 --- a/app/assets/javascripts/discourse/dialects/dialect.js +++ b/app/assets/javascripts/discourse/dialects/dialect.js @@ -133,6 +133,19 @@ function invalidBoundary(args, prev) { if (args.spaceOrTagBoundary && (!last.match(/(\s|\>)$/))) { return true; } } +/** + Returns the number of (terminated) lines in a string. + + @method countLines + @param {string} str the string. + @returns {Integer} number of terminated lines in str +**/ +function countLines(str) { + var index = -1, count = 0; + while ((index = str.indexOf("\n", index + 1)) !== -1) { count++; } + return count; +} + /** An object used for rendering our dialects. @@ -359,102 +372,71 @@ Discourse.Dialect = { var linebreaks = dialect.options.traditional_markdown_linebreaks || Discourse.SiteSettings.traditional_markdown_linebreaks; - - // Some replacers should not be run with traditional linebreaks if (linebreaks && args.skipIfTradtionalLinebreaks) { return; } args.start.lastIndex = 0; - var m = (args.start).exec(block); + var result = [], match = (args.start).exec(block); + if (!match) { return; } - if (!m) { return; } + var lastChance = function() { + return !next.some(function(e) { return e.indexOf(args.stop) !== -1; }); + }; - var startPos = args.start.lastIndex - m[0].length, - leading, - blockContents = [], - result = [], - lineNumber = block.lineNumber; - - if (startPos > 0) { - leading = block.slice(0, startPos); - lineNumber += (leading.split("\n").length - 1); - - var para = ['p']; - this.processInline(leading).forEach(function (l) { - para.push(l); - }); - - result.push(para); + // shave off start tag and leading text, if any. + var pos = args.start.lastIndex - match[0].length, + leading = block.slice(0, pos), + trailing = match[2] ? match[2].replace(/^\n*/, "") : ""; + if (block.indexOf(args.stop, pos + args.stop.length) === -1 && lastChance()) { return; } + if (leading.length > 0) { result.push(['p'].concat(this.processInline(leading))); } + if (trailing.length > 0) { + next.unshift(MD.mk_block(trailing, block.trailing, + block.lineNumber + countLines(leading) + (match[2] ? match[2].length : 0) - trailing.length)); } - if (m[2]) { - next.unshift(MD.mk_block(m[2], null, lineNumber + 1)); - } + // find matching stop tag in blocks. + var contentBlocks = [], nesting = 0, endPos, ep, offset, startPos, sp, m, b; + blockloop: + while (b = next.shift()) { + args.start.lastIndex = 0; + startPos = []; sp = 0; + while (m = (args.start).exec(b)) { + startPos.push(args.start.lastIndex - m[0].length); + args.start.lastIndex = args.start.lastIndex - (m[2] ? m[2].length : 0); + } + endPos = []; ep = 0; offset = 0; + while ((pos = b.indexOf(args.stop, offset)) !== -1) { + endPos.push(pos); + offset += (pos + args.stop.length); + } - lineNumber++; + while (ep < endPos.length) { + if (sp < startPos.length && startPos[sp] < endPos[ep]) { + sp++; nesting++; + } else if (nesting > 0) { + ep++; nesting--; + } else { + break blockloop; + } + } - var blockClosed = false; - for (var i=0; i= 0) { - blockClosed = true; + if (lastChance()) { + ep = endPos.length - 1; break; } + + nesting += startPos.length - sp; + contentBlocks.push(b); } - if (!blockClosed) { - if (m[2]) { next.shift(); } - return; + if (ep < endPos.length) { + var before = b.slice(0, endPos[ep]).replace(/\n*$/, ""), + after = b.slice(endPos[ep] + args.stop.length).replace(/^\n*/, ""); + if (before.length > 0) contentBlocks.push(MD.mk_block(before, "", b.lineNumber)); + if (after.length > 0) next.unshift(MD.mk_block(after, "", b.lineNumber + countLines(before))); } - var numOpen = 1; - while (next.length > 0) { - var b = next.shift(), - blockLine = b.lineNumber, - diff = ((typeof blockLine === "undefined") ? lineNumber : blockLine) - lineNumber, - endFound = b.indexOf(args.stop), - leadingContents = b.slice(0, endFound), - trailingContents = b.slice(endFound+args.stop.length), - m2; - - if (endFound === -1) { - leadingContents = b; - } - - args.start.lastIndex = 0; - if (m2 = (args.start).exec(leadingContents)) { - numOpen++; - args.start.lastIndex -= m2[0].length - 1; - while (m2 = (args.start).exec(leadingContents)) { - numOpen++; - args.start.lastIndex -= m2[0].length - 1; - } - } - - if (endFound >= 0) { numOpen--; } - for (var j=1; j= 0) { - if (trailingContents) { - next.unshift(MD.mk_block(trailingContents.replace(/^\s+/, ""))); - } - - blockContents.push(leadingContents.replace(/\s+$/, "")); - - if (numOpen === 0) { - break; - } - blockContents.push(args.stop); - } else { - blockContents.push(b); - } - } - - var emitterResult = args.emitter.call(this, blockContents, m, dialect.options); - if (emitterResult) { - result.push(emitterResult); - } + var emitterResult = args.emitter.call(this, contentBlocks, match, dialect.options); + if (emitterResult) { result.push(emitterResult); } return result; }); }, diff --git a/test/javascripts/lib/bbcode_test.js b/test/javascripts/lib/bbcode_test.js index f07c1274a21..9d26b1fa633 100644 --- a/test/javascripts/lib/bbcode_test.js +++ b/test/javascripts/lib/bbcode_test.js @@ -129,6 +129,16 @@ test("quote formatting", function() { "

abc

\n\n

hello

", "handles new lines properly"); + formatQ("[quote=\"Alice, post:1, topic:1\"]\n[quote=\"Bob, post:2, topic:1\"]\n[/quote]\n[/quote]", + "", + "quotes can be nested"); + + formatQ("[quote=\"Alice, post:1, topic:1\"]\n[quote=\"Bob, post:2, topic:1\"]\n[/quote]", + "", + "handles mismatched nested quote tags"); }); test("quotes with trailing formatting", function() { diff --git a/test/javascripts/lib/markdown_test.js b/test/javascripts/lib/markdown_test.js index 7524aa68784..85f8cf40b53 100644 --- a/test/javascripts/lib/markdown_test.js +++ b/test/javascripts/lib/markdown_test.js @@ -301,7 +301,7 @@ test("links with full urls", function() { test("Code Blocks", function() { cooked("
\nhello\n
\n", - "

\nhello

", + "

hello

", "pre blocks don't include extra lines"); cooked("```\na\nb\nc\n\nd\n```", From 90d14d9ffc81ebaa473d43cd980d540fef169355 Mon Sep 17 00:00:00 2001 From: Jens Maier Date: Mon, 28 Jul 2014 18:53:10 +0200 Subject: [PATCH 5/5] add comments and improve variable names --- .../javascripts/discourse/dialects/dialect.js | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/discourse/dialects/dialect.js b/app/assets/javascripts/discourse/dialects/dialect.js index 94d81d1d0ed..663e7205c6b 100644 --- a/app/assets/javascripts/discourse/dialects/dialect.js +++ b/app/assets/javascripts/discourse/dialects/dialect.js @@ -386,6 +386,7 @@ Discourse.Dialect = { var pos = args.start.lastIndex - match[0].length, leading = block.slice(0, pos), trailing = match[2] ? match[2].replace(/^\n*/, "") : ""; + // just give up if there's no stop tag in this or any next block if (block.indexOf(args.stop, pos + args.stop.length) === -1 && lastChance()) { return; } if (leading.length > 0) { result.push(['p'].concat(this.processInline(leading))); } if (trailing.length > 0) { @@ -393,47 +394,58 @@ Discourse.Dialect = { block.lineNumber + countLines(leading) + (match[2] ? match[2].length : 0) - trailing.length)); } - // find matching stop tag in blocks. - var contentBlocks = [], nesting = 0, endPos, ep, offset, startPos, sp, m, b; + // go through the available blocks to find the matching stop tag. + var contentBlocks = [], nesting = 0, actualEndPos = -1, currentBlock; blockloop: - while (b = next.shift()) { + while (currentBlock = next.shift()) { + // collect all the start and stop tags in the current block args.start.lastIndex = 0; - startPos = []; sp = 0; - while (m = (args.start).exec(b)) { + var startPos = [], m; + while (m = (args.start).exec(currentBlock)) { startPos.push(args.start.lastIndex - m[0].length); args.start.lastIndex = args.start.lastIndex - (m[2] ? m[2].length : 0); } - endPos = []; ep = 0; offset = 0; - while ((pos = b.indexOf(args.stop, offset)) !== -1) { + var endPos = [], offset = 0; + while ((pos = currentBlock.indexOf(args.stop, offset)) !== -1) { endPos.push(pos); offset += (pos + args.stop.length); } + // go through the available end tags: + var ep = 0, sp = 0; // array indices while (ep < endPos.length) { if (sp < startPos.length && startPos[sp] < endPos[ep]) { + // there's an end tag, but there's also another start tag first. we need to go deeper. sp++; nesting++; } else if (nesting > 0) { + // found an end tag, but we must go up a level first. ep++; nesting--; } else { + // found an end tag and we're at the top: done! + actualEndPos = endPos[ep]; break blockloop; } } if (lastChance()) { - ep = endPos.length - 1; + // when lastChance() becomes true the first time, currentBlock contains the last + // end tag available in the input blocks but it's not on the right nesting level + // or we would have terminated the loop already. the only thing we can do is to + // treat the last available end tag as tho it were matched with our start tag + // and let the emitter figure out how to render the garbage inside. + actualEndPos = endPos[endPos.length - 1]; break; } + // any left-over start tags still increase the nesting level nesting += startPos.length - sp; - contentBlocks.push(b); + contentBlocks.push(currentBlock); } - if (ep < endPos.length) { - var before = b.slice(0, endPos[ep]).replace(/\n*$/, ""), - after = b.slice(endPos[ep] + args.stop.length).replace(/^\n*/, ""); - if (before.length > 0) contentBlocks.push(MD.mk_block(before, "", b.lineNumber)); - if (after.length > 0) next.unshift(MD.mk_block(after, "", b.lineNumber + countLines(before))); - } + var before = currentBlock.slice(0, actualEndPos).replace(/\n*$/, ""), + after = currentBlock.slice(actualEndPos + args.stop.length).replace(/^\n*/, ""); + if (before.length > 0) contentBlocks.push(MD.mk_block(before, "", currentBlock.lineNumber)); + if (after.length > 0) next.unshift(MD.mk_block(after, "", currentBlock.lineNumber + countLines(before))); var emitterResult = args.emitter.call(this, contentBlocks, match, dialect.options); if (emitterResult) { result.push(emitterResult); }