[MYFACES-2801] HtmlResponseWriterImpl needs to deal with <script> and <style> tags properly on both html and xhtml

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.0.0
• Fix Version/s: 2.0.1
• Component/s: JSR-314
• Assignee: Leonardo Uribe
• Reporter: Leonardo Uribe
• Created: 09/Jul/10 04:55
• Updated: 26/Jul/11 19:36
• Resolved: 10/Jul/10 22:19

Description

I started to solve this issue on MYFACES-2779, but definitively it deserves to have a separate issue.

HtmlResponseWriterImpl needs to deal with <script> and <style> tags properly on both html and xhtml. There is a very old issue (MYFACES-1353) that suggest a review now donated trinidad code for xhtml.

After checking the ResponseWriter implementations of trinidad, and do some test on ri about, the following points should be taken into account before solve this issue:

• When jsp is used, part of the page is rendered by it and the other part are renderer by jsf using the ResponseWriter implementation. That means if there is a <script> on a page, no xml comment wraps the content. But the things are different in facelets, because the jsf ResponseWriter implementation is used to render everything. So, if facelets founds a <script> or <style>, it call ResponseWriter.startElement and ResponseWriter.endElement and all related methods.

• When facelets is used, we first parse .xhtml files with a SAX parser. That means invalid xml markup makes facelets fail. For example:

<script type="text/javascript">
alert("<EM>This won't work<\/EM>");
</script>

Since <script> and <style> content is PCDATA on the dtd, the SAX parser does not make distinction and parse the code inside <script> as if the content was more xhtml.

• The following notation is valid for text/html responses:

<style type="text/css">
.x23

{ background:red; }

</style>

<style type="text/css">
<![CDATA[
.x24

{ background:red; }

]]>
</style>

<style type="text/css">
/* <![CDATA[ */
.x25

{ background:red; }

/* ]]> */
</style>

<script type="text/javascript">
/* <![CDATA[ */
alert("<EM>This work 1</EM>");
/* ]]> */
</script>

<script type="text/javascript">
//<![CDATA[
alert("<EM>This work 2</EM>");
//]]>
</script>

<script type="text/javascript">
<!--
alert("<EM>This work 3</EM>");
//-->
</script>

<script type="text/javascript">
alert("This work 4");
</script>

• This syntax is not valid in xhtml:

<script type="text/javascript">
<!--
alert("<EM>This won't work in xhtml</EM>");
//-->
</script>

In this case, using //<![CDATA[ or /<![CDATA[/ is preferred

• This syntax pass the SAX parser but fails later if text/html content type is used and IE.

<script type="text/javascript">
<![CDATA[
/* Only valid if it is used Xhtml as XML */
alert("<EM>This work on firefox</EM>");
]]>
</script>

• "<!-" and "//->" could be added as wrapper for <script> content when text/html is used.

• "//<![CDATA[" and "//]]>" must be added as a wrapper for <script> and <style> (without "//") content and xhtml is used. The reason is in html, <script> and <style> content are considered cdata, but in xhtml it is pcdata by the dtd, but in fact, the intention is, content inside these tags should not be parsed by the web client. Mojarra append this blocks automatically, and I think it is fine to do this too.

• The set used to prevent include content inside tags that should be empty should only be used in text/html reponses. In xhtml this is not required.

• Trinidad HtmlResponseWriter has a fast solution to check for empty html elements. We should include it on myfaces.

Issue Links

• is duplicated by: MYFACES-1963 <script src=""> generates extra html comments
• is part of: MYFACES-2779 PartialResponse bogus response on render all because of falsely positioned development stage script tag