Merge pull request #215 from mawinter69/remove-inline-js

remove inline js for favicon
jenkinsci · Aug 27, 2024 · 96d9592 · 96d9592
2 parents 1477c0c + 7739798
commit 96d9592
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 21 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/simpletheme/FaviconUrlThemeElement.java b/src/main/java/org/jenkinsci/plugins/simpletheme/FaviconUrlThemeElement.java
@@ -11,9 +11,7 @@
 public class FaviconUrlThemeElement extends UrlThemeElement {
 
     private static final String SCRIPT_INCLUDE =
-            "<script src=\"{0}/plugin/simple-theme-plugin/simple-theme.js\"></script>";
-    private static final String FAVICON_SCRIPT =
-            "<script>\n" + "window[''org.jenkinsci.plugins.simpletheme''].replaceFavicon(\"{0}\");\n" + "</script>";
+            "<script id=\"simple-theme-script\" data-url=\"{0}\" src=\"{1}/plugin/simple-theme-plugin/simple-theme.js\"></script>";
 
     @DataBoundConstructor
     public FaviconUrlThemeElement(String url) {
@@ -22,8 +20,8 @@ public FaviconUrlThemeElement(String url) {
 
     @Override
     public void collectHeaderFragment(Set<String> fragments, boolean injectCss) {
-        fragments.add(MessageFormat.format(SCRIPT_INCLUDE, Jenkins.get().getRootUrlFromRequest()));
-        fragments.add(MessageFormat.format(FAVICON_SCRIPT, getUrl()));
+        fragments.add(
+                MessageFormat.format(SCRIPT_INCLUDE, getUrl(), Jenkins.get().getRootUrlFromRequest()));
     }
 
     @Extension

diff --git a/src/main/webapp/simple-theme.js b/src/main/webapp/simple-theme.js
@@ -2,32 +2,26 @@
     "use strict";
 
     function removeAll() {
-        var links = Array.from(document.getElementsByTagName('link')),
-            link, i;
+        const links = document.getElementsByTagName('link');
 
-        for (i = 0; i < links.length; i++) {
-            link = links[i];
+        for (const link of links) {
             if (link.rel.split(/\s+/).some(e => e === 'icon')) {
-                link.parentNode.removeChild(link);
+                link.remove();
             }
         }
     }
 
     function add(url) {
-        var link = document.createElement('link');
+        const link = document.createElement('link');
         link.setAttribute('rel', 'icon');
         document.getElementsByTagName('head')[0].appendChild(link);
         link.setAttribute('href', url);
     }
 
-    function replaceFavicon(url) {
-        document.addEventListener("DOMContentLoaded", function(event) {
-            removeAll();
-            add(url);
-        });
-    }
+    document.addEventListener("DOMContentLoaded", function(event) {
+        const script = document.getElementById("simple-theme-script");
+        removeAll();
+        add(script.dataset.url);
+    });
 
-    global['org.jenkinsci.plugins.simpletheme'] = {
-        replaceFavicon: replaceFavicon
-    };
 })(this, document);
diff --git a/src/test/java/org/jenkinsci/plugins/simpletheme/SimpleThemeConfigurationTest.java b/src/test/java/org/jenkinsci/plugins/simpletheme/SimpleThemeConfigurationTest.java
@@ -59,7 +59,7 @@ public void testConfigCreatesHtml() throws Exception {
         assertElementPresentByXPath(page, "//script[contains(@src,'SOMEJS.js')]");
 
         assertElementPresentByXPath(page, "//script[contains(@src,'simple-theme.js')]");
-        assertElementPresentByXPath(page, "//script[contains(text(),'FAVICON.png')]");
+        assertElementPresentByXPath(page, "//script[contains(@data-url,'FAVICON.png')]");
     }
 
     private void fill(SimpleThemeDecorator decorator) {