Support multiple previews per message

- Load up to 5 previews per message (to avoid abuse)
- Do not load multiple times the same URL
- Prepare preview containers per message instead of appending (to maintain correct order)
- Store an array of previews instead of a single preview in `Msg` objects
- Consolidate preview rendering for new messages and upon refresh/load history (when rendering entire channels)
- Update `parse` tests to reflect previous point
- Add test for multiple URLs
- Switch preview tests from `assert` API to `expect` API
This commit is contained in:
Jérémie Astori 2017-07-06 02:16:01 -04:00
parent c2f2c69e91
commit 64ebe0f437
No known key found for this signature in database
GPG Key ID: B9A4F245CD67BDE8
10 changed files with 185 additions and 91 deletions

View File

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

View File

@ -3,6 +3,7 @@
const $ = require("jquery"); const $ = require("jquery");
const templates = require("../views"); const templates = require("../views");
const options = require("./options"); const options = require("./options");
const renderPreview = require("./renderPreview");
const utils = require("./utils"); const utils = require("./utils");
const sorting = require("./sorting"); const sorting = require("./sorting");
@ -15,7 +16,7 @@ module.exports = {
renderChannel, renderChannel,
renderChannelMessages, renderChannelMessages,
renderChannelUsers, renderChannelUsers,
renderNetworks renderNetworks,
}; };
function buildChannelMessages(channel, messages) { function buildChannelMessages(channel, messages) {
@ -35,9 +36,9 @@ function buildChatMessage(data) {
target = "#chan-" + chat.find(".active").data("id"); target = "#chan-" + chat.find(".active").data("id");
} }
if (data.msg.preview) { data.msg.previews.forEach((preview) => {
data.msg.preview.shown = options.shouldOpenMessagePreview(data.msg.preview.type); preview.shown = options.shouldOpenMessagePreview(preview.type);
} });
const chan = chat.find(target); const chan = chat.find(target);
let template = "msg"; let template = "msg";
@ -72,6 +73,12 @@ function buildChatMessage(data) {
const msg = $(templates[template](data.msg)); const msg = $(templates[template](data.msg));
const text = msg.find(".text"); const text = msg.find(".text");
if (data.msg.previews.length) {
data.msg.previews.forEach((preview) => {
renderPreview(preview, msg);
});
}
if (template === "msg_action") { if (template === "msg_action") {
text.html(templates.actions[type](data.msg)); text.html(templates.actions[type](data.msg));
} }

View File

@ -0,0 +1,57 @@
"use strict";
const $ = require("jquery");
const options = require("./options");
const templates = require("../views");
module.exports = renderPreview;
function renderPreview(preview, msg) {
preview.shown = options.shouldOpenMessagePreview(preview.type);
const container = msg.closest(".chat");
let bottom = false;
if (container.length) {
bottom = container.isScrollBottom();
}
msg.find(`[data-url="${preview.link}"]`)
.first()
.append(templates.msg_preview({preview: preview}));
if (preview.shown && bottom) {
handleImageInPreview(msg.find(".toggle-content"), container);
}
container.trigger("keepToBottom");
}
$("#chat").on("click", ".toggle-button", function() {
const self = $(this);
const container = self.closest(".chat");
const content = self.parent().next(".toggle-content");
const bottom = container.isScrollBottom();
if (bottom && !content.hasClass("show")) {
handleImageInPreview(content, container);
}
content.toggleClass("show");
// If scrollbar was at the bottom before toggling the preview, keep it at the bottom
if (bottom) {
container.scrollBottom();
}
});
function handleImageInPreview(content, container) {
const img = content.find("img");
// Trigger scroll logic after the image loads
if (img.length && !img.width()) {
img.on("load", function() {
container.trigger("keepToBottom");
});
}
}

View File

@ -1,51 +1,13 @@
"use strict"; "use strict";
const $ = require("jquery"); const $ = require("jquery");
const Handlebars = require("handlebars/runtime");
const renderPreview = require("../renderPreview");
const socket = require("../socket"); const socket = require("../socket");
const templates = require("../../views");
const options = require("../options");
socket.on("msg:preview", function(data) { socket.on("msg:preview", function(data) {
data.preview.shown = options.shouldOpenMessagePreview(data.preview.type);
const msg = $("#msg-" + data.id); const msg = $("#msg-" + data.id);
const container = msg.closest(".chat");
const bottom = container.isScrollBottom();
msg.find(".text").append(templates.msg_preview({preview: data.preview})); data.link = Handlebars.Utils.escapeExpression(data.link);
renderPreview(data.preview, msg);
if (data.preview.shown && bottom) {
handleImageInPreview(msg.find(".toggle-content"), container);
}
container.trigger("keepToBottom");
}); });
$("#chat").on("click", ".toggle-button", function() {
const self = $(this);
const container = self.closest(".chat");
const content = self.parent().next(".toggle-content");
const bottom = container.isScrollBottom();
if (bottom && !content.hasClass("show")) {
handleImageInPreview(content, container);
}
content.toggleClass("show");
// If scrollbar was at the bottom before toggling the preview, keep it at the bottom
if (bottom) {
container.scrollBottom();
}
});
function handleImageInPreview(content, container) {
const img = content.find("img");
// Trigger scroll logic after the image loads
if (img.length && !img.width()) {
img.on("load", function() {
container.trigger("keepToBottom");
});
}
}

View File

@ -7,10 +7,5 @@
{{> user_name nick=from}} {{> user_name nick=from}}
{{/if}} {{/if}}
</span> </span>
<span class="text"> <span class="text">{{{parse text}}}</span>
{{~{parse text}~}}
{{#if preview}}
{{> msg_preview}}
{{/if}}
</span>
</div> </div>

View File

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

View File

@ -23,7 +23,11 @@ module.exports = function(client, chan, msg) {
return; return;
} }
const link = escapeHeader(links[0].link); 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
.forEach((link) => {
fetch(link, function(res) { fetch(link, function(res) {
if (res === null) { if (res === null) {
return; return;
@ -31,6 +35,7 @@ module.exports = function(client, chan, msg) {
parse(msg, link, res, client); parse(msg, link, res, client);
}); });
});
}; };
function parse(msg, url, res, client) { function parse(msg, url, res, client) {
@ -110,7 +115,7 @@ function emitPreview(client, msg, preview) {
} }
} }
msg.preview = preview; msg.previews.push(preview);
client.emit("msg:preview", { client.emit("msg:preview", {
id: msg.id, id: msg.id,

View File

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

View File

@ -1,6 +1,6 @@
"use strict"; "use strict";
var assert = require("assert"); const expect = require("chai").expect;
var util = require("../util"); var util = require("../util");
var link = require("../../src/plugins/irc-events/link.js"); var link = require("../../src/plugins/irc-events/link.js");
@ -36,9 +36,10 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.type, "link"); expect(data.preview.type).to.equal("link");
assert.equal(data.preview.head, "test title"); expect(data.preview.head).to.equal("test title");
assert.equal(data.preview.body, "simple description"); expect(data.preview.body).to.equal("simple description");
expect(message.previews.length).to.equal(1);
done(); done();
}); });
}); });
@ -55,7 +56,7 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.head, "opengraph test"); expect(data.preview.head, "opengraph test");
done(); done();
}); });
}); });
@ -72,7 +73,7 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.body, "opengraph description"); expect(data.preview.body).to.equal("opengraph description");
done(); done();
}); });
}); });
@ -89,8 +90,8 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.head, "Google"); expect(data.preview.head).to.equal("Google");
assert.equal(data.preview.thumb, "http://localhost:9002/real-test-image.png"); expect(data.preview.thumb).to.equal("http://localhost:9002/real-test-image.png");
done(); done();
}); });
}); });
@ -107,7 +108,7 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.thumb, ""); expect(data.preview.thumb).to.be.empty;
done(); done();
}); });
}); });
@ -124,8 +125,8 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.head, "Untitled page"); expect(data.preview.head).to.equal("Untitled page");
assert.equal(data.preview.thumb, "http://localhost:9002/real-test-image.png"); expect(data.preview.thumb).to.equal("http://localhost:9002/real-test-image.png");
done(); done();
}); });
}); });
@ -142,8 +143,8 @@ describe("Link plugin", function() {
}); });
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.head, "404 image"); expect(data.preview.head).to.equal("404 image");
assert.equal(data.preview.thumb, ""); expect(data.preview.thumb).to.be.empty;
done(); done();
}); });
}); });
@ -156,9 +157,45 @@ describe("Link plugin", function() {
link(this.irc, this.network.channels[0], message); link(this.irc, this.network.channels[0], message);
this.irc.once("msg:preview", function(data) { this.irc.once("msg:preview", function(data) {
assert.equal(data.preview.type, "image"); expect(data.preview.type).to.equal("image");
assert.equal(data.preview.link, "http://localhost:9002/real-test-image.png"); expect(data.preview.link).to.equal("http://localhost:9002/real-test-image.png");
done(); done();
}); });
}); });
it("should load multiple URLs found in messages", function(done) {
const message = this.irc.createMessage({
text: "http://localhost:9002/one http://localhost:9002/two"
});
link(this.irc, this.network.channels[0], message);
this.app.get("/one", function(req, res) {
res.send("<title>first title</title>");
});
this.app.get("/two", function(req, res) {
res.send("<title>second title</title>");
});
const loaded = {
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;
}
if (loaded.one && loaded.two) {
expect(message.previews.length).to.equal(2);
done();
}
});
});
}); });

View File

@ -20,7 +20,8 @@ MockClient.prototype.createMessage = function(opts) {
var message = _.extend({ var message = _.extend({
text: "dummy message", text: "dummy message",
nick: "test-user", nick: "test-user",
target: "#test-channel" target: "#test-channel",
previews: [],
}, opts); }, opts);
return message; return message;