Skip to content
Permalink
Browse files
[FIXED JENKINS-20772] Properly render response in case Apply results …
…in an error page.
  • Loading branch information
jglick committed Dec 23, 2013
1 parent 9e5817a commit 0e8195c43d744b65e46ce5d66262e29fbdb9fb35
Showing with 2 additions and 0 deletions.
  1. +2 −0 core/src/main/resources/lib/form/apply/apply.js
@@ -41,6 +41,8 @@ Behaviour.specify("INPUT.apply-button", 'apply', 0, function (e) {
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;
$(containerId).appendChild(doc.getElementsByTagName('body')[0]);
var r = YAHOO.util.Dom.getClientRegion();
responseDialog.cfg.setProperty("width",r.width*3/4+"px");
responseDialog.cfg.setProperty("height",r.height*3/4+"px");

5 comments on commit 0e8195c

@daniel-beck
Copy link
Member

@daniel-beck daniel-beck commented on 0e8195c Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probably suffice to show just doc.getElementById("main-panel") for regular error dialogs. To make sure it's a regular error dialog, could wrap the error in core/src/main/resources/jenkins/model/Jenkins/oops.jelly in another div with id="error-description" attribute, and show just that using getElementById, but keep getElementsByTagName('body')[0] as fallback if no such element is found?

@jglick
Copy link
Member Author

@jglick jglick commented on 0e8195c Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could make refinements such as these. My priority was to make sure something descriptive was displayed.

@daniel-beck
Copy link
Member

@daniel-beck daniel-beck commented on 0e8195c Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion implemented in #1076

I don't know how to make the dialog scrollable though (issue already present with just this change as well, Firefox 26 on OS X 10.8.6). Any ideas? The problem is that for the test case you provided (invalid expression in freestyle job's cron trigger), you don't even get to see the wrapped AntlrExceptions that actually explain what's wrong.

@jglick
Copy link
Member Author

@jglick jglick commented on 0e8195c Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how to make the dialog scrollable though

Nor I. Beyond my knowledge of YUI.

@daniel-beck
Copy link
Member

@daniel-beck daniel-beck commented on 0e8195c Dec 23, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrolling fixed in #1077. The container div had automatic size, so it was way larger than the dialog box and cut off by some YUI container element with overflow:hidden. This PR fixes that by defining a size slightly smaller than the dialog and an overflow:scroll style attribute.

Please sign in to comment.