Use Jenkins modal for 'Apply' button failures (#8394)

* Init * Linting * Test fixes * Switch to dialog for simple error case --------- Co-authored-by: Tim Jacomb <[email protected]>
jenkinsci · Dec 20, 2023 · 3a1ac2c · 3a1ac2c
1 parent 2ece2cc
commit 3a1ac2c
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 83 deletions.
diff --git a/core/src/main/resources/jenkins/model/Jenkins/oops.properties b/core/src/main/resources/jenkins/model/Jenkins/oops.properties
@@ -1,4 +1,4 @@
-problemHappened=A problem occurred while processing the request.
+problemHappened=A problem occurred while processing the request
 checkJIRA=Please check <a href="https://www.jenkins.io/redirect/issue-tracker">our bug tracker</a> to see if a similar problem has already been reported.
 vote=If it is already reported, please vote and put a comment on it to let us gauge the impact of the problem.
 pleaseReport=If you think this is a new issue, please file a new issue.

diff --git a/core/src/main/resources/lib/form/apply/apply.js b/core/src/main/resources/lib/form/apply/apply.js
@@ -3,104 +3,59 @@ Behaviour.specify(
   "apply",
   0,
   function (e) {
-    var id;
-    var containerId = "container" + iota++;
-
-    var responseDialog = new YAHOO.widget.Panel("wait" + iota++, {
-      fixedcenter: true,
-      close: true,
-      draggable: true,
-      zindex: 4,
-      modal: true,
-      visible: false,
-    });
-
-    responseDialog.setHeader("Error");
-    responseDialog.setBody("<div id='" + containerId + "'></div>");
-    responseDialog.render(document.body);
-    var target; // iframe
-
-    function attachIframeOnload(target, f) {
-      if (target.attachEvent) {
-        target.attachEvent("onload", f);
-      } else {
-        target.onload = f;
-      }
-    }
-
     e.addEventListener("click", function (e) {
-      var f = e.target.closest("FORM");
+      const f = e.target.closest("FORM");
 
       // create a throw-away IFRAME to avoid back button from loading the POST result back
-      id = "iframe" + iota++;
-      target = document.createElement("iframe");
+      const id = "iframe" + iota++;
+      const target = document.createElement("iframe");
       target.setAttribute("id", id);
       target.setAttribute("name", id);
       target.style.height = "100%";
       target.style.width = "100%";
-      document.getElementById(containerId).appendChild(target);
+      document.querySelector("body").appendChild(target);
 
-      attachIframeOnload(target, function () {
+      f.target = target.id;
+      f.elements["core:apply"].value = "true";
+      f.dispatchEvent(new Event("jenkins:apply")); // give everyone a chance to write back to DOM
+
+      try {
+        buildFormTree(f);
+        f.submit();
+      } finally {
+        f.elements["core:apply"].value = null;
+        f.target = "_self";
+      }
+
+      target.addEventListener("load", () => {
         if (
           target.contentWindow &&
           target.contentWindow.applyCompletionHandler
         ) {
           // apply-aware server is expected to set this handler
           target.contentWindow.applyCompletionHandler(window);
-        } else {
-          // otherwise this is possibly an error from the server, so we need to render the whole content.
-          var doc = target.contentDocument || target.contentWindow.document;
-          var error = doc.getElementById("error-description");
-          var r = YAHOO.util.Dom.getClientRegion();
-          var contentHeight = r.height / 5;
-          var contentWidth = r.width / 2;
-          if (!error) {
-            // fallback if it's not a regular error dialog from oops.jelly: use the entire body
-            error = document.createElement("div");
-            error.setAttribute("id", "error-description");
-            error.appendChild(doc.getElementsByTagName("body")[0]);
-            contentHeight = (r.height * 3) / 4;
-            contentWidth = (r.width * 3) / 4;
-          }
-
-          let oldError = document.getElementById("error-description");
-          if (oldError) {
-            // Remove old error if there is any
-            document.getElementById(containerId).removeChild(oldError);
-          }
+          // Remove the iframe from the DOM
+          target.remove();
+          return;
+        }
 
-          document.getElementById(containerId).appendChild(error);
+        // otherwise this is possibly an error from the server, so we need to render the whole content.
+        const doc = target.contentDocument || target.contentWindow.document;
+        let error = doc.getElementById("error-description");
 
-          var dialogStyleHeight = contentHeight + 40;
-          var dialogStyleWidth = contentWidth + 20;
+        if (!error) {
+          // Fallback if it's not a regular error dialog from oops.jelly: use the entire body
+          error = document.createElement("div");
+          error.appendChild(doc.querySelector("#page-body"));
+        }
 
-          document.getElementById(containerId).style.height =
-            contentHeight + "px";
-          document.getElementById(containerId).style.width =
-            contentWidth + "px";
-          document.getElementById(containerId).style.overflow = "scroll";
+        dialog.modal(error, {
+          minWidth: "850px",
+        });
 
-          responseDialog.cfg.setProperty("width", dialogStyleWidth + "px");
-          responseDialog.cfg.setProperty("height", dialogStyleHeight + "px");
-          responseDialog.center();
-          responseDialog.show();
-        }
-        window.setTimeout(function () {
-          // otherwise Firefox will fail to leave the "connecting" state
-          document.getElementById(id).remove();
-        }, 0);
+        // Remove the iframe from the DOM
+        target.remove();
       });
-
-      f.target = target.id;
-      f.elements["core:apply"].value = "true";
-      f.dispatchEvent(new Event("jenkins:apply")); // give everyone a chance to write back to DOM
-      try {
-        buildFormTree(f);
-        f.submit();
-      } finally {
-        f.elements["core:apply"].value = null;
-        f.target = "_self";
-      }
     });
   },
 );
diff --git a/test/src/test/java/hudson/model/ViewTest.java b/test/src/test/java/hudson/model/ViewTest.java
@@ -700,7 +700,7 @@ public void simplifiedOriginalDescription() throws Exception {
         assertThat(e.getStatusCode(), equalTo(500));
 
         // This should have a different message, but this is the current behavior demonstrating the problem.
-        assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request."));
+        assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request"));
 
         OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class);
         Map<Saveable, OldDataMonitor.VersionRange> data = odm.getData();
@@ -744,7 +744,7 @@ public void simplifiedWithValidXmlAndBadField() throws Exception {
         assertThat(e.getStatusCode(), equalTo(500));
 
         // This should have a different message, but this is the current behavior demonstrating the problem.
-        assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request."));
+        assertThat(e.getResponse().getContentAsString(), containsString("A problem occurred while processing the request"));
 
         OldDataMonitor odm = ExtensionList.lookupSingleton(OldDataMonitor.class);
         Map<Saveable, OldDataMonitor.VersionRange> data = odm.getData();

diff --git a/test/src/test/java/jenkins/security/StackTraceSuppressionTest.java b/test/src/test/java/jenkins/security/StackTraceSuppressionTest.java
@@ -161,7 +161,7 @@ public void save() {
     }
 
     private void checBaseResponseContent(String content) {
-        assertThat(content, containsString("A problem occurred while processing the request."));
+        assertThat(content, containsString("A problem occurred while processing the request"));
         assertThat(content, containsString("Logging ID="));
         assertThat(content, containsString("Oops!"));
     }