Author: kkolinko
Date: Sun Feb 14 06:47:40 2010
New Revision: 909979

URL: http://svn.apache.org/viewvc?rev=909979&view=rev
Log:
veto

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=909979&r1=909978&r2=909979&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sun Feb 14 06:47:40 2010
@@ -185,7 +185,33 @@
   Test cases for both bugs and the JSP TCK pass with this patch applied.
   http://people.apache.org/~markt/patches/2010-02-02-bug42390.patch
   +1: markt
-  -1: 
+  -1: kkolinko:
+    1. There is a copy-paste issue in the patch:
+       vec.add(tagVarInfos[i]); call was replaced by vec.add(varInfos[i]);
+       which is wrong. This error is not present in trunk.
+
+    2. isImplemetedAsFragment() method is wrong.
+       1) A fragment can also be created with <jsp:attribute> when calling a 
tag,
+         when the attribute is declared being of type JspFragment.
+         -see JSP.5.10 <jsp:attribute>, or the places where the following 
method is called:
+         Generator#GeneratorVisitor#generateJspFragment(Node, String)
+
+       2) It should navigate up the parents chain. The SimpleTag can be one
+         of our parents, not necessary the immediate one.
+
+    3. I think that BZ 48616 cannot/should not be fixed - see Comment 15 to
+      the issue.
+
+    4. Should we fix BZ 42390 in TC 5.5, and bring on BZ 48616 there, if it
+    was historically working? I have doubts.
+
+    Reviewing this as a whole, I have questions regarding the following field:
+      ScriptingVariabler#ScriptingVariableVisitor#scriptVars
+    It is used to introduce behaviour like requested in BZ 48616, but ...
+    The BZ 42390 fix (r804734) effectively eliminates it. It looks like
+    it'd be better to remove it and care about redeclarations somewhere
+    else.
+    - to discuss on dev@
 
   Additional patches (trivial):
   http://svn.apache.org/viewvc?rev=905643&view=rev (misprint)



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

Reply via email to