[#HUDSON-5633] Missing spaces in rendered HTML in 1.346 - Hudson JIRA

Hudson

Missing spaces in rendered HTML in 1.346

Created: 13/Feb/10 09:54 AM Updated: 15/Feb/10 01:28 PM Resolved: 15/Feb/10 01:28 PM

Component/s:

gui

Affects Version/s:

None

Fix Version/s:

None

Description

« Hide

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?

Sort Order:

[ Permalink | « Hide ]

mindless added a comment - 13/Feb/10 04:00 PM

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

[ Permalink | « Hide ]

abayer added a comment - 13/Feb/10 05:03 PM

Yeah, I believe it can be in that case.

[ Permalink | « Hide ]

kohsuke added a comment - 15/Feb/10 09:27 AM

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?

[ Permalink | « Hide ]

mindless added a comment - 15/Feb/10 11:09 AM

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

[ Permalink | « Hide ]

kohsuke added a comment - 15/Feb/10 11:12 AM

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

[ Permalink | « Hide ]

scm_issue_link added a comment - 15/Feb/10 01:28 PM

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://hudson-ci.org/commit/27528
Log:
[FIXED HUDSON-5633] restored the backward compatible whitespace normalization behavior in Jelly