Bug 51490 - if.jsp has many bugs and c:if example not useful
Summary: if.jsp has many bugs and c:if example not useful
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Examples (show other bugs)
Version: 7.0.16
Hardware: PC All
: P2 minor (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-08 18:05 UTC by charles
Modified: 2011-07-11 09:39 UTC (History)
1 user (show)



Attachments
Three versions of the "if" example, including a fix. (2.82 KB, application/zip)
2011-07-08 18:05 UTC, charles
Details
Modified example illustrating issues. (1.52 KB, text/html)
2011-07-08 20:25 UTC, charles
Details

Note You need to log in before you can comment on or make changes to this bug.
Description charles 2011-07-08 18:05:44 UTC
Created attachment 27276 [details]
Three versions of the "if" example, including a fix.

See:
  apache-tomcat-7.0.16/webapps/examples/jsp/tagplugin/if.jsp

Small things:
 * Has unclosed font tags and hr tags (old HTML)
 * Has doubled font tags: "<font <font" (broken)
 * Uses "</br>" sometimes (broken) - should be "<br/>" always

Medium thing:
 * The content in the following prints even when it's not working!  E.g., if the jstl.jar is not there!
    <c:if test="${2>0}">
        It's true that (2>0)!
    </c:if>
The User Agent simply prints the content of the unknown tag.

Please add after this:
    <br/>
    <c:if test="${0>2}">
	  If you see this, then (0>2)! Fail!
    </c:if>


Major things:
 * Must use the old non-XML syntax with .jsp file suffix:
      <%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
 * The scope for "theTruth" should be "page" to avoid testing issues.

If the taglib line is removed and the following is added to the <html> tag:
      xmlns:c="http://java.sun.com/jsp/jstl/core"
then the example does NOT work, even though it might look like it does!
In this case, "theTruth" is still set from if.jsp (if scope="session") and the contents of the c:if tag still appear, even with no JSTL processing.

To use xmlns:c rather than the taglib line, the developer must
1) Switch to the .jspx file suffix
2) Use the jsp:root element
3) Or configure Tomcat to treat .jsp as XML

The above is not clear, especially when the example appears to work in many cases when it should not.

I have attached:
1) A fixed if.jsp - please use this.
2) An if.jspx version - please include this for contrast.
3) An if-xmlns.jsp version that does NOT work, but appears to work if if.jsp is visited first.  This is to test the example.

Thanks!
Comment 1 Christopher Schultz 2011-07-08 19:51:02 UTC
A couple of notes:

1. It might be nice to clean-up the sloppy HTML. Patches, even (especially!) for documentation are always appreciated.

2. For your "medium thing", you say neither what the output should be nor what the output actually is. Please be absolutely clear when filing a bug report.

3. Failure to use JSTL properly does not constitute a bug in Tomcat. For example, specifying xmlns:c="..." in the <html> element of a .jsp file is not a valid way to reference a taglib and should not be expected to work. Nor is this something that Tomcat can reasonably detect and protect the user against.

4. For the if-xmlns-works-after-if-but-not-before problem, we'll have to look at that.
Comment 2 charles 2011-07-08 20:25:21 UTC
Created attachment 27277 [details]
Modified example illustrating issues.
Comment 3 Konstantin Kolinko 2011-07-08 20:53:56 UTC
It would be nice if you were providing patches in svn diff (or diff -u) format,
http://tomcat.apache.org/bugreport.html#Patches_and_enhancement_requests

The source code for the examples in Tomcat 7 is
http://svn.apache.org/repos/asf/tomcat/trunk/webapps/examples/



I would be -1 for any cleanup for if.jsp, foreach.jsp and choose.jsp.  Those are examples not for the JSTL library but for Tomcat Tag Plugins. That is a different beast.

I suspect that broken HTML code there is intended (to better test this feature), though I am a bit lazy to look up through the history of those files.
Comment 4 charles 2011-07-08 21:00:56 UTC
Hi Christopher,
Thanks for looking at this!

For (1), I hope that the example is updated at some point in the near future.

For (2):
===== Begin Original if.jsp output ===
Tag Plugin Examples - <c:if>

Plugin Introductory Notes
Brief Instructions for Writing Plugins


Set the test result to a variable
The result of testing for (1==1) is: true
Conditionally execute the body
It's true that (2>0)! 
===== End Original if.jsp output ===

Now we look on the web and see that the taglib line can be replaced with an
xmlns:c attribute: xmlns:c="http://java.sun.com/jsp/jstl/core".
We delete the taglib line and add the attribute.
See if-bogus.jsp attached ("Modified example illustrating the issues").

If we visit if-bogus.jsp, the output is exactly the same as above!
If we restart Tomcat and visit if-bogus.jsp, then the output is exactly the
same again!
If we refresh the page, we finally get something slightly different
(the "true" disappears, but the "2>0" line remains):
===== Begin if-bogus.jsp output =====
Tag Plugin Examples - <c:if>

Plugin Introductory Notes
Brief Instructions for Writing Plugins


Set the test result to a variable
The result of testing for (1==1) is:
Conditionally execute the body
It's true that (2>0)! 
===== End if-bogus.jsp output =====

If we visit if.jsp again, it sets the "theTruth" session variable.
Now if we go back to if-bogus.jsp and refresh, it looks exactly like the
"Original if.jsp output" - it looks like it's working, but it's not.

I sent an updated if.jsp in the first attachment.
This fixes HTML issues and makes it clear when things are actually working:
1) Use scope="page" to avoid side effects from other example pages
2) Use a c:if that actually depends on JSTL working.

The output of the updated if.jsp example is exactly the same as before, but
only if things are working.

Here is the output from if-xmlns.jsp (same as if-bogus.jsp except based on the
updated if.jsp example):

===== Begin if-xmlns.jsp output =====
Tag Plugin Examples - <c:if>

Plugin Introductory Notes>
Brief Instructions for Writing Plugins


Set the test result to a variable
The result of testing for (1==1) is:
Conditionally execute the body
It's true that (2>0)!
If you see this, then (0>2)! Fail! 
===== Begin if-xmlns.jsp output =====

In the above output, note that
1) the "true" is not there (as expected)
2) the misleading (2>0) line is still there, but
3) a new line gets printed indicating that it's not actually working

New note:
Apparently, a scope="session" variable will be used by a scope="page" variable
of the same name.

After restarting Tomcat, then it's possible to switch between the updated
if.jsp and if-xmlns.jsp and the output will not change (no side effects, but a
restart is required as explained in "new note" above).

I agree with your note on (3) regarding failure to use JSTL properly.
Note that updating the example will help tremendously with (3)!
Currently, people look at the web, create something like if-xmlns.jsp, and then
think (at first) that it's working.

(4) the if-xmlns-works-after-if-but-not-before problem
This is just a scoping issue for "theTruth" as pointed out and as corrected in
the updated if.jsp example.

I hope that this helps.
Comment 5 charles 2011-07-08 21:38:06 UTC
(In reply to comment #4)
> ===== Begin if-xmlns.jsp output =====
> Tag Plugin Examples - <c:if>
> 
> Plugin Introductory Notes>
> Brief Instructions for Writing Plugins
> 
> 
> Set the test result to a variable
> The result of testing for (1==1) is:
> Conditionally execute the body
> It's true that (2>0)!
> If you see this, then (0>2)! Fail! 
> ===== End if-xmlns.jsp output =====

Should be "End" on the line above rather than "Begin".

> After restarting Tomcat, then it's possible to switch between the updated
> if.jsp and if-xmlns.jsp and the output will not change (no side effects, but a
> restart is required as explained in "new note" above).

Clarification:
The output will not change for the *respective* updated if.jsp and if-xmlns.jsp examples.  This is how it should be if there are no side effects between the two.
Comment 6 Mark Thomas 2011-07-11 09:39:26 UTC
The issues with the c:if examples have been fixed in trunk and will be included in the next release.

I echo Konstantin's comments that diffs in the correct format are much easier to work with than an entirely new file.