From 3453707784243c88bdd3c7437f940cd90bd938b1 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 26 Nov 2018 11:15:23 +0100 Subject: [PATCH] FEATURE: allows html tooltips (#6665) --- .../javascripts/discourse/lib/tooltip.js.es6 | 15 +++-- test/javascripts/lib/tooltip-test.js.es6 | 60 +++++++++++++++---- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/tooltip.js.es6 b/app/assets/javascripts/discourse/lib/tooltip.js.es6 index 6028a87d3a..227e0f6c3c 100644 --- a/app/assets/javascripts/discourse/lib/tooltip.js.es6 +++ b/app/assets/javascripts/discourse/lib/tooltip.js.es6 @@ -3,10 +3,13 @@ import { escapeExpression } from "discourse/lib/utilities"; const fadeSpeed = 300; const tooltipID = "#discourse-tooltip"; -export function showTooltip() { - const $this = $(this); +export function showTooltip($this) { const $parent = $this.offsetParent(); - const content = escapeExpression($this.attr("data-tooltip")); + // html tooltip are risky try your best to sanitize anything + // displayed as html to avoid XSS attacks + const content = $this.attr("data-tooltip") + ? escapeExpression($this.attr("data-tooltip")) + : $this.attr("data-html-tooltip") || ""; const retina = window.devicePixelRatio && window.devicePixelRatio > 1 ? "class='retina'" @@ -19,7 +22,7 @@ export function showTooltip() { hideTooltip(tooltipID); - $(this).after(` + $this.after(`
${content}
@@ -74,7 +77,9 @@ export function hideTooltip() { export function registerTooltip(jqueryContext) { if (jqueryContext.length) { - jqueryContext.off("click").on("click", showTooltip); + jqueryContext + .off("click") + .on("click", event => showTooltip($(event.currentTarget))); } } diff --git a/test/javascripts/lib/tooltip-test.js.es6 b/test/javascripts/lib/tooltip-test.js.es6 index f65d326da5..ece7b0939e 100644 --- a/test/javascripts/lib/tooltip-test.js.es6 +++ b/test/javascripts/lib/tooltip-test.js.es6 @@ -4,22 +4,56 @@ import { registerTooltip } from "discourse/lib/tooltip"; QUnit.module("lib:tooltip", { beforeEach() { fixture().html( - "test" + ` + test + test + ` ); } }); -QUnit.test("it prevents XSS injection", assert => { - const $testLink = fixture(".test-link"); - registerTooltip($testLink); - $testLink.click(); +QUnit.test("text support", async assert => { + const $testTextLink = fixture(".test-text-link"); + registerTooltip($testTextLink); - andThen(() => { - assert.equal( - fixture(".tooltip-content") - .html() - .trim(), - "XSS<s onmouseover=alert(document.domain)>XSS" - ); - }); + await $testTextLink.click(); + + assert.equal( + fixture(".tooltip-content") + .html() + .trim(), + "XSS<s onmouseover=alert(document.domain)>XSS", + "it prevents XSS injection" + ); + + assert.equal( + fixture(".tooltip-content") + .text() + .trim(), + "XSSXSS", + "it returns content as plain text" + ); +}); + +QUnit.test("html support", async assert => { + const $testHtmlLink = fixture(".test-html-link"); + registerTooltip($testHtmlLink); + + await $testHtmlLink.click(); + + assert.equal( + fixture(".tooltip-content") + .html() + .trim(), + "

test

", + "it doesn’t escape HTML" + ); + + assert.equal( + fixture(".tooltip-content") + .text() + .trim(), + "test", + "it returns content as plain text" + ); });