Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-42443

f:select should show a spinner while AJAX requests are in-flight

    • Icon: New Feature New Feature
    • Resolution: Fixed
    • Icon: Minor Minor
    • core
    • Jenkins 2.167

      Port the fix from JENKINS-32007 to core

          [JENKINS-42443] f:select should show a spinner while AJAX requests are in-flight

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          core/src/main/java/hudson/util/FormFillFailure.java
          core/src/main/resources/lib/form/select/select.js
          http://jenkins-ci.org/commit/jenkins/ed329d1088c164df560b9299cd9096ae76519418
          Log:
          [FIXED JENKINS-42443] Make f:select display spinner during AJAX requests

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: core/src/main/java/hudson/util/FormFillFailure.java core/src/main/resources/lib/form/select/select.js http://jenkins-ci.org/commit/jenkins/ed329d1088c164df560b9299cd9096ae76519418 Log: [FIXED JENKINS-42443] Make f:select display spinner during AJAX requests

          Code changed in jenkins
          User: Stephen Connolly
          Path:
          core/src/main/java/hudson/util/FormFillFailure.java
          http://jenkins-ci.org/commit/jenkins/79c761d51a244db006b655160017989039c8b44b
          Log:
          Fixing @since tags from merge of JENKINS-42443

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Stephen Connolly Path: core/src/main/java/hudson/util/FormFillFailure.java http://jenkins-ci.org/commit/jenkins/79c761d51a244db006b655160017989039c8b44b Log: Fixing @since tags from merge of JENKINS-42443

          Users reported a HTTP Error 404 in my plugin, after investigating and running git bisect, i've managed to narrow the culprit down to this Jenkins Issue / commit. For more information please see the google group discussion.

           

          Thank you very much.

          Goran Sarenkapa added a comment - Users reported a HTTP Error 404 in my plugin, after investigating and running git bisect, i've managed to narrow the culprit down to this Jenkins Issue / commit. For more information please see the google group discussion.   Thank you very much.

          jordangs I believe the issue here is how you are configuring things.

          The select CSS style is used to trigger the AJAX field population and the intent is that the style should only be present on select tags generated by f:select.

          Your plugin is using a very non-Jenkins way to build its configuration, in fact you are not even using the AJAX select population that is triggered by adding the select CSS style: https://github.com/jenkinsci/zap-plugin/blob/development/src/main/resources/org/jenkinsci/plugins/zap/ZAPDriver/config.jelly#L246

          I suspect that your plugin has always been making 404 requests and getting 404 responses but this change is now just showing the error, have a look at the network request inspector when loading the config page to see if my theory is correct.

          There are two ways to your issue:

          1. Just remove the select CSS class from your <select> tags; or
          2. Switch to the Jenkins idiomatic AJAX select population by adding a doFillSelectedExportFormats() method to your descriptor and either provide the required fillUrl attribute in your <select> tag or switch to using <f:select>

          Stephen Connolly added a comment - jordangs I believe the issue here is how you are configuring things. The select CSS style is used to trigger the AJAX field population and the intent is that the style should only be present on select tags generated by f:select. Your plugin is using a very non-Jenkins way to build its configuration, in fact you are not even using the AJAX select population that is triggered by adding the select CSS style: https://github.com/jenkinsci/zap-plugin/blob/development/src/main/resources/org/jenkinsci/plugins/zap/ZAPDriver/config.jelly#L246 I suspect that your plugin has always been making 404 requests and getting 404 responses but this change is now just showing the error, have a look at the network request inspector when loading the config page to see if my theory is correct. There are two ways to your issue: 1. Just remove the select CSS class from your <select> tags; or 2. Switch to the Jenkins idiomatic AJAX select population by adding a doFillSelectedExportFormats() method to your descriptor and either provide the required fillUrl attribute in your <select> tag or switch to using <f:select>

          Closing after re-open, because the re-open was for the change functioning as designed and reporting a 404 response from the AJAX fill request

          Stephen Connolly added a comment - Closing after re-open, because the re-open was for the change functioning as designed and reporting a 404 response from the AJAX fill request

          Oleksii Vasylkivskyi added a comment - - edited

          stephenconnolly, could you please suggest on the following?

           

          in doFillXyzItems() method if we throw an exception (other than FormValidation) there will be stack trace shown to the user under select entry field.

          If we throw FormValidation exception in doFillXyzItems(), there will be some java script error so expected behavior for such case is unknown for me.

          Was there any intent to handle FormValidation exception differently than other ones? Does it make sense to request such functionality?

          Example:

          field A (select entry)

          field B (select entry)

          B select items will be filled based on A.

          The question is how to handle doFillBItems() when A validation failed and do not scare user by stack trace but show some meaningful information. At this point of time I do not know how to catch in doFillBItems() information about failed validation of A field, so I just use A and there can be exception (which will be good to handle somehow).

          Definitely I can make extra logic and return old value which was stored for B (checked that it is possible). But also it could be nice to have Jenkins it handle for us and show just FormValidation error instead of stack trace and preserve values what were stored before (as it works now).

          And actually the it can be super nice to have information about validation of field on which we are depending in doFillBItems() or even do not trigger doFillBItems() if A validation failed.

          Oleksii Vasylkivskyi added a comment - - edited stephenconnolly , could you please suggest on the following?   in doFillXyzItems() method if we throw an exception (other than FormValidation) there will be stack trace shown to the user under select entry field. If we throw FormValidation exception in doFillXyzItems(), there will be some java script error so expected behavior for such case is unknown for me. Was there any intent to handle FormValidation exception differently than other ones? Does it make sense to request such functionality? Example: field A (select entry) field B (select entry) B select items will be filled based on A. The question is how to handle doFillBItems() when A validation failed and do not scare user by stack trace but show some meaningful information. At this point of time I do not know how to catch in doFillBItems() information about failed validation of A field, so I just use A and there can be exception (which will be good to handle somehow). Definitely I can make extra logic and return old value which was stored for B (checked that it is possible). But also it could be nice to have Jenkins it handle for us and show just FormValidation error instead of stack trace and preserve values what were stored before (as it works now). And actually the it can be super nice to have information about validation of field on which we are depending in doFillBItems() or even do not trigger doFillBItems() if A validation failed.

            stephenconnolly Stephen Connolly
            stephenconnolly Stephen Connolly
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: