• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Major Major
    • core
    • None

      I see that there have been changes to HTML rendering in 1.346. This seems to have caused at least one regression: under Build Executor Status, I see for example

      BuildingNB-Core-Build #4033

      rather than the expected

      Building NB-Core-Build #4033

      I am guessing that this is the issue (lib/hudson/executors.jelly):

      ${%Building}
      <!-- XML... -->
      <a href="...">${...fullDisplayName}</a>&#160;<a href="$...">#${....number}</a>
      

      If you turn off ignorable whitespace processing, then no space appears between "Building" and the job name. Using a nbsp is not a good idea here for layout reasons, so perhaps

      ${%Building}&#32;
      

      is needed?

      There might be other places with the same regression - can the regression be fixed at its source (Stapler?) for better compatibility, especially with plugins?

          [JENKINS-5633] Missing spaces in rendered HTML in 1.346

          Alan Harder added a comment -

          I guess change for JENKINS-5632 can be reverted if a more general fix is made? (and changelog.html item adjusted)

          Alan Harder added a comment - I guess change for JENKINS-5632 can be reverted if a more general fix is made? (and changelog.html item adjusted)

          Andrew Bayer added a comment -

          Yeah, I believe it can be in that case.

          Andrew Bayer added a comment - Yeah, I believe it can be in that case.

          I looked into this in Jelly, and realized that the original whitespace normalization rules in Jelly is messed up.

          It appears to be modeled after the whitespace normalization in XSLT — that is,

          1. designated elements marked as "preserve whitespace" will keep their whitespace.
          2. otherwise, text nodes in the stylesheet that consists entirely of whitespace are removed.

          The "bug" that makes Jelly behave differently from XSLT is the whitespace normalization handling when expressions ${...} are involved. In which case the leading and trailing whitespace is stripped. IOW, "<a> CR text CR </a>" would evaluate to "<a> CR text CR </a>" without any change, but "<a> CR ${expr} CR </a>" would evaluate to "<a>${expr}</a>", except when expressions show up in the mixed content model, so "<a> CR ${expr} <b/></a>" would still evaluate to "<a> CR ${expr} <b/></a>".

          IMHO, this is a mess.

          I'm pretty sure the same logic can be restored, but I'm entertaining the possibility that maybe we can take a hit and bring in a consistent whitespace normalization rule.

          The obvious other arguments are that the compatibility is a king and we haven't heard from anyone so far complaining the old strange whitespace normalization rule (including myself.)

          Any thoughts?

          Kohsuke Kawaguchi added a comment - I looked into this in Jelly, and realized that the original whitespace normalization rules in Jelly is messed up. It appears to be modeled after the whitespace normalization in XSLT — that is, 1. designated elements marked as "preserve whitespace" will keep their whitespace. 2. otherwise, text nodes in the stylesheet that consists entirely of whitespace are removed. The "bug" that makes Jelly behave differently from XSLT is the whitespace normalization handling when expressions ${...} are involved. In which case the leading and trailing whitespace is stripped. IOW, "<a> CR text CR </a>" would evaluate to "<a> CR text CR </a>" without any change, but "<a> CR ${expr} CR </a>" would evaluate to "<a>${expr}</a>", except when expressions show up in the mixed content model, so "<a> CR ${expr} <b/></a>" would still evaluate to "<a> CR ${expr} <b/></a>". IMHO, this is a mess. I'm pretty sure the same logic can be restored, but I'm entertaining the possibility that maybe we can take a hit and bring in a consistent whitespace normalization rule. The obvious other arguments are that the compatibility is a king and we haven't heard from anyone so far complaining the old strange whitespace normalization rule (including myself.) Any thoughts?

          Alan Harder added a comment -

          Does the code in place now have a "consistent whitespace normalization rule", or would further changes be needed?

          Guess I'm ok either way.. I'm sure I've hit oddities here, but when I did I just added/removed whitespace to make it work as I wanted and moved on. "Fixing" this will likely lead to minor issues filed over the next several releases as people find affected jellies (or can we find these cases somehow?).. one could argue that a change that makes people need to use <j:whitespace> a lot isn't really an "improvement" though

          Alan Harder added a comment - Does the code in place now have a "consistent whitespace normalization rule", or would further changes be needed? Guess I'm ok either way.. I'm sure I've hit oddities here, but when I did I just added/removed whitespace to make it work as I wanted and moved on. "Fixing" this will likely lead to minor issues filed over the next several releases as people find affected jellies (or can we find these cases somehow?).. one could argue that a change that makes people need to use <j:whitespace> a lot isn't really an "improvement" though

          After thinking about it some more, I decided to just resurrect the original behavior.

          Kohsuke Kawaguchi added a comment - After thinking about it some more, I decided to just resurrect the original behavior.

          Code changed in hudson
          User: : kohsuke
          Path:
          trunk/hudson/main/core/pom.xml
          trunk/hudson/main/core/src/main/resources/hudson/model/AbstractProject/main.jelly
          trunk/hudson/main/core/src/main/resources/hudson/model/AllView/noJob.jelly
          trunk/hudson/main/core/src/main/resources/hudson/tasks/test/AbstractTestResultAction/summary.jelly
          trunk/www/changelog.html
          http://jenkins-ci.org/commit/27528
          Log:
          [FIXED JENKINS-5633] restored the backward compatible whitespace normalization behavior in Jelly

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : kohsuke Path: trunk/hudson/main/core/pom.xml trunk/hudson/main/core/src/main/resources/hudson/model/AbstractProject/main.jelly trunk/hudson/main/core/src/main/resources/hudson/model/AllView/noJob.jelly trunk/hudson/main/core/src/main/resources/hudson/tasks/test/AbstractTestResultAction/summary.jelly trunk/www/changelog.html http://jenkins-ci.org/commit/27528 Log: [FIXED JENKINS-5633] restored the backward compatible whitespace normalization behavior in Jelly

            Unassigned Unassigned
            jglick Jesse Glick
            Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:
              Resolved: