Styling: Fix an offset bug

That caused empty inline code hints to be considered valid.

Also update the tests that were failing due to the changes in the
previous commit.
This commit is contained in:
JC Brand 2021-06-22 16:52:50 +02:00
parent 6dea5959cc
commit fb6bafdf6c
3 changed files with 24 additions and 41 deletions

View File

@ -54,6 +54,7 @@ module.exports = function(config) {
{ pattern: "src/plugins/chatview/tests/oob.js", type: 'module' }, { pattern: "src/plugins/chatview/tests/oob.js", type: 'module' },
{ pattern: "src/plugins/chatview/tests/receipts.js", type: 'module' }, { pattern: "src/plugins/chatview/tests/receipts.js", type: 'module' },
{ pattern: "src/plugins/chatview/tests/spoilers.js", type: 'module' }, { pattern: "src/plugins/chatview/tests/spoilers.js", type: 'module' },
{ pattern: "src/plugins/chatview/tests/styling.js", type: 'module' },
{ pattern: "src/plugins/chatview/tests/xss.js", type: 'module' }, { pattern: "src/plugins/chatview/tests/xss.js", type: 'module' },
{ pattern: "src/plugins/controlbox/tests/controlbox.js", type: 'module' }, { pattern: "src/plugins/controlbox/tests/controlbox.js", type: 'module' },
{ pattern: "src/plugins/controlbox/tests/login.js", type: 'module' }, { pattern: "src/plugins/controlbox/tests/login.js", type: 'module' },
@ -89,7 +90,6 @@ module.exports = function(config) {
{ pattern: "src/plugins/rosterview/tests/presence.js", type: 'module' }, { pattern: "src/plugins/rosterview/tests/presence.js", type: 'module' },
{ pattern: "src/plugins/rosterview/tests/protocol.js", type: 'module' }, { pattern: "src/plugins/rosterview/tests/protocol.js", type: 'module' },
{ pattern: "src/plugins/rosterview/tests/roster.js", type: 'module' }, { pattern: "src/plugins/rosterview/tests/roster.js", type: 'module' },
{ pattern: "src/shared/chat/tests/styling.js", type: 'module' },
], ],
proxies: { proxies: {

View File

@ -130,7 +130,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 5); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 5);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<span class="styling-directive">~</span><del> Hello! <span title=":poop:">💩</span> </del><span class="styling-directive">~</span>'); '<span class="styling-directive">~</span><del> Hello! <span title=":poop:">💩</span> </del><span class="styling-directive">~</span>');
@ -206,7 +205,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'Here\'s a code block: \n'+ 'Here\'s a code block: \n'+
'<div class="styling-directive">```</div><code class="block">Inside the code-block, &lt;code&gt;hello&lt;/code&gt; we don\'t enable *styling hints* like ~these~\n'+ '<div class="styling-directive">```</div><code class="block">Inside the code-block, &lt;code&gt;hello&lt;/code&gt; we don\'t enable *styling hints* like ~these~\n'+
@ -218,7 +216,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 2); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 2);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<div class="styling-directive">```</div>'+ '<div class="styling-directive">```</div>'+
'<code class="block">ignored\n(println "Hello, world!")\n</code>'+ '<code class="block">ignored\n(println "Hello, world!")\n</code>'+
@ -254,7 +251,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 1); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 1);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>This is quoted text\nThis is also quoted</blockquote>\nThis is not quoted'); '<blockquote>This is quoted text\nThis is also quoted</blockquote>\nThis is not quoted');
@ -263,7 +259,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 2); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 2);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>This is <span class="styling-directive">*</span><b>quoted</b><span class="styling-directive">*</span> text\n'+ '<blockquote>This is <span class="styling-directive">*</span><b>quoted</b><span class="styling-directive">*</span> text\n'+
'This is <span class="styling-directive">`</span><code>also _quoted_</code><span class="styling-directive">`</span></blockquote>\n'+ 'This is <span class="styling-directive">`</span><code>also _quoted_</code><span class="styling-directive">`</span></blockquote>\n'+
@ -274,23 +269,20 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 3); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 3);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text); await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === "<blockquote><blockquote>This is doubly quoted text</blockquote></blockquote>");
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === "<blockquote><blockquote>This is doubly quoted text</blockquote></blockquote>");
msg_text = `>> This is doubly quoted text`; msg_text = `>> This is doubly quoted text`;
msg = mock.createChatMessage(_converse, contact_jid, msg_text) msg = mock.createChatMessage(_converse, contact_jid, msg_text)
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 4); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 4);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text); await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === "<blockquote><blockquote>This is doubly quoted text</blockquote></blockquote>");
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === "<blockquote><blockquote>This is doubly quoted text</blockquote></blockquote>");
msg_text = ">```\n>ignored\n> <span></span> (println \"Hello, world!\")\n>```\n> This should show up as monospace, preformatted text ^"; msg_text = ">```\n>ignored\n> <span></span> (println \"Hello, world!\")\n>```\n> This should show up as monospace, preformatted text ^";
msg = mock.createChatMessage(_converse, contact_jid, msg_text) msg = mock.createChatMessage(_converse, contact_jid, msg_text)
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 5); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 5);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>'+ '<blockquote>'+
'<div class="styling-directive">```</div>'+ '<div class="styling-directive">```</div>'+
@ -304,7 +296,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 6); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 6);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>```\n (println "Hello, world!")</blockquote>\n\n'+ '<blockquote>```\n (println "Hello, world!")</blockquote>\n\n'+
'The entire blockquote is a preformatted text block, but this line is plaintext!'); 'The entire blockquote is a preformatted text block, but this line is plaintext!');
@ -314,7 +305,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 7); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 7);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>Also, icons.js is loaded from /dist, instead of dist.</blockquote>\n'+ '<blockquote>Also, icons.js is loaded from /dist, instead of dist.</blockquote>\n'+
'<a target="_blank" rel="noopener" href="https://conversejs.org/docs/html/configuration.html#assets-path">https://conversejs.org/docs/html/configuration.html#assets-path</a>'); '<a target="_blank" rel="noopener" href="https://conversejs.org/docs/html/configuration.html#assets-path">https://conversejs.org/docs/html/configuration.html#assets-path</a>');
@ -324,7 +314,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 8); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 8);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>Where is it located?</blockquote>\n'+ '<blockquote>Where is it located?</blockquote>\n'+
'<a target="_blank" rel="noopener" '+ '<a target="_blank" rel="noopener" '+
@ -335,7 +324,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 9); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 9);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>What do you think of it?</blockquote>\n <span title=":poop:">💩</span>'); '<blockquote>What do you think of it?</blockquote>\n <span title=":poop:">💩</span>');
@ -344,7 +332,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 10); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 10);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
'<blockquote>What do you think of it?</blockquote>\n<span class="styling-directive">~</span><del>hello</del><span class="styling-directive">~</span>'); '<blockquote>What do you think of it?</blockquote>\n<span class="styling-directive">~</span><del>hello</del><span class="styling-directive">~</span>');
@ -353,7 +340,6 @@ describe("An incoming chat Message", function () {
await _converse.handleMessageStanza(msg); await _converse.handleMessageStanza(msg);
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 11); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 11);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === 'hello world &gt; this is not a quote'); await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === 'hello world &gt; this is not a quote');
msg_text = '> What do you think of it romeo?\n Did you see this romeo?'; msg_text = '> What do you think of it romeo?\n Did you see this romeo?';
@ -381,15 +367,16 @@ describe("An incoming chat Message", function () {
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 12); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length === 12);
msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') ===
`<blockquote>What do you think of it <span class="mention">romeo</span>?</blockquote>\n Did you see this <span class="mention">romeo</span>?`); `<blockquote>What do you think of it <span class="mention">romeo</span>?</blockquote>\n Did you see this <span class="mention">romeo</span>?`);
expect(true).toBe(true);
done(); done();
})); }));
it("won't style invalid block quotes", it("won't style invalid block quotes",
mock.initConverse(['chatBoxesFetched'], {}, mock.initConverse(['chatBoxesFetched'], {},
async function (done, _converse) { async function (done, _converse) {
await mock.waitForRoster(_converse, 'current', 1); await mock.waitForRoster(_converse, 'current', 1);
const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit'; const contact_jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
@ -420,8 +407,8 @@ describe("An incoming chat Message", function () {
await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length); await u.waitUntil(() => view.querySelectorAll('.chat-msg__text').length);
const msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop(); const msg_el = Array.from(view.querySelectorAll('converse-chat-message-body')).pop();
expect(msg_el.innerText).toBe(msg_text);
await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === '```\ncode```'); await u.waitUntil(() => msg_el.innerHTML.replace(/<!-.*?->/g, '') === '```\ncode```');
expect(true).toBe(true);
done(); done();
})); }));
}); });

View File

@ -8,7 +8,8 @@ import { html } from 'lit';
import { renderStylingDirectiveBody } from 'shared/directives/styling.js'; import { renderStylingDirectiveBody } from 'shared/directives/styling.js';
const styling_directives = ['*', '_', '~', '`', '```', '>']; const bracketing_directives = ['*', '_', '~', '`'];
const styling_directives = [...bracketing_directives, '```', '>'];
const styling_map = { const styling_map = {
'*': {'name': 'strong', 'type': 'span'}, '*': {'name': 'strong', 'type': 'span'},
'_': {'name': 'emphasis', 'type': 'span'}, '_': {'name': 'emphasis', 'type': 'span'},
@ -51,8 +52,8 @@ function isValidDirective (d, text, i, opening) {
if (is_quote && i > 0 && text[i-1] !== '\n') { if (is_quote && i > 0 && text[i-1] !== '\n') {
// Quote directives must be on newlines // Quote directives must be on newlines
return false; return false;
} else if (!is_quote && d === text[i+1]) { } else if (bracketing_directives.includes(d) && (text[i+1] === d)) {
// Immediately followed by another directive of the same type // Don't consider empty bracketing directives as valid (e.g. **, `` etc.)
return false; return false;
} }
} else { } else {
@ -60,6 +61,10 @@ function isValidDirective (d, text, i, opening) {
if (i < text.length-1 && regex.test(text.slice(i))) { if (i < text.length-1 && regex.test(text.slice(i))) {
return false; return false;
} }
if (bracketing_directives.includes(d) && (text[i-1] === d)) {
// Don't consider empty directives as valid (e.g. **, `` etc.)
return false;
}
} }
return true; return true;
} }
@ -84,20 +89,6 @@ function getDirective (text, i, opening=true) {
return d; return d;
} }
/**
* Given an opening directive "d", an index "i" and the text, check whether
* we've found the closing directive.
* @param { String } d -The directive
* @param { Number } i - The directive index
* @param { String } text -The text in which the directive appears
*/
function isDirectiveEnd (d, i, text) {
const dtype = styling_map[d].type; // directive type
return i === text.length || getDirective(text, i, false) === d || (dtype === 'span' && text[i] === '\n');
}
/** /**
* Given a directive "d", which occurs in "text" at index "i", check that it * Given a directive "d", which occurs in "text" at index "i", check that it
* has a valid closing directive and return the length from start to end of the * has a valid closing directive and return the length from start to end of the
@ -114,20 +105,25 @@ function getDirectiveLength (d, text, i) {
i += text.slice(i).split(/\n[^>]/).shift().length; i += text.slice(i).split(/\n[^>]/).shift().length;
return i-begin; return i-begin;
} else if (styling_map[d].type === 'span') { } else if (styling_map[d].type === 'span') {
const line = text.slice(i+1).split('\n').shift(); const line = text.slice(i).split('\n').shift();
let j = 0; let j = 0;
let idx = line.indexOf(d); let idx = line.indexOf(d);
while (idx !== -1) { while (idx !== -1) {
if (isDirectiveEnd(d, i+1+idx, text)) return idx+1+2*d.length; if (getDirective(text, i+idx, false) === d) {
return idx+2*d.length;
}
idx = line.indexOf(d, j++); idx = line.indexOf(d, j++);
} }
return 0; return 0;
} else { } else {
// block directives
const substring = text.slice(i+1); const substring = text.slice(i+1);
let j = 0; let j = 0;
let idx = substring.indexOf(d); let idx = substring.indexOf(d);
while (idx !== -1) { while (idx !== -1) {
if (isDirectiveEnd(d, i+1+idx, text)) return idx+1+2*d.length; if (getDirective(text, i+1+idx, false) === d) {
return idx+1+2*d.length;
}
idx = substring.indexOf(d, j++); idx = substring.indexOf(d, j++);
} }
return 0; return 0;