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

Reactive Reference parameter with Formated HTML loses script section when binding to other build parameters

    • Icon: Task Task
    • Resolution: Not A Defect
    • Icon: Minor Minor
    • active-choices-plugin
    • None
    • Jenkins 2.32.2, Active Choices 1.5.2, Chrome v. 56

      Using an Active Choices Reactive Reference Parameter and having it return Formatted HTML with a script portion (onchange, etc...) works great unless the groovy script references other dependent build parameters such as strings, Active Choice parameters, etc... The returned HTML renders properly, just without the script. Attached is a (hopefully) very simple job configuration to demonstrate.

      The groovy script for generating the sample test is as follows (DummyVar is a string parameter defined just above this parameter):

      //Uncomment this line, causing DummyVar to be referenced, and the script will be lost at runtime.
      //def a = DummyVar
      
      def FullHTML = ""
      FullHTML += "<head>"
      FullHTML += "<script type='text/javascript'> jQuery(document).on('ready', function() {	function updateOutputValue()  { jQuery('#value').val('Booya!');     };"
      FullHTML += " updateOutputValue();"
      FullHTML += "});"
      FullHTML += "</script>"
      FullHTML += "</head>"
      FullHTML += "<body><form><input id='value' name='value' > </form></body>"
      return FullHTML
      

        1. config.xml
          4 kB
        2. config.xml
          4 kB
        3. config[1].xml
          5 kB
        4. config-5-4-2017.xml
          8 kB
        5. config-doubleload.xml
          7 kB
        6. image-2017-06-01-22-46-45-391.png
          image-2017-06-01-22-46-45-391.png
          11 kB
        7. image-2017-06-01-22-48-32-552.png
          image-2017-06-01-22-48-32-552.png
          13 kB
        8. screenshot-1.png
          screenshot-1.png
          12 kB

          [JENKINS-42034] Reactive Reference parameter with Formated HTML loses script section when binding to other build parameters

          Not sure what the issue is about exactly. Had a quick look at the issue, but didn't have enough time to read all the comments.

          If someone could summarize the issue, maybe I can help? We have 4 small/medium issues fixed already, besides the security issue in Scriptler that is blocking our release.

          In the meantime I think we can squeeze a few more issues likes this for the next release

          Bruno P. Kinoshita added a comment - Not sure what the issue is about exactly. Had a quick look at the issue, but didn't have enough time to read all the comments. If someone could summarize the issue, maybe I can help? We have 4 small/medium issues fixed already, besides the security issue in Scriptler that is blocking our release. In the meantime I think we can squeeze a few more issues likes this for the next release

          Bill Matzen added a comment -

          I would be so appreciative if you could take a look into this one. I understand the comment thread is pretty long and has evolved a bit from the original message. But if you focus on the one from 5/4/2017 (beginning with "I can duplicate this in another environment that has nothing to do with Javascript..."), that is the most isolated/concise example, and the root issue that is causing me grief. If this could be working around or dare I say "fixed" (if it is truly a defect), we would be able to do some really cool things with this plugin (beyond what we're already doing!).

          Bill Matzen added a comment - I would be so appreciative if you could take a look into this one. I understand the comment thread is pretty long and has evolved a bit from the original message. But if you focus on the one from 5/4/2017 (beginning with "I can duplicate this in another environment that has nothing to do with Javascript..."), that is the most isolated/concise example, and the root issue that is causing me grief. If this could be working around or dare I say "fixed" (if it is truly a defect), we would be able to do some really cool things with this plugin (beyond what we're already doing!).

          Bruno P. Kinoshita added a comment - - edited

          Hi bmatzen, thanks a lot for attaching a config.xml file. It saves me minutes, and sometimes hours, trying to reproduce the issue

           In less than 5 minutes I successfully reproduced your issue. However, at least in Eclipse I can see a serialization exception, where Jenkins' Stapler (its web framework), or more specifically one of its dependencies responsible for parsing, complains about a certain type of object that it could not understand.

           The issue is, from what I could tell, in this part of your active-choices script:

          a.b.c.@Name.each{{{}}
               println "Found name '" + it
            names << it}

          The `it` variable in the loop will be an instance of `groovy.util.slurpersupport.Attribute`. You are adding it to the array names (which doesn't use any type as with generics in Java).

          The array of `groovy.util.slurpersupport.Attribute` objects is returned to the client, but the framework fails to serialize them, and so you only see the old value, that was calculated in the first time the parameter was executed.

          To solve the issue, you have to make sure to send collections of strings, or elements that the framework will successfully serialize as strings (better stick to always returning strings if possible). Here's a quick way to fix it:

          a.b.c.@Name.each{{{}}
          {{       println "Found name '" + it}}
          {{       names << it.text()}}
          }

          The `.text()` method is documented here.

          Tested on Eclipse IDE, with the latest version from git master branch, but I believe it should work with 1.5.3 as well. Thanks a lot for your patience, and let us know if that works!

          Cheers

          Bruno

          Bruno P. Kinoshita added a comment - - edited Hi bmatzen , thanks a lot for attaching a config.xml file. It saves me minutes, and sometimes hours, trying to reproduce the issue  In less than 5 minutes I successfully reproduced your issue. However, at least in Eclipse I can see a serialization exception, where Jenkins' Stapler (its web framework), or more specifically one of its dependencies responsible for parsing, complains about a certain type of object that it could not understand.  The issue is, from what I could tell, in this part of your active-choices script: a.b.c.@Name.each {{{}}      p rintln "Found name '" + it   names << it } The `it` variable in the loop will be an instance of `groovy.util.slurpersupport.Attribute`. You are adding it to the array names (which doesn't use any type as with generics in Java). The array of `groovy.util.slurpersupport.Attribute` objects is returned to the client, but the framework fails to serialize them, and so you only see the old value, that was calculated in the first time the parameter was executed. To solve the issue, you have to make sure to send collections of strings, or elements that the framework will successfully serialize as strings (better stick to always returning strings if possible). Here's a quick way to fix it: a.b.c.@Name.each {{{}} {{       println "Found name '" + it}} {{       names << it.text()}} } The `.text()` method is documented here . Tested on Eclipse IDE, with the latest version from git master branch, but I believe it should work with 1.5.3 as well. Thanks a lot for your patience, and let us know if that works! Cheers Bruno

          JIRA is not happy with my formatting, sorry. Bill, hope you know which part of the code I'm referring to. Let me know if you want me to upload my updated config.xml. Ignore the extra { }`s that JIRA keeps adding to my preformatted blocks (easier to work on Java code than on this JIRA editor )

          Bruno P. Kinoshita added a comment - JIRA is not happy with my formatting, sorry. Bill, hope you know which part of the code I'm referring to. Let me know if you want me to upload my updated config.xml. Ignore the extra { }`s that JIRA keeps adding to my preformatted blocks (easier to work on Java code than on this JIRA editor )

          Bill Matzen added a comment -

          Hi Bruno, thank you for your investigation into this - this does in fact resolve that sample. and I'll watch out for those types of issues in the future.
          The prior example (2017-05-03 16:47), however, remains an issue, and appears unrelated to this. This does go back to Javascript so a little more complex, but if you wouldn't mind focusing in on that comment / attachment, that remains a sticking point for me. If you could also touch on the "double load" behavior - that strongly plays into this - that would be informative.  Again, thank you so much for this plugin and for investigation my issues.

          Thanks,
          Bill

          Bill Matzen added a comment - Hi Bruno, thank you for your investigation into this - this does in fact resolve that sample. and I'll watch out for those types of issues in the future. The prior example (2017-05-03 16:47), however, remains an issue, and appears unrelated to this. This does go back to Javascript so a little more complex, but if you wouldn't mind focusing in on that comment / attachment, that remains a sticking point for me. If you could also touch on the "double load" behavior - that strongly plays into this - that would be informative.  Again, thank you so much for this plugin and for investigation my issues. Thanks, Bill

          Hi bmatzen, glad it worked.

          Searching for 2017-05-03 on this ticket doesn't return anything for me. In your previous comments, only when you quoted a part of the text I could find the attachment.

          I *think* JIRA takes into consideration each user's time zone when displaying comments and attachments. As I live in a weird time zone, probably what I see is different than most others here  Could you point to a file name, or quote the beginning of the comment, please, Bill?

          I will try to chase what's going on in the JavaScript layer. I am aware of parameters being evaluated twice. But as it is normally harmless, and I never found a simple explanation, I've been postponing working on this. Maybe now we can spend some time investigating it together.

          Thanks and sorry for the mess with time zones & comments.

          Bruno

          Bruno P. Kinoshita added a comment - Hi bmatzen , glad it worked. Searching for 2017-05-03 on this ticket doesn't return anything for me. In your previous comments, only when you quoted a part of the text I could find the attachment. I * think * JIRA takes into consideration each user's time zone when displaying comments and attachments. As I live in a weird time zone, probably what I see is different than most others here  Could you point to a file name, or quote the beginning of the comment, please, Bill? I will try to chase what's going on in the JavaScript layer. I am aware of parameters being evaluated twice. But as it is normally harmless, and I never found a simple explanation, I've been postponing working on this. Maybe now we can spend some time investigating it together. Thanks and sorry for the mess with time zones & comments. Bruno

          Bill Matzen added a comment -

          Sorry about that Bruno - ya, the one that starts out "I believe I have narrowed down what is happening here" and references "config-doubleload.xml"...

          Thanks,
          Bill

          Bill Matzen added a comment - Sorry about that Bruno - ya, the one that starts out "I believe I have narrowed down what is happening here" and references "config-doubleload.xml"... Thanks, Bill

          Not a problem, and thanks for the prompt reply.

          Here's what is says for me on that comment: bmatzen added a comment - 2017-05-04 11:47, so I believe JIRA is indeed storing some UTC value and then rendering the currentUser() tz time. Would be good to have a comment ID somewhere I think... sorry for digressing.

          Will update the ticket once I have time to investigate it (bit busy with the biouno website today)

          Bruno

          Bruno P. Kinoshita added a comment - Not a problem, and thanks for the prompt reply. Here's what is says for me on that comment:  bmatzen  added a comment - 2017-05-04 11:47, so I believe JIRA is indeed storing some UTC value and then rendering the currentUser() tz time. Would be good to have a comment ID somewhere I think... sorry for digressing. Will update the ticket once I have time to investigate it (bit busy with the biouno website today) Bruno

          Hmmm, did some initial analysis, and there are a few different ways to fix it. I think the scripts are evaluated many times because we have, for the cascade parameters and for dynamic parameters, they are updated once they are displayed on the UI.

          There is an old issue, somewhere in JIRA (bad memory, sorry) where users had to click somewhere to active check boxes or radio buttons. It is tricky to control the DOM creation and the dynamic parameters. So it was the easiest solution at that time (I think it was after a major refactoring in the code, when it was uno-choice, in our own update centre).

          Now it looks like we need to properly fix it. I will log an issue for that, linking to this one. Feel free to watch and comment on that issue.

          What I have in my mind as a possible and good solution is:

          • Create a graph/tree of all parameters, hierarchically
          • Walk the graph/tree, evaluating each parameter
          • Re-use this graph model to evaluate active-choices parameters without an UI

          This last option has been on my mind for a while. There is another old issue, to support timers/pipelines/cron jobs/etc. But given the current nature of the plug-in, it is impossible to evaluate plug-ins without an UI.

          With this graph/tree model, we would solve the issue of evaluating parameters once in the UI (DOM/JS/HTML), and also adding the ability to have new backend parameters (Java only).

           

          Bruno P. Kinoshita added a comment - Hmmm, did some initial analysis, and there are a few different ways to fix it. I think the scripts are evaluated many times because we have, for the cascade parameters and for dynamic parameters , they are updated once they are displayed on the UI. There is an old issue, somewhere in JIRA (bad memory, sorry) where users had to click somewhere to active check boxes or radio buttons. It is tricky to control the DOM creation and the dynamic parameters. So it was the easiest solution at that time (I think it was after a major refactoring in the code, when it was uno-choice, in our own update centre). Now it looks like we need to properly fix it. I will log an issue for that, linking to this one. Feel free to watch and comment on that issue. What I have in my mind as a possible and good solution is: Create a graph/tree of all parameters, hierarchically Walk the graph/tree, evaluating each parameter Re-use this graph model to evaluate active-choices parameters without an UI This last option has been on my mind for a while. There is another old issue, to support timers/pipelines/cron jobs/etc. But given the current nature of the plug-in, it is impossible to evaluate plug-ins without an UI. With this graph/tree model, we would solve the issue of evaluating parameters once in the UI (DOM/JS/HTML), and also adding the ability to have new backend parameters (Java only).  

          Bill Matzen added a comment -

          This would be great Bruno - I see the new issue (JENKINS-45507) and will monitor that for updates. I'm also more than happy to test anything out before you officially release any update. Thank you so much for investigating this and getting to the root issue.

          Thanks,
          Bill

          Bill Matzen added a comment - This would be great Bruno - I see the new issue ( JENKINS-45507 ) and will monitor that for updates. I'm also more than happy to test anything out before you officially release any update. Thank you so much for investigating this and getting to the root issue. Thanks, Bill

            ioannis Ioannis Moutsatsos
            bmatzen Bill Matzen
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: