You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by ro...@apache.org on 2011/08/31 15:42:52 UTC
svn commit: r1163619 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry5/corelib/components/
main/java/org/apache/tapestry5/internal/services/ test/app1/
test/java/org/apache/tapestry5/integration/app1/ test/java/org/apache...
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")