Author: markt
Date: Thu Feb 18 10:27:33 2010
New Revision: 911310

URL: http://svn.apache.org/viewvc?rev=911310&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48616
This is a regression caused by the fix for 
https://issues.apache.org/bugzilla/show_bug.cgi?id=42390
JspFragments are scriptless, so no need to declare or sync scripting variables 
for fragments. Since errors in syncing the scripting variables for JSP 
Fragments caused 48616 & 42390, this fixes both these bugs too. (kkolinko)

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Feb 18 10:27:33 2010
@@ -143,18 +143,6 @@
   +1: markt, kkolinko
   -1: 
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48616
-  This is a regression caused by the fix for
-  https://issues.apache.org/bugzilla/show_bug.cgi?id=42390
-  JspFragments are scriptless, so no need to declare or sync scripting
-  variables for fragments. Since errors in syncing the scripting variables for
-  JSP Fragments caused 48616 & 42390, this fixes both these bugs too.
-  https://issues.apache.org/bugzilla/show_bug.cgi?id=48616#c21
-  (https://issues.apache.org/bugzilla/attachment.cgi?id=24992)
-  +1: kkolinko, markt, mturk
-  -1:
-
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48612
   Prevent exception on shutdown
   Port of r896193 and r905343

Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java 
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java Thu Feb 
18 10:27:33 2010
@@ -333,6 +333,9 @@
             }
 
             public void visit(Node.CustomTag n) throws JasperException {
+                // XXX - Actually there is no need to declare those
+                // "_jspx_" + varName + "_" + nestingLevel variables when we 
are
+                // inside a JspFragment.
 
                 if (n.getCustomNestingLevel() > 0) {
                     TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
@@ -2484,6 +2487,11 @@
         }
 
         private void declareScriptingVars(Node.CustomTag n, int scope) {
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                return;
+            }
 
             Vector vec = n.getScriptingVars(scope);
             if (vec != null) {
@@ -2531,6 +2539,14 @@
             if (n.getCustomNestingLevel() == 0) {
                 return;
             }
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                // Thus, there is no need to save/ restore/ sync them.
+                // Note, that JspContextWrapper.syncFoo() methods will take
+                // care of saving/ restoring/ sync'ing of JspContext 
attributes.
+                return;
+            }
 
             TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
             VariableInfo[] varInfos = n.getVariableInfos();
@@ -2591,6 +2607,14 @@
             if (n.getCustomNestingLevel() == 0) {
                 return;
             }
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                // Thus, there is no need to save/ restore/ sync them.
+                // Note, that JspContextWrapper.syncFoo() methods will take
+                // care of saving/ restoring/ sync'ing of JspContext 
attributes.
+                return;
+            }
 
             TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
             VariableInfo[] varInfos = n.getVariableInfos();
@@ -2645,6 +2669,15 @@
          * given scope.
          */
         private void syncScriptingVars(Node.CustomTag n, int scope) {
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                // Thus, there is no need to save/ restore/ sync them.
+                // Note, that JspContextWrapper.syncFoo() methods will take
+                // care of saving/ restoring/ sync'ing of JspContext 
attributes.
+                return;
+            }
+
             TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
             VariableInfo[] varInfos = n.getVariableInfos();
 

Modified: 
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- 
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java 
(original)
+++ 
tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java 
Thu Feb 18 10:27:33 2010
@@ -58,17 +58,17 @@
     static class ScriptingVariableVisitor extends Node.Visitor {
 
        private ErrorDispatcher err;
-       private Hashtable scriptVars;
+       private Map<String, Integer> scriptVars;
        
        public ScriptingVariableVisitor(ErrorDispatcher err) {
            this.err = err;
-           scriptVars = new Hashtable();
+           scriptVars = new HashMap<String, Integer>();
        }
 
        public void visit(Node.CustomTag n) throws JasperException {
            setScriptingVars(n, VariableInfo.AT_BEGIN);
            setScriptingVars(n, VariableInfo.NESTED);
-           new ScriptingVariableVisitor(err).visitBody(n);
+           visitBody(n);
            setScriptingVars(n, VariableInfo.AT_END);
        }
 
@@ -104,7 +104,7 @@
                    }
                    String varName = varInfos[i].getVarName();
                    
-                   Integer currentRange = (Integer) scriptVars.get(varName);
+                   Integer currentRange = scriptVars.get(varName);
                    if (currentRange == null
                            || ownRange.compareTo(currentRange) > 0) {
                        scriptVars.put(varName, ownRange);
@@ -127,7 +127,7 @@
                        }
                    }
 
-                   Integer currentRange = (Integer) scriptVars.get(varName);
+                   Integer currentRange = scriptVars.get(varName);
                    if (currentRange == null
                            || ownRange.compareTo(currentRange) > 0) {
                        scriptVars.put(varName, ownRange);

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Feb 18 10:27:33 2010
@@ -57,6 +57,12 @@
         Add some debug logging to the compiler where exceptions were previously
         swallowed. (markt)
       </add>
+      <fix>
+        <bug>48616</bug>: Don't declare or syncrhonize scripting variables for
+        JSP fragments since they are scriptless. This is an alternative fix for
+        <bug>42390</bug> that avoids both the original problem and the
+        regression in the first fix. (kkolinko) 
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to