Author: robertdzeigler Date: Wed Aug 31 13:42:51 2011 New Revision: 1163619
URL: http://svn.apache.org/viewvc?rev=1163619&view=rev Log: TAP5-1620: Tml parsing expression error TAP5-1448: Example for org.apache.tapestry5.corelib.components.Errors uses invalid xml Added: tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ExpressionInJsFunction.tml tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ExpressionInJsFunction.java Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Errors.xdoc tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Errors.xdoc URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Errors.xdoc?rev=1163619&r1=1163618&r2=1163619&view=diff ============================================================================== --- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Errors.xdoc (original) +++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Errors.xdoc Wed Aug 31 13:42:51 2011 @@ -15,7 +15,7 @@ <t:form> - <t:errors> + <t:errors/> <t:label for="search"/> <t:textfield t:id="search"/> @@ -36,4 +36,4 @@ </section> </body> -</document> \ No newline at end of file +</document> Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java?rev=1163619&r1=1163618&r2=1163619&view=diff ============================================================================== --- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java (original) +++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/SaxTemplateParser.java Wed Aug 31 13:42:51 2011 @@ -130,6 +130,10 @@ public class SaxTemplateParser // but invalid expansion. private static final Pattern EXPANSION_PATTERN = Pattern.compile("\\$\\{\\s*(((?!\\$\\{).)*)\\s*}"); + private static final char EXPANSION_STRING_DELIMITTER='\''; + private static final char OPEN_BRACE='{'; + private static final char CLOSE_BRACE='}'; + //private static final Pattern EXPANSION_PATTERN = Pattern.compile("\\$\\{\\"); private static final Set<String> MUST_BE_ROOT = CollectionFactory.newSet("extend", "container"); @@ -1090,7 +1094,6 @@ public class SaxTemplateParser // TAPESTRY-2028 means that the whitespace has likely been stripped out // of the text // already anyway. - while (matcher.find()) { int matchStart = matcher.start(); @@ -1098,19 +1101,66 @@ public class SaxTemplateParser if (matchStart != startx) { String prefix = text.substring(startx, matchStart); - tokenAccumulator.add(new TextToken(prefix, textStartLocation)); } // Group 1 includes the real text of the expansion, with whitespace // around the // expression (but inside the curly braces) excluded. - + // But note that we run into a problem. The original + // EXPANSION_PATTERN used a reluctant quantifier to match the + // smallest instance of ${} possible. But if you have ${'}'} or + // ${{'key': 'value'}} (maps, cf TAP5-1605) then you run into issues + // b/c the expansion becomes {'key': 'value' which is wrong. + // A fix to use greedy matching with negative lookahead to prevent + // ${...}...${...} all matching a single expansion is close, but + // has issues when an expansion is used inside a javascript function + // (see TAP5-1620). The solution is to use the greedy + // EXPANSION_PATTERN as before to bound the search for a single + // expansion, then check for {} consistency, ignoring opening and + // closing braces that occur within '' (the property expression + // language doesn't support "" for strings). That should include: + // 'This string has a } in it' and 'This string has a { in it.' + // Note also that the property expression language doesn't support + // escaping the string character ('), so we don't have to worry + // about that. String expression = matcher.group(1); + //count of 'open' braces. Expression ends when it hits 0. In most cases, + // it should end up as 1 b/c "expression" is everything inside ${}, so + // the following will typically not find the end of the expression. + int openBraceCount=1; + int expressionEnd = expression.length(); + boolean inQuote=false; + for(int i=0;i<expression.length();i++) { + char c = expression.charAt(i); + //basically, if we're inQuote, we ignore everything until we hit the quote end, so we only care if the character matches the quote start (meaning we're at the end of the quote). + //note that I don't believe expression support escaped quotes... + if (c==EXPANSION_STRING_DELIMITTER) { + inQuote = !inQuote; + continue; + } else if (inQuote) { + continue; + } else if (c == CLOSE_BRACE) { + openBraceCount--; + if (openBraceCount == 0) { + expressionEnd = i; + break; + } + } else if (c == OPEN_BRACE) { + openBraceCount++; + } + } + if (expressionEnd < expression.length()) { + //then we gobbled up some } that we shouldn't have... like the closing } of a javascript + //function. + tokenAccumulator.add(new ExpansionToken(expression.substring(0,expressionEnd), textStartLocation)); + //can't just assign to + startx=matcher.start(1) + expressionEnd + 1; + } else { + tokenAccumulator.add(new ExpansionToken(expression, textStartLocation)); - tokenAccumulator.add(new ExpansionToken(expression, textStartLocation)); - - startx = matcher.end(); + startx = matcher.end(); + } } // Catch anything after the final regexp match. Added: tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ExpressionInJsFunction.tml URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ExpressionInJsFunction.tml?rev=1163619&view=auto ============================================================================== --- tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ExpressionInJsFunction.tml (added) +++ tapestry/tapestry5/trunk/tapestry-core/src/test/app1/ExpressionInJsFunction.tml Wed Aug 31 13:42:51 2011 @@ -0,0 +1,22 @@ +<html t:type="border" xmlns:t="http://tapestry.apache.org/schema/tapestry_5_1_0.xsd" xmlns:p="tapestry:parameter"> + +<script> + function test_func() { + $('target').setValue(${'"test1"'}); + }; + + function test_func_with_map() { + $('target').setValue('${{'key':'test2'}}'); + }; + +</script> + + <h1>Expansions within a JS function</h1> + +<t:form> + <input id="target"/> + <input id="button1" type="button" onclick="test_func();" value="test1"/> + <input id="button2" type="button" onclick="test_func_with_map();" value="test2"/> +</t:form> + +</html> Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java?rev=1163619&r1=1163618&r2=1163619&view=diff ============================================================================== --- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java (original) +++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java Wed Aug 31 13:42:51 2011 @@ -136,6 +136,19 @@ public class CoreBehaviorsTests extends assertText("numberkeysmap", "{1:one}"); } + @Test + public void expressions_in_js_functions() throws Exception + { + openLinks("Expressions in JS Functions Demo"); + + click("button1"); + waitForCondition("selenium.getValue('target') == 'test1'", PAGE_LOAD_TIMEOUT); + + click("button2"); + waitForCondition("selenium.getValue('target') == '{key=test2}'", PAGE_LOAD_TIMEOUT); + + } + /** * {@link org.apache.tapestry5.internal.transform.InjectContainerWorker} is * largely tested by the forms tests Added: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ExpressionInJsFunction.java URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ExpressionInJsFunction.java?rev=1163619&view=auto ============================================================================== --- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ExpressionInJsFunction.java (added) +++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/ExpressionInJsFunction.java Wed Aug 31 13:42:51 2011 @@ -0,0 +1,24 @@ +// Copyright 2011 The Apache Software Foundation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package org.apache.tapestry5.integration.app1.pages; + +import java.util.Iterator; +import java.util.Map; + +/** + * Demonstrates the use of expressions within a javascript function. See TAP5-1620 + */ +public class ExpressionInJsFunction +{ +} Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java?rev=1163619&r1=1163618&r2=1163619&view=diff ============================================================================== --- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java (original) +++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java Wed Aug 31 13:42:51 2011 @@ -493,6 +493,8 @@ public class Index new Item("MapExpressionInExpansions", "Map Expressions in Expansions Demo", "Maps can be used in expansions"), + new Item("ExpressionInJsFunction", "Expressions in JS Functions Demo", "Expressions can be used inside javascript functions"), + new Item("FormFieldFocusDemo", "FormFieldFocus Demo", "Setting the Form focus on a specific field"), new Item("FormFragmentExplicitVisibleBoundsDemo", "Form Fragment Explicit Visible Bounds Demo", "Check for form fragment parent visibility can be bounded to")