Merge pull request #1353 from thelounge/astorije/better-preview-ordering

Enforce correct order for previews on server-side prefectch rather than at client parsing
This commit is contained in:
Pavel Djundik 2017-07-21 11:12:37 +03:00 committed by GitHub
commit f642a3c776
12 changed files with 100 additions and 97 deletions

View File

@ -943,7 +943,7 @@ kbd {
#chat .time, #chat .time,
#chat .from, #chat .from,
#chat .text { #chat .content {
display: block; display: block;
padding: 2px 0; padding: 2px 0;
flex: 0 0 auto; flex: 0 0 auto;
@ -965,7 +965,7 @@ kbd {
position: relative; position: relative;
} }
#chat .text { #chat .content {
flex: 1 1 auto; flex: 1 1 auto;
overflow: hidden; overflow: hidden;
} }
@ -1024,7 +1024,7 @@ kbd {
#chat.colored-nicks .user.color-31 { color: #ff4846; } #chat.colored-nicks .user.color-31 { color: #ff4846; }
#chat.colored-nicks .user.color-32 { color: #ff199b; } #chat.colored-nicks .user.color-32 { color: #ff199b; }
#chat .text { #chat .content {
padding-left: 10px; padding-left: 10px;
padding-right: 6px; padding-right: 6px;
} }
@ -1093,14 +1093,14 @@ kbd {
display: none !important; display: none !important;
} }
#chat .join .text, #chat .join .content,
#chat .kick .text, #chat .kick .content,
#chat .mode .text, #chat .mode .content,
#chat .nick .text, #chat .nick .content,
#chat .part .text, #chat .part .content,
#chat .quit .text, #chat .quit .content,
#chat .topic .text, #chat .topic .content,
#chat .topic_set_by .text { #chat .topic_set_by .content {
color: #999; color: #999;
} }
@ -1978,7 +1978,7 @@ part/quit messages where we don't load previews (adds a blank line otherwise) */
#chat .time, #chat .time,
#chat .from, #chat .from,
#chat .text { #chat .content {
border: 0; border: 0;
display: inline; display: inline;
padding: 0; padding: 0;
@ -2097,7 +2097,7 @@ part/quit messages where we don't load previews (adds a blank line otherwise) */
#chat .part-reason, #chat .part-reason,
#chat .quit-reason, #chat .quit-reason,
#chat .new-topic, #chat .new-topic,
#chat .action-text, #chat .action .text,
#chat table.channel-list .topic { #chat table.channel-list .topic {
white-space: pre-wrap; white-space: pre-wrap;
} }

View File

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

View File

@ -71,10 +71,10 @@ function buildChatMessage(data) {
} }
const msg = $(templates[template](data.msg)); const msg = $(templates[template](data.msg));
const text = msg.find(".text"); const content = msg.find(".content");
if (template === "msg_action") { if (template === "msg_action") {
text.html(templates.actions[type](data.msg)); content.html(templates.actions[type](data.msg));
} }
data.msg.previews.forEach((preview) => { data.msg.previews.forEach((preview) => {

View File

@ -8,6 +8,10 @@ const templates = require("../views");
module.exports = renderPreview; module.exports = renderPreview;
function renderPreview(preview, msg) { function renderPreview(preview, msg) {
if (preview.type === "loading") {
return;
}
preview.shown = options.shouldOpenMessagePreview(preview.type); preview.shown = options.shouldOpenMessagePreview(preview.type);
const container = msg.closest(".chat"); const container = msg.closest(".chat");
@ -34,7 +38,7 @@ function renderPreview(preview, msg) {
$("#chat").on("click", ".toggle-button", function() { $("#chat").on("click", ".toggle-button", function() {
const self = $(this); const self = $(this);
const container = self.closest(".chat"); const container = self.closest(".chat");
const content = self.closest(".text") const content = self.closest(".content")
.find(`.preview[data-url="${self.data("url")}"] .toggle-content`); .find(`.preview[data-url="${self.data("url")}"] .toggle-content`);
const bottom = container.isScrollBottom(); const bottom = container.isScrollBottom();

View File

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

View File

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

View File

@ -3,5 +3,5 @@
{{tz time}} {{tz time}}
</span> </span>
<span class="from"></span> <span class="from"></span>
<span class="text"></span> <span class="content"></span>
</div> </div>

View File

@ -3,7 +3,7 @@
{{tz time}} {{tz time}}
</span> </span>
<span class="from">[{{command}}]</span> <span class="from">[{{command}}]</span>
<span class="text"> <span class="content">
{{#each params}} {{#each params}}
<span>{{this}}</span> <span>{{this}}</span>
{{/each}} {{/each}}

View File

@ -24,30 +24,28 @@ module.exports = function(client, chan, msg) {
return; return;
} }
Array.from(new Set( // Remove duplicate links msg.previews = Array.from(new Set( // Remove duplicate links
links.map((link) => escapeHeader(link.link)) links.map((link) => escapeHeader(link.link))
)) )).map((link) => ({
.slice(0, 5) // Only preview the first 5 URLs in message to avoid abuse type: "loading",
.forEach((link) => { head: "",
fetch(link, function(res) { body: "",
thumb: "",
link: link,
})).slice(0, 5); // Only preview the first 5 URLs in message to avoid abuse
msg.previews.forEach((preview) => {
fetch(preview.link, function(res) {
if (res === null) { if (res === null) {
return; return;
} }
parse(msg, link, res, client); parse(msg, preview, res, client);
}); });
}); });
}; };
function parse(msg, url, res, client) { function parse(msg, preview, res, client) {
const preview = {
type: "",
head: "",
body: "",
thumb: "",
link: url,
};
switch (res.type) { switch (res.type) {
case "text/html": case "text/html":
var $ = cheerio.load(res.data); var $ = cheerio.load(res.data);
@ -130,8 +128,6 @@ function emitPreview(client, msg, preview) {
} }
} }
msg.previews.push(preview);
client.emit("msg:preview", { client.emit("msg:preview", {
id: msg.id, id: msg.id,
preview: preview preview: preview

View File

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

View File

@ -37,15 +37,13 @@ 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 class=\"preview\" 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 class=\"preview\" 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:
@ -53,8 +51,7 @@ 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 class=\"preview\" 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:
@ -62,26 +59,13 @@ 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 class=\"preview\" 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 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)); const actual = testCases.map((testCase) => parse(testCase.input));
@ -97,8 +81,7 @@ 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 class=\"preview\" 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);
@ -113,8 +96,7 @@ 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 class=\"preview\" data-url=\"https://theos.kyriasis.com/~kyrias/stats/archlinux.html\"></div>"
}, { }, {
input: "abc (www.example.com)", input: "abc (www.example.com)",
expected: expected:
@ -122,22 +104,19 @@ 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 class=\"preview\" 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 class=\"preview\" 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 class=\"preview\" 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));
@ -274,8 +253,7 @@ 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 class=\"preview\" data-url=\"irc://freenode.net/thelounge\"></div>"
}, { }, {
input: "\x02#\x038,9thelounge", input: "\x02#\x038,9thelounge",
expected: expected:
@ -314,16 +292,14 @@ 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 class=\"preview\" 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 class=\"preview\" data-url=\"HTTP://example.com\"></div>"
}]; }];
const actual = testCases.map((testCase) => parse(testCase.input)); 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\">" + "<a href=\"http://example.com/#hash\" target=\"_blank\" rel=\"noopener\">" +
"http://example.com/#hash" + "http://example.com/#hash" +
"</a>" + "</a>"
"<div class=\"preview\" data-url=\"http://example.com/#hash\"></div>"
}]; }];
const actual = testCases.map((testCase) => parse(testCase.input)); const actual = testCases.map((testCase) => parse(testCase.input));
@ -355,8 +330,7 @@ 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 class=\"preview\" data-url=\"http://example.com/path\"></div>"
); );
}); });
}); });

View File

@ -27,12 +27,21 @@ describe("Link plugin", function() {
}); });
it("should be able to fetch basic information about URLs", function(done) { it("should be able to fetch basic information about URLs", function(done) {
const url = "http://localhost:9002/basic";
const message = this.irc.createMessage({ const message = this.irc.createMessage({
text: "http://localhost:9002/basic" text: url
}); });
link(this.irc, this.network.channels[0], message); link(this.irc, this.network.channels[0], message);
expect(message.previews).to.deep.equal([{
body: "",
head: "",
link: url,
thumb: "",
type: "loading"
}]);
this.app.get("/basic", function(req, res) { this.app.get("/basic", function(req, res) {
res.send("<title>test title</title><meta name='description' content='simple description'>"); res.send("<title>test title</title><meta name='description' content='simple description'>");
}); });
@ -41,8 +50,9 @@ describe("Link plugin", function() {
expect(data.preview.type).to.equal("link"); expect(data.preview.type).to.equal("link");
expect(data.preview.head).to.equal("test title"); expect(data.preview.head).to.equal("test title");
expect(data.preview.body).to.equal("simple description"); 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.previews).to.deep.equal([data.preview]);
done(); done();
}); });
}); });
@ -178,6 +188,20 @@ describe("Link plugin", function() {
link(this.irc, this.network.channels[0], message); link(this.irc, this.network.channels[0], message);
expect(message.previews).to.eql([{
body: "",
head: "",
link: "http://localhost:9002/one",
thumb: "",
type: "loading"
}, {
body: "",
head: "",
link: "http://localhost:9002/two",
thumb: "",
type: "loading"
}]);
this.app.get("/one", function(req, res) { this.app.get("/one", function(req, res) {
res.send("<title>first title</title>"); res.send("<title>first title</title>");
}); });
@ -186,22 +210,19 @@ describe("Link plugin", function() {
res.send("<title>second title</title>"); res.send("<title>second title</title>");
}); });
const loaded = { const previews = [];
one: false,
two: false
};
this.irc.on("msg:preview", function(data) { this.irc.on("msg:preview", function(data) {
if (data.preview.link === "http://localhost:9002/one") { if (data.preview.link === "http://localhost:9002/one") {
expect(data.preview.head).to.equal("first title"); expect(data.preview.head).to.equal("first title");
loaded.one = true; previews[0] = data.preview;
} else if (data.preview.link === "http://localhost:9002/two") { } else if (data.preview.link === "http://localhost:9002/two") {
expect(data.preview.head).to.equal("second title"); expect(data.preview.head).to.equal("second title");
loaded.two = true; previews[1] = data.preview;
} }
if (loaded.one && loaded.two) { if (previews[0] && previews[1]) {
expect(message.previews.length).to.equal(2); expect(message.previews).to.eql(previews);
done(); done();
} }
}); });