Skip to content

Commit

Permalink
Replace YUI tooltips with Tippy.js (jenkinsci#6408)
Browse files Browse the repository at this point in the history
Co-authored-by: Alexander Brandes <brandes.alexander@web.de>
Co-authored-by: Yaroslav <91559310+yaroslavafenkin@users.noreply.github.com>
Co-authored-by: Daniel Beck <1831569+daniel-beck@users.noreply.github.com>
Co-authored-by: Kevin Guerroudj <91883215+Kevin-CB@users.noreply.github.com>
Co-authored-by: Tim Jacomb <timjacomb1+github@gmail.com>
Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>
Co-authored-by: Alexander Brandes <mc.cache@web.de>
Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>
Co-authored-by: Basil Crow <me@basilcrow.com>
Co-authored-by: Tim Jacomb <timjacomb1@gmail.com>
  • Loading branch information
11 people committed Nov 25, 2022
1 parent 86786b0 commit 221ff94
Show file tree
Hide file tree
Showing 32 changed files with 309 additions and 220 deletions.
14 changes: 11 additions & 3 deletions core/src/main/java/org/jenkins/ui/icon/IconSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private static String prependTitleIfRequired(String icon, String title) {

// for Jelly
@Restricted(NoExternalUse.class)
public static String getSymbol(String name, String title, String tooltip, String classes, String pluginName, String id) {
public static String getSymbol(String name, String title, String tooltip, String htmlTooltip, String classes, String pluginName, String id) {
String translatedName = cleanName(name);

String identifier = Util.fixEmpty(pluginName) == null ? "core" : pluginName;
Expand All @@ -95,10 +95,14 @@ public static String getSymbol(String name, String title, String tooltip, String
String symbol = symbolsForLookup.get(translatedName);
symbol = symbol.replaceAll("(class=\").*?(\")", "$1$2");
symbol = symbol.replaceAll("(tooltip=\").*?(\")", "");
symbol = symbol.replaceAll("(data-html-tooltip=\").*?(\")", "");
symbol = symbol.replaceAll("(id=\").*?(\")", "");
if (!tooltip.isEmpty()) {
if (!tooltip.isEmpty() && htmlTooltip.isEmpty()) {
symbol = symbol.replaceAll("<svg", "<svg tooltip=\"" + Functions.htmlAttributeEscape(tooltip) + "\"");
}
if (!htmlTooltip.isEmpty()) {
symbol = symbol.replaceAll("<svg", "<svg data-html-tooltip=\"" + Functions.htmlAttributeEscape(htmlTooltip) + "\"");
}
if (!id.isEmpty()) {
symbol = symbol.replaceAll("<svg", "<svg id=\"" + Functions.htmlAttributeEscape(id) + "\"");
}
Expand All @@ -124,10 +128,14 @@ public static String getSymbol(String name, String title, String tooltip, String
symbol = symbol.replaceAll("(<title>).*(</title>)", "$1$2");
symbol = symbol.replaceAll("(class=\").*?(\")", "$1$2");
symbol = symbol.replaceAll("(tooltip=\").*?(\")", "$1$2");
symbol = symbol.replaceAll("(data-html-tooltip=\").*?(\")", "$1$2");
symbol = symbol.replaceAll("(id=\").*?(\")", "");
if (!tooltip.isEmpty()) {
if (!tooltip.isEmpty() && htmlTooltip.isEmpty()) {
symbol = symbol.replaceAll("<svg", "<svg tooltip=\"" + Functions.htmlAttributeEscape(tooltip) + "\"");
}
if (!htmlTooltip.isEmpty()) {
symbol = symbol.replaceAll("<svg", "<svg data-html-tooltip=\"" + Functions.htmlAttributeEscape(htmlTooltip) + "\"");
}
if (!id.isEmpty()) {
symbol = symbol.replaceAll("<svg", "<svg id=\"" + Functions.htmlAttributeEscape(id) + "\"");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ THE SOFTWARE.
-->

<?jelly escape-by-default='true'?>
<l:icon class="icon-lock icon-sm" tooltip="${%Keep this build forever}:&lt;br/&gt;${h.xmlEscape(build.whyKeepLog)}" xmlns:l="/lib/layout"/>
<l:icon class="icon-lock icon-sm" tooltip="${%Keep this build forever}:\n${build.whyKeepLog}" xmlns:l="/lib/layout"/>
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:if xmlns:j="jelly:core" xmlns:l="/lib/layout" test="${it.isTagged()}">
<a href="${rootURL}/${it.run.url}tagBuild/">
<l:icon class="icon-save icon-sm" tooltip="${h.escape(it.tooltip)}"/>
<l:icon class="icon-save icon-sm" tooltip="${it.tooltip}"/>
</a>
</j:if>
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ THE SOFTWARE.
</j:otherwise>
</j:choose>
<j:set var="isQueued" value="${app.queue.contains(job)}"/>
<a id="${id}" tooltip="${h.htmlAttributeEscape(title)}" class="jenkins-table__button jenkins-!-build-color" href="${href}">
<a id="${id}" tooltip="${title}" class="jenkins-table__button jenkins-!-build-color" href="${href}">
<span class="${isQueued ? 'pulse-animation': ''}">
<l:icon src="symbol-play" />
</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ THE SOFTWARE.
<td class="build-row-cell">
<div class="pane build-name">
<div class="build-icon">
<a class="build-status-link" href="${link}console"><l:icon alt="${build.iconColor.description} &gt; ${%Console Output}" class="${build.buildStatusIconClassName} icon-sm" tooltip="${build.iconColor.description} &gt; ${%Console Output}"/></a>
<a class="build-status-link" href="${link}console" tooltip="${build.iconColor.description} > ${%Console Output}">
<l:icon class="${build.buildStatusIconClassName} icon-sm" />
</a>
</div>
<a class="model-link inside build-link display-name" update-parent-class=".build-row" href="${link}">${build.displayName}</a>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ THE SOFTWARE.
</j:choose>
<j:if test="${!item.params.isEmpty()}">
<div style="float:right;margin-right:10px;">
<a href="#" tooltip="Build Parameters:${h.escape(item.params)}"><l:icon class="icon-notepad icon-sm" /></a>
<a href="#" tooltip="Build Parameters:${item.params}"><l:icon class="icon-notepad icon-sm" /></a>
</div>
</j:if>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ THE SOFTWARE.
</st:attribute>
</st:documentation>
<st:adjunct includes="lib.form.repeatable.repeatable"/>
<button tooltip="${h.xmlEscape(attrs.value ?: '%Delete')}" class="repeatable-delete danger" type="button">
<button tooltip="${attrs.value ?: '%Delete'}" class="repeatable-delete danger" type="button">
<l:icon src="symbol-close" />
</button>
</j:jelly>
56 changes: 29 additions & 27 deletions core/src/main/resources/lib/hudson/buildHealth.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,40 @@ THE SOFTWARE.
<x:element name="${useTdElement!=null?'td':'div'}">
<x:attribute name="data">${buildHealth.score}</x:attribute>
<x:attribute name="class">jenkins-table__cell--tight jenkins-table__icon healthReport</x:attribute>
<j:if test="${!empty(healthReports)}">
<x:attribute name="data-html-tooltip">
<div class="jenkins-tooltip--table-wrapper">
<table class="jenkins-table">
<thead>
<tr>
<th class="jenkins-!-padding-left-0" align="center">W</th>
<th align="left">${%Description}</th>
<th align="right">%</th>
</tr>
</thead>
<tbody>
<j:forEach var="rpt" items="${healthReports}">
<tr>
<td align="left" class="jenkins-table__cell--tight jenkins-table__icon">
<div class="jenkins-table__cell__button-wrapper">
<l:icon src="symbol-weather-${buildHealth.iconClassName}" />
</div>
</td>
<td align="left">${rpt.localizableDescription}</td>
<td align="right">${rpt.score}</td>
</tr>
</j:forEach>
</tbody>
</table>
</div>
</x:attribute>
</j:if>
<j:if test="${buildHealth!=null}">
<div class="jenkins-table__cell__button-wrapper">
<j:choose>
<j:when test="${!empty(healthReports)}">
<a class="build-health-link jenkins-table__button" href="${empty(link)?'#':link}" style="${attrs.style}">
<l:icon src="symbol-weather-${buildHealth.iconClassName}" />
<l:icon src="symbol-weather-${buildHealth.iconClassName}" />
</a>
</j:when>
<j:otherwise>
Expand All @@ -52,31 +80,5 @@ THE SOFTWARE.
</j:choose>
</div>
</j:if>
<j:if test="${!empty(healthReports)}">
<div class="jenkins-tooltip healthReportDetails">
<table class="jenkins-table">
<thead>
<tr>
<th class="jenkins-!-padding-left-0" align="center">W</th>
<th align="left">${%Description}</th>
<th align="right">%</th>
</tr>
</thead>
<tbody>
<j:forEach var="rpt" items="${healthReports}">
<tr>
<td align="left" class="jenkins-table__cell--tight jenkins-table__icon">
<div class="jenkins-table__cell__button-wrapper">
<l:icon src="symbol-weather-${buildHealth.iconClassName}" />
</div>
</td>
<td align="left">${rpt.localizableDescription}</td>
<td align="right">${rpt.score}</td>
</tr>
</j:forEach>
</tbody>
</table>
</div>
</j:if>
</x:element>
</j:jelly>
2 changes: 1 addition & 1 deletion core/src/main/resources/lib/hudson/queue.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ THE SOFTWARE.
<j:set var="stuck" value="${item.isStuck()}"/>
<j:choose>
<j:when test="${h.hasPermission(item.task,item.task.READ)}">
<a href="${rootURL}/${item.task.url}" class="model-link inside tl-tr" tooltip="${h.escape(item.causesDescription)}${h.escape(item.why)}${h.escape(item.params)}&lt;br&gt;${%WaitingFor(item.inQueueForString)}">
<a href="${rootURL}/${item.task.url}" class="model-link inside tl-tr" tooltip="${item.causesDescription} ${item.why} ${item.params} \n ${%WaitingFor(item.inQueueForString)}">
<l:breakable value="${item.task.fullDisplayName}"/>
</a>
<!-- TODO include estimated number as in BuildHistoryWidget/entries.jelly if possible -->
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/resources/lib/layout/helpIcon.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<j:if test="${attrs.iconSize != null}">
<j:set var="iconSize" value="icon-${iconSize}"/>
</j:if>
<l:icon tooltip="${h.htmlAttributeEscape(tooltip)}"
<l:icon tooltip="${tooltip}"
class="${class} ${iconSize}"
src="symbol-help-circle" />
</j:jelly>
12 changes: 7 additions & 5 deletions core/src/main/resources/lib/layout/icon.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,13 @@ THE SOFTWARE.
</st:attribute>

<st:attribute name="onclick" deprecated="true">onclick handler. Deprecated; assign an ID and look up the element that way to attach event handlers.</st:attribute>
<st:attribute name="title" deprecated="true">title, deprecated use tooltip instead, but beware of its support for HTML</st:attribute>
<st:attribute name="title" deprecated="true">title, deprecated use tooltip instead, or htmlTooltip if you intend to pass HTML.</st:attribute>
<st:attribute name="style">style</st:attribute>
<st:attribute name="tooltip">
tooltip (supports HTML for PNG and symbol icons).
Make sure to call h.htmlAttributeEscape on all user-specified parts of the value to prevent cross-site scripting.
Icons based on classic (non-symbol) SVG do not support HTML tooltips due to how SECURITY-1955 was fixed in Jenkins 2.252 and 2.235.4, but since such icons can be upgraded to symbols, it is important to still escape user-specified parts of the text (resulting in double escaping while the icon is based on classic SVG).</st:attribute>
Adds a tooltip to the icon, ignores HTML except 'br' tags (but '\n' should be preferred for line breaks).</st:attribute>
<st:attribute name="htmlTooltip">
Tooltip but with HTML support. Make sure to call h.htmlAttributeEscape on all user-specified parts of the value to prevent cross-site scripting.
Use 'tooltip' if you don't need to pass HTML.</st:attribute>
<st:attribute name="alt">alt, adds invisible text suitable for screen-readers for symbols, sets the alt attribute for normal images</st:attribute>
</st:documentation>

Expand All @@ -71,6 +72,7 @@ THE SOFTWARE.
<j:arg value="${iconSrc.substring(7)}" />
<j:arg value="${alt ?: ''}" />
<j:arg value="${attrs.tooltip ?: ''}" />
<j:arg value="${attrs.htmlTooltip ?: ''}" />
<j:arg value="${attrs.class ?: iconMetadata.classSpec ?: ''}" />
<j:arg value="${h.extractPluginNameFromIconSrc(iconSrc)}" />
<j:arg value="${attrs.id ?: ''}" />
Expand All @@ -96,7 +98,7 @@ THE SOFTWARE.

<j:otherwise>
<img class="${attrs.class}" src="${iconSrc}" style="${imgStyle}" title="${attrs.title}" height="${attrs.height}"
alt="${attrs.alt}" width="${attrs.width}" onclick="${attrs.onclick}" tooltip="${attrs.tooltip}" id="${attrs.id}" />
alt="${attrs.alt}" width="${attrs.width}" onclick="${attrs.onclick}" tooltip="${attrs.tooltip}" data-html-tooltip="${attrs.htmlTooltip}" id="${attrs.id}" />
</j:otherwise>
</j:choose>
</j:otherwise>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/resources/lib/layout/svgIcon.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
style="${attrs.style}"
onclick="${attrs.onclick}"
id="${attrs.id}"
tooltip="${attrs.tooltip != null ? h.xmlEscape(attrs.tooltip) : null}">
tooltip="${attrs.tooltip != null ? attrs.tooltip : null}">

<j:choose>
<j:when test="${attrs.href != null}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ public class IconSetJenkins68805Test {
@Issue("JENKINS-68805")
void getSymbol_notSettingTooltipDoesntAddTooltipAttribute_evenWithAmpersand() {
// cache a symbol with tooltip containing ampersand:
String symbolWithTooltip = IconSet.getSymbol("download", "Title", "With&Ampersand", "class1 class2", "", "id");
String symbolWithTooltip = IconSet.getSymbol("download", "Title", "With&Ampersand", "", "class1 class2", "", "id");
assertThat(symbolWithTooltip, containsString("tooltip"));
assertThat(symbolWithTooltip, containsString("With&"));

// Same symbol, no tooltip
String symbolWithoutTooltip = IconSet.getSymbol("download", "Title", "", "class1 class2", "", "id");
String symbolWithoutTooltip = IconSet.getSymbol("download", "Title", "", "", "class1 class2", "", "id");

assertThat(symbolWithoutTooltip, not(containsString("tooltip")));
}
Expand Down
12 changes: 6 additions & 6 deletions core/src/test/java/org/jenkins/ui/icon/IconSetTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void testIconSetSize() {

@Test
void getSymbol() {
String symbol = IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2", "", "id");
String symbol = IconSet.getSymbol("download", "Title", "Tooltip", "", "class1 class2", "", "id");

assertThat(symbol, containsString("<span class=\"jenkins-visually-hidden\">Title</span>"));
assertThat(symbol, containsString("tooltip=\"Tooltip\""));
Expand All @@ -31,8 +31,8 @@ void getSymbol() {

@Test
void getSymbol_cachedSymbolDoesntReturnAttributes() {
IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2", "", "id");
String symbol = IconSet.getSymbol("download", "", "", "", "", "");
IconSet.getSymbol("download", "Title", "Tooltip", "", "class1 class2", "", "id");
String symbol = IconSet.getSymbol("download", "", "", "", "", "", "");

assertThat(symbol, not(containsString("<span class=\"jenkins-visually-hidden\">Title</span>")));
assertThat(symbol, not(containsString("tooltip=\"Tooltip\"")));
Expand All @@ -43,8 +43,8 @@ void getSymbol_cachedSymbolDoesntReturnAttributes() {

@Test
void getSymbol_cachedSymbolAllowsSettingAllAttributes() {
IconSet.getSymbol("download", "Title", "Tooltip", "class1 class2", "", "id");
String symbol = IconSet.getSymbol("download", "Title2", "Tooltip2", "class3 class4", "", "id2");
IconSet.getSymbol("download", "Title", "Tooltip", "", "class1 class2", "", "id");
String symbol = IconSet.getSymbol("download", "Title2", "Tooltip2", "", "class3 class4", "", "id2");

assertThat(symbol, not(containsString("<span class=\"jenkins-visually-hidden\">Title</span>")));
assertThat(symbol, not(containsString("tooltip=\"Tooltip\"")));
Expand All @@ -62,7 +62,7 @@ void getSymbol_cachedSymbolAllowsSettingAllAttributes() {
*/
@Test
void getSymbol_notSettingTooltipDoesntAddTooltipAttribute() {
String symbol = IconSet.getSymbol("download", "Title", "", "class1 class2", "", "id");
String symbol = IconSet.getSymbol("download", "Title", "", "", "class1 class2", "", "id");

assertThat(symbol, not(containsString("tooltip")));
}
Expand Down
15 changes: 5 additions & 10 deletions test/src/test/java/hudson/model/QueueTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,10 @@

import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.Page;
import com.gargoylesoftware.htmlunit.ScriptResult;
import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.html.DomElement;
import com.gargoylesoftware.htmlunit.html.DomNode;
import com.gargoylesoftware.htmlunit.html.DomNodeList;
import com.gargoylesoftware.htmlunit.html.HtmlAnchor;
import com.gargoylesoftware.htmlunit.html.HtmlElement;
import com.gargoylesoftware.htmlunit.html.HtmlFileInput;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlFormUtil;
Expand Down Expand Up @@ -1272,14 +1270,11 @@ private String buildAndExtractTooltipAttribute() throws Exception {

HtmlPage page = wc.goTo("");

DomElement buildQueue = page.getElementById("buildQueue");
DomNodeList<HtmlElement> anchors = buildQueue.getElementsByTagName("a");
HtmlAnchor anchorWithTooltip = (HtmlAnchor) anchors.stream()
.filter(a -> a.getAttribute("tooltip") != null && !a.getAttribute("tooltip").isEmpty())
.findFirst().orElseThrow(IllegalStateException::new);
page.executeJavaScript("document.querySelector('#buildQueue a[tooltip]:not([tooltip=\"\"])')._tippy.show()");
wc.waitForBackgroundJavaScript(1000);
ScriptResult result = page.executeJavaScript("document.querySelector('.tippy-content').innerHTML;");

String tooltip = anchorWithTooltip.getAttribute("tooltip");
return tooltip;
return result.getJavaScriptResult().toString();
}

public static class BrokenAffinityKeyProject extends Project<BrokenAffinityKeyProject, BrokenAffinityKeyBuild> implements TopLevelItem {
Expand Down
4 changes: 2 additions & 2 deletions test/src/test/java/hudson/model/RunTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ private void ensureXssIsPrevented(FreeStyleProject upProject, String validationP
HtmlPage htmlPage = wc.goTo(upProject.getUrl());

// trigger the tooltip display
htmlPage.executeJavaScript("document.querySelector('#buildHistory table .build-badge img').dispatchEvent(new Event('mouseover'));");
htmlPage.executeJavaScript("document.querySelector('#buildHistory table .build-badge img')._tippy.show()");
wc.waitForBackgroundJavaScript(500);
ScriptResult result = htmlPage.executeJavaScript("document.querySelector('#tt').innerHTML;");
ScriptResult result = htmlPage.executeJavaScript("document.querySelector('.tippy-content').innerHTML;");
Object jsResult = result.getJavaScriptResult();
assertThat(jsResult, instanceOf(String.class));
String jsResultString = (String) jsResult;
Expand Down
18 changes: 0 additions & 18 deletions test/src/test/java/hudson/scm/AbstractScmTagActionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@

package hudson.scm;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;

import com.gargoylesoftware.htmlunit.html.DomElement;
Expand All @@ -45,7 +41,6 @@
import java.io.File;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

public class AbstractScmTagActionTest {
Expand All @@ -66,19 +61,6 @@ public void regularTextDisplayedCorrectly() throws Exception {
assertEquals(tagToKeep, tooltip);
}

@Test
@Issue("SECURITY-1537")
public void preventXssInTagAction() throws Exception {
FreeStyleProject p = j.createFreeStyleProject();
p.setScm(new FakeSCM("<img src='x' onerror=alert(123)>XSS"));

j.buildAndAssertSuccess(p);

String tooltip = buildAndExtractTooltipAttribute(p);
assertThat(tooltip, not(containsString("<")));
assertThat(tooltip, startsWith("&lt;"));
}

private String buildAndExtractTooltipAttribute(FreeStyleProject p) throws Exception {
JenkinsRule.WebClient wc = j.createWebClient();

Expand Down
Loading

0 comments on commit 221ff94

Please sign in to comment.