Replace YUI tooltips with Tippy.js (#6408)

Co-authored-by: Alexander Brandes <[email protected]> Co-authored-by: Yaroslav <[email protected]> Co-authored-by: Daniel Beck <[email protected]> Co-authored-by: Kevin Guerroudj <[email protected]> Co-authored-by: Tim Jacomb <[email protected]> Co-authored-by: Tim Jacomb <[email protected]> Co-authored-by: Alexander Brandes <[email protected]> Co-authored-by: Daniel Beck <[email protected]> Co-authored-by: Basil Crow <[email protected]> Co-authored-by: Tim Jacomb <[email protected]>
jenkinsci · Nov 25, 2022 · 221ff94 · 221ff94
1 parent 86786b0
commit 221ff94
Show file tree

Hide file tree

Showing 32 changed files with 309 additions and 220 deletions.
diff --git a/core/src/main/java/org/jenkins/ui/icon/IconSet.java b/core/src/main/java/org/jenkins/ui/icon/IconSet.java
@@ -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;
@@ -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) + "\"");
             }
@@ -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) + "\"");
         }

diff --git a/core/src/main/resources/hudson/model/Run/KeepLogBuildBadge/badge.jelly b/core/src/main/resources/hudson/model/Run/KeepLogBuildBadge/badge.jelly
@@ -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"/>
diff --git a/core/src/main/resources/hudson/scm/AbstractScmTagAction/badge.jelly b/core/src/main/resources/hudson/scm/AbstractScmTagAction/badge.jelly
@@ -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>
diff --git a/core/src/main/resources/hudson/views/BuildButtonColumn/column.jelly b/core/src/main/resources/hudson/views/BuildButtonColumn/column.jelly
@@ -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>

diff --git a/core/src/main/resources/hudson/widgets/HistoryWidget/entry.jelly b/core/src/main/resources/hudson/widgets/HistoryWidget/entry.jelly
@@ -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>

diff --git a/core/src/main/resources/jenkins/widgets/HistoryPageFilter/queue-items.jelly b/core/src/main/resources/jenkins/widgets/HistoryPageFilter/queue-items.jelly
@@ -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>

diff --git a/core/src/main/resources/lib/form/repeatableDeleteButton.jelly b/core/src/main/resources/lib/form/repeatableDeleteButton.jelly
@@ -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>
diff --git a/core/src/main/resources/lib/hudson/buildHealth.jelly b/core/src/main/resources/lib/hudson/buildHealth.jelly
@@ -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>
@@ -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>
diff --git a/core/src/main/resources/lib/hudson/queue.jelly b/core/src/main/resources/lib/hudson/queue.jelly
@@ -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 -->

diff --git a/core/src/main/resources/lib/layout/helpIcon.jelly b/core/src/main/resources/lib/layout/helpIcon.jelly
@@ -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>
diff --git a/core/src/main/resources/lib/layout/icon.jelly b/core/src/main/resources/lib/layout/icon.jelly
@@ -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>
 
@@ -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 ?: ''}" />
@@ -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>

diff --git a/core/src/main/resources/lib/layout/svgIcon.jelly b/core/src/main/resources/lib/layout/svgIcon.jelly
@@ -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}">

diff --git a/core/src/test/java/org/jenkins/ui/icon/IconSetJenkins68805Test.java b/core/src/test/java/org/jenkins/ui/icon/IconSetJenkins68805Test.java
@@ -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")));
     }

diff --git a/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java b/core/src/test/java/org/jenkins/ui/icon/IconSetTest.java
@@ -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\""));
@@ -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\"")));
@@ -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\"")));
@@ -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")));
     }

diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java
@@ -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;
@@ -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 {

diff --git a/test/src/test/java/hudson/model/RunTest.java b/test/src/test/java/hudson/model/RunTest.java
@@ -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;

diff --git a/test/src/test/java/hudson/scm/AbstractScmTagActionTest.java b/test/src/test/java/hudson/scm/AbstractScmTagActionTest.java
@@ -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;
@@ -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 {
@@ -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();