Enforce correct order for previews on server-side prefectch rather than at client parsing

This has the benefit of not adding `.preview` divs everywhere, anytime we use `parse()`, and also to un-tie the position of the preview blocks from the result of the helper. This means that templates that call `parse` and have some extra markup after that are not constrained anymore.

This is effectively an alternative, better way to fix https://github.com/thelounge/lounge/issues/1343, but the initial fix that was put in place (https://github.com/thelounge/lounge/pull/1347) is still relevant, for example to make sure a preview stays hidden (and does not add extra margin/padding/etc.) if the link does not prefetch.

client/js/libs/handlebars/parse.js

@@ -81,8 +81,5 @@ module.exports = function parse(text) {
 		}
 
 		return fragments;
-	}).join("") + linkParts.map((part) => {
-		const escapedLink = Handlebars.Utils.escapeExpression(part.link);
-		return `<div class="preview" data-url="${escapedLink}"></div>`;
 	}).join("");
 };

client/views/actions/action.tpl

@@ -1,2 +1,6 @@
 {{> ../user_name nick=from}}
 <span class="action-text">{{{parse text}}}</span>
+
+{{#each links}}
+  <div class="preview" data-url="{{this}}"></div>
+{{/each}}

client/views/msg.tpl

@@ -7,5 +7,11 @@
 			{{> user_name nick=from}}
 		{{/if}}
 	</span>
-	<span class="text">{{{parse text}}}</span>
+	<span class="text">
+		{{{parse text}}}
+
+		{{#each links}}
+			<div class="preview" data-url="{{this}}"></div>
+		{{/each}}
+	</span>
 </div>

src/models/msg.js

@@ -31,6 +31,7 @@ function Msg(attr) {
 	_.defaults(this, attr, {
 		from: "",
 		id: id++,
+		links: [],
 		previews: [],
 		text: "",
 		type: Msg.Type.MESSAGE,

src/plugins/irc-events/link.js

@@ -24,19 +24,19 @@ module.exports = function(client, chan, msg) {
 		return;
 	}
 
-	Array.from(new Set( // Remove duplicate links
+	msg.links = Array.from(new Set( // Remove duplicate links
 		links.map((link) => escapeHeader(link.link))
-	))
+	)).slice(0, 5); // Only preview the first 5 URLs in message to avoid abuse
-		.slice(0, 5) // Only preview the first 5 URLs in message to avoid abuse
-		.forEach((link) => {
-			fetch(link, function(res) {
-				if (res === null) {
-					return;
-				}
 
-				parse(msg, link, res, client);
+	msg.links.forEach((link) => {
-			});
+		fetch(link, function(res) {
+			if (res === null) {
+				return;
+			}
+
+			parse(msg, link, res, client);
 		});
+	});
 };
 
 function parse(msg, url, res, client) {

src/plugins/irc-events/message.js

@@ -89,11 +89,12 @@ module.exports = function(irc, network) {
 			self: self,
 			highlight: highlight
 		});
-		chan.pushMessage(client, msg, !self);
 
 		// No prefetch URLs unless are simple MESSAGE or ACTION types
 		if ([Msg.Type.MESSAGE, Msg.Type.ACTION].indexOf(data.type) !== -1) {
 			LinkPrefetch(client, chan, msg);
 		}
+
+		chan.pushMessage(client, msg, !self);
 	}
 };

test/client/js/libs/handlebars/parse.js

@@ -37,15 +37,13 @@ describe("parse Handlebars helper", () => {
 			expected:
 				"<a href=\"irc://freenode.net/thelounge\" target=\"_blank\" rel=\"noopener\">" +
 					"irc://freenode.net/thelounge" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"irc://freenode.net/thelounge\"></div>"
 		}, {
 			input: "www.nooooooooooooooo.com",
 			expected:
 				"<a href=\"http://www.nooooooooooooooo.com\" target=\"_blank\" rel=\"noopener\">" +
 					"www.nooooooooooooooo.com" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"http://www.nooooooooooooooo.com\"></div>"
 		}, {
 			input: "look at https://thelounge.github.io/ for more information",
 			expected:
@@ -53,8 +51,7 @@ describe("parse Handlebars helper", () => {
 				"<a href=\"https://thelounge.github.io/\" target=\"_blank\" rel=\"noopener\">" +
 					"https://thelounge.github.io/" +
 				"</a>" +
-				" for more information" +
+				" for more information"
-				"<div class=\"preview\" data-url=\"https://thelounge.github.io/\"></div>"
 		}, {
 			input: "use www.duckduckgo.com for privacy reasons",
 			expected:
@@ -62,26 +59,13 @@ describe("parse Handlebars helper", () => {
 				"<a href=\"http://www.duckduckgo.com\" target=\"_blank\" rel=\"noopener\">" +
 					"www.duckduckgo.com" +
 				"</a>" +
-				" for privacy reasons" +
+				" for privacy reasons"
-				"<div class=\"preview\" data-url=\"http://www.duckduckgo.com\"></div>"
 		}, {
 			input: "svn+ssh://example.org",
 			expected:
 				"<a href=\"svn+ssh://example.org\" target=\"_blank\" rel=\"noopener\">" +
 					"svn+ssh://example.org" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"svn+ssh://example.org\"></div>"
-		}, {
-			input: "https://example.com https://example.org",
-			expected:
-				"<a href=\"https://example.com\" target=\"_blank\" rel=\"noopener\">" +
-					"https://example.com" +
-				"</a> " +
-				"<a href=\"https://example.org\" target=\"_blank\" rel=\"noopener\">" +
-					"https://example.org" +
-				"</a>" +
-				"<div class=\"preview\" data-url=\"https://example.com\"></div>" +
-				"<div class=\"preview\" data-url=\"https://example.org\"></div>"
 		}];
 
 		const actual = testCases.map((testCase) => parse(testCase.input));
@@ -97,8 +81,7 @@ describe("parse Handlebars helper", () => {
 				"bonuspunkt: your URL parser misparses this URL: " +
 				"<a href=\"https://msdn.microsoft.com/en-us/library/windows/desktop/ms644989(v&#x3D;vs.85).aspx\" target=\"_blank\" rel=\"noopener\">" +
 					"https://msdn.microsoft.com/en-us/library/windows/desktop/ms644989(v&#x3D;vs.85).aspx" +
-				"</a>" +
+				"</a>";
-				"<div class=\"preview\" data-url=\"https://msdn.microsoft.com/en-us/library/windows/desktop/ms644989(v&#x3D;vs.85).aspx\"></div>";
 
 		const actual = parse(input);
 
@@ -113,8 +96,7 @@ describe("parse Handlebars helper", () => {
 				"<a href=\"https://theos.kyriasis.com/~kyrias/stats/archlinux.html\" target=\"_blank\" rel=\"noopener\">" +
 					"https://theos.kyriasis.com/~kyrias/stats/archlinux.html" +
 				"</a>" +
-				"&gt;" +
+				"&gt;"
-				"<div class=\"preview\" data-url=\"https://theos.kyriasis.com/~kyrias/stats/archlinux.html\"></div>"
 		}, {
 			input: "abc (www.example.com)",
 			expected:
@@ -122,22 +104,19 @@ describe("parse Handlebars helper", () => {
 				"<a href=\"http://www.example.com\" target=\"_blank\" rel=\"noopener\">" +
 					"www.example.com" +
 				"</a>" +
-				")" +
+				")"
-				"<div class=\"preview\" data-url=\"http://www.example.com\"></div>"
 		}, {
 			input: "http://example.com/Test_(Page)",
 			expected:
 				"<a href=\"http://example.com/Test_(Page)\" target=\"_blank\" rel=\"noopener\">" +
 					"http://example.com/Test_(Page)" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"http://example.com/Test_(Page)\"></div>"
 		}, {
 			input: "www.example.com/Test_(Page)",
 			expected:
 				"<a href=\"http://www.example.com/Test_(Page)\" target=\"_blank\" rel=\"noopener\">" +
 					"www.example.com/Test_(Page)" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"http://www.example.com/Test_(Page)\"></div>"
 		}];
 
 		const actual = testCases.map((testCase) => parse(testCase.input));
@@ -274,8 +253,7 @@ describe("parse Handlebars helper", () => {
 					"<span class=\"irc-italic\">freenode.net</span>" +
 					"/" +
 					"<span class=\"irc-fg4 irc-bg8\">thelounge</span>" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"irc://freenode.net/thelounge\"></div>"
 		}, {
 			input: "\x02#\x038,9thelounge",
 			expected:
@@ -314,16 +292,14 @@ describe("parse Handlebars helper", () => {
 				"like.." +
 				"<a href=\"http://example.com\" target=\"_blank\" rel=\"noopener\">" +
 					"http://example.com" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"http://example.com\"></div>"
 		}, {
 			input: "like..HTTP://example.com",
 			expected:
 				"like.." +
 				"<a href=\"HTTP://example.com\" target=\"_blank\" rel=\"noopener\">" +
 					"HTTP://example.com" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"HTTP://example.com\"></div>"
 		}];
 
 		const actual = testCases.map((testCase) => parse(testCase.input));
@@ -339,8 +315,7 @@ describe("parse Handlebars helper", () => {
 				"" +
 				"<a href=\"http://example.com/#hash\" target=\"_blank\" rel=\"noopener\">" +
 					"http://example.com/#hash" +
-				"</a>" +
+				"</a>"
-				"<div class=\"preview\" data-url=\"http://example.com/#hash\"></div>"
 		}];
 
 		const actual = testCases.map((testCase) => parse(testCase.input));
@@ -355,8 +330,7 @@ describe("parse Handlebars helper", () => {
 
 		expect(actual).to.equal(
 			"Url: <a href=\"http://example.com/path\" target=\"_blank\" rel=\"noopener\">http://example.com/path</a> " +
-			"Channel: <span class=\"inline-channel\" role=\"button\" tabindex=\"0\" data-chan=\"##channel\">##channel</span>" +
+			"Channel: <span class=\"inline-channel\" role=\"button\" tabindex=\"0\" data-chan=\"##channel\">##channel</span>"
-			"<div class=\"preview\" data-url=\"http://example.com/path\"></div>"
 		);
 	});
 });

test/plugins/link.js

@@ -27,8 +27,9 @@ describe("Link plugin", function() {
 	});
 
 	it("should be able to fetch basic information about URLs", function(done) {
+		const url = "http://localhost:9002/basic";
 		const message = this.irc.createMessage({
-			text: "http://localhost:9002/basic"
+			text: url
 		});
 
 		link(this.irc, this.network.channels[0], message);
@@ -41,8 +42,10 @@ describe("Link plugin", function() {
 			expect(data.preview.type).to.equal("link");
 			expect(data.preview.head).to.equal("test title");
 			expect(data.preview.body).to.equal("simple description");
-			expect(data.preview.link).to.equal("http://localhost:9002/basic");
+			expect(data.preview.link).to.equal(url);
-			expect(message.previews.length).to.equal(1);
+
+			expect(message.links).to.deep.equal([url]);
+			expect(message.previews).to.deep.equal([data.preview]);
 			done();
 		});
 	});
@@ -178,6 +181,11 @@ describe("Link plugin", function() {
 
 		link(this.irc, this.network.channels[0], message);
 
+		expect(message.links).to.deep.equal([
+			"http://localhost:9002/one",
+			"http://localhost:9002/two"
+		]);
+
 		this.app.get("/one", function(req, res) {
 			res.send("<title>first title</title>");
 		});
@@ -186,22 +194,18 @@ describe("Link plugin", function() {
 			res.send("<title>second title</title>");
 		});
 
-		const loaded = {
+		const previews = [];
-			one: false,
-			two: false
-		};
 
 		this.irc.on("msg:preview", function(data) {
 			if (data.preview.link === "http://localhost:9002/one") {
 				expect(data.preview.head).to.equal("first title");
-				loaded.one = true;
 			} else if (data.preview.link === "http://localhost:9002/two") {
 				expect(data.preview.head).to.equal("second title");
-				loaded.two = true;
 			}
+			previews.push(data.preview);
 
-			if (loaded.one && loaded.two) {
+			if (previews.length === 2) {
-				expect(message.previews.length).to.equal(2);
+				expect(message.previews).to.deep.equal(previews);
 				done();
 			}
 		});

test/util.js

@@ -21,6 +21,7 @@ MockClient.prototype.createMessage = function(opts) {
 		text: "dummy message",
 		nick: "test-user",
 		target: "#test-channel",
+		links: [],
 		previews: [],
 	}, opts);