Created attachment 22665 [details] replace whitespace with single space Enabling trimSpaces collapses all spaces to nothing which can end up removing desired whitespace from the generated html. A possible solution to this is to reduce whitespace to a single space rather than removing it altogether. The attached patch has this implementation.
trimSpaces is equivalent to trimDirectiveWhitespaces Section JSP.3.3.8 is clear that the whitespace must be removed completely. trimSpaces can currently be true or false. I would not be against an enhancement that added the value "single" and in that case using the code in your patch. Let us know if you need some pointers on writing a new patch.
Hi Mark, Thanks for the comment. The main question I have is whether you'd want to leave the -trimspaces option for JspC backwards-compatible (ie, if there's no "true", "false", or "single" after it, treat it as "true"), or whether you'd be ok with changing the behavior to requiring the value after the flag. Also, if there's anything else that's important to keep in mind when writing a patch for tomcat, please share. Thanks! (In reply to comment #1) > trimSpaces is equivalent to trimDirectiveWhitespaces > > Section JSP.3.3.8 is clear that the whitespace must be removed completely. > > trimSpaces can currently be true or false. I would not be against an > enhancement that added the value "single" and in that case using the code in > your patch. > > Let us know if you need some pointers on writing a new patch. >
Keep the current behaviour (ie true) if nothing is specified for trimspaces since that is backwards compatible. Things to keep in mind: - patches should be in diff -u format - avoid changing stuff that isn't relevant (ie don't mix a functional patch with code clean-up) - keep the patch as simple as you can Note I have re-opened this as an enhancement so it doesn't get lost.
Created attachment 22843 [details] Adds a third state for the trimSpaces flag This patch adds the "single" value for trimSpaces. I wasn't able to find any automated tests for jasper so I did lots of manual testing. If there is a test suite and someone can point me to it, I can add in an automated test or two. Also, when trying to manually test this, I noticed that precompiling tags that worked against 6.0.18 didn't work against the trunk. So, I was able to test the embeddable servlet option functionality against the trunk but only able to test the jspc functionality against 6.0.18. Also, none of the places of code affected by the patch appear to have been changed between 6.0.18 and the current trunk.
Hi all, I was just wondering if there was a timeline for review/inclusion of this patch. Thanks!
(In reply to comment #5) > Hi all, I was just wondering if there was a timeline for review/inclusion of > this patch. Thanks! I'm wondering this as well. Any ETA for a release?
I think it's worth mentioning that adding this feature potentially makes Tomcat incompatible with other app servers: if people use this "single" feature, and then move to another app server that doesn't support this kind of configuration, they'll have to change all their JSPs to suit. If this feature is added, it should be very clear in the documentation that its use should be restricted to dire circumstances, and that JSP authors should be using the more portable workaround to forcing spaces to appear between directives, etc.: ${thingOne}${' '}${thingTwo}
(In reply to comment #7) > I think it's worth mentioning that adding this feature potentially makes Tomcat > incompatible with other app servers: if people use this "single" feature, and > then move to another app server that doesn't support this kind of > configuration, they'll have to change all their JSPs to suit. > In other web servers the space will not be removed at all. They will render several spaces instead of that single one. Nothing is lost, but just wasting some bandwidth.
First of all, let me apologise for letting this one slip through the cracks. I'm currently reviewing all open Tomcat enhancement requests as part of the end-of-life process. I am struggling to see a use case for this option. trimSpaces applies to each block of template text in turn. I'm struggling to see a (non-contrived) use case where the option for a single space would be useful. Therefore, I am resolving this as WONTFIX. I'd be more than happy to revisit this but I would ask that a sample JSP be provided (ideally one that works when dropped into a clean Tomcat install) that demonstrates a use case for this new option.
Hello, I am reopening this enhancement request, at the request of Mark Thomas. I have come across an issue with trimSpace removing white space from parts of the HTML code. The use case is found in the following mailing list thread. http://mail-archives.apache.org/mod_mbox/tomcat-users/201705.mbox/%3CCAFOY9=DN=Y-_7iZm38Kf4waGSOpzw4NyuLYOLA_PLxfm7VO4Cg@mail.gmail.com%3E Thanks
A variation of this patch has been applied to 9.0.x for 9.0.0.M22 onwards.