You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2008/05/23 00:35:33 UTC

svn commit: r659289 - in /tapestry/tapestry5/trunk/tapestry-core/src/main: java/org/apache/tapestry5/corelib/components/ java/org/apache/tapestry5/corelib/internal/ java/org/apache/tapestry5/internal/services/ java/org/apache/tapestry5/services/ resour...

Author: hlship
Date: Thu May 22 15:35:30 2008
New Revision: 659289

URL: http://svn.apache.org/viewvc?rev=659289&view=rev
Log:
TAPESTRY-2379: FormInjector and FormFragment should use the (new) MarkupWriterListener interface to place the hidden field(s) in an appropriate location

Added:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/ComponentActionSink.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/HiddenFieldPositioner.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/HiddenFieldLocationRulesImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/HiddenFieldLocationRules.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/RelativeElementPosition.java
Modified:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormFragment.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormInjector.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/FormSupportImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/MarkupWriterImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tapestry.js

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/Form.java Thu May 22 15:35:30 2008
@@ -19,6 +19,7 @@
 import org.apache.tapestry5.annotations.Mixin;
 import org.apache.tapestry5.annotations.Parameter;
 import org.apache.tapestry5.annotations.Persist;
+import org.apache.tapestry5.corelib.internal.ComponentActionSink;
 import org.apache.tapestry5.corelib.internal.FormSupportImpl;
 import org.apache.tapestry5.corelib.mixins.RenderInformals;
 import org.apache.tapestry5.dom.Element;
@@ -27,7 +28,6 @@
 import org.apache.tapestry5.internal.services.ComponentResultProcessorWrapper;
 import org.apache.tapestry5.internal.services.HeartbeatImpl;
 import org.apache.tapestry5.internal.util.Base64ObjectInputStream;
-import org.apache.tapestry5.internal.util.Base64ObjectOutputStream;
 import org.apache.tapestry5.ioc.Location;
 import org.apache.tapestry5.ioc.annotations.Inject;
 import org.apache.tapestry5.ioc.internal.util.InternalUtils;
@@ -166,7 +166,7 @@
     // Collects a stream of component actions. Each action goes in as a UTF string (the component
     // component id), followed by a ComponentAction
 
-    private Base64ObjectOutputStream actions;
+    private ComponentActionSink actionSink;
 
     @SuppressWarnings("unused")
     @Mixin
@@ -198,18 +198,11 @@
     void beginRender(MarkupWriter writer)
     {
 
-        try
-        {
-            actions = new Base64ObjectOutputStream();
-        }
-        catch (IOException ex)
-        {
-            throw new RuntimeException(ex);
-        }
+        actionSink = new ComponentActionSink();
 
         name = renderSupport.allocateClientId(resources);
 
-        formSupport = new FormSupportImpl(name, actions, clientBehaviorSupport, clientValidation);
+        formSupport = new FormSupportImpl(name, actionSink, clientBehaviorSupport, clientValidation);
 
         if (zone != null) clientBehaviorSupport.linkZone(name, zone);
 
@@ -267,24 +260,13 @@
 
         writer.end(); // form
 
-        // Now, inject into the div the remaining hidden field (the list of actions).
-
-        try
-        {
-            actions.close();
-        }
-        catch (IOException ex)
-        {
-            throw new RuntimeException(ex);
-        }
-
         div.element("input",
 
                     "type", "hidden",
 
                     "name", FORM_DATA,
 
-                    "value", actions.toBase64());
+                    "value", actionSink.toBase64());
     }
 
     void cleanupRender()

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormFragment.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormFragment.java?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormFragment.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormFragment.java Thu May 22 15:35:30 2008
@@ -18,18 +18,14 @@
 import org.apache.tapestry5.annotations.Environmental;
 import org.apache.tapestry5.annotations.Parameter;
 import org.apache.tapestry5.annotations.SupportsInformalParameters;
+import org.apache.tapestry5.corelib.internal.ComponentActionSink;
 import org.apache.tapestry5.corelib.internal.FormSupportAdapter;
+import org.apache.tapestry5.corelib.internal.HiddenFieldPositioner;
 import org.apache.tapestry5.corelib.internal.WrappedComponentAction;
 import org.apache.tapestry5.dom.Element;
 import org.apache.tapestry5.internal.services.ClientBehaviorSupport;
 import org.apache.tapestry5.ioc.annotations.Inject;
-import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
-import org.apache.tapestry5.ioc.internal.util.Defense;
-import org.apache.tapestry5.runtime.Component;
-import org.apache.tapestry5.services.ComponentSource;
-import org.apache.tapestry5.services.Environment;
-import org.apache.tapestry5.services.FormSupport;
-import org.apache.tapestry5.services.Request;
+import org.apache.tapestry5.services.*;
 
 import java.util.List;
 
@@ -38,6 +34,11 @@
  * automatically bypass validation when the fragment is invisible.  The trick is to also bypass server-side form
  * processing for such fields when the form is submitted; the fragment uses a hidden field to track its client-side
  * visibility and will bypass field component submission logic for the components it encloses.
+ * <p/>
+ * <p/>
+ * In addition, should the client-side element for a Form fragment be removed before the enclosing form is submitted,
+ * then none of the fields inside the fragment will be processed (this can be considered an extension of the "if not
+ * visible, don't process" option above).
  *
  * @see org.apache.tapestry5.corelib.mixins.TriggerFragment
  */
@@ -73,7 +74,6 @@
     @Environmental
     private RenderSupport renderSupport;
 
-
     @Inject
     private ComponentSource componentSource;
 
@@ -85,31 +85,15 @@
 
     private String clientId;
 
-    private String controlName;
-
-    private List<WrappedComponentAction> componentActions;
+    private ComponentActionSink componentActions;
 
     @Inject
     private Request request;
 
-    static class HandleSubmission implements ComponentAction<FormFragment>
-    {
-        private final String controlName;
-
-        private final List<WrappedComponentAction> actions;
-
-        public HandleSubmission(String controlName, List<WrappedComponentAction> actions)
-        {
-            this.controlName = controlName;
-            this.actions = actions;
-        }
-
-        public void execute(FormFragment component)
-        {
-            component.handleSubmission(controlName, actions);
-        }
-    }
+    @Inject
+    private HiddenFieldLocationRules rules;
 
+    private HiddenFieldPositioner hiddenFieldPositioner;
 
     private void handleSubmission(String elementName, List<WrappedComponentAction> actions)
     {
@@ -137,9 +121,10 @@
 
         String id = resources.getId();
 
-        controlName = formSupport.allocateControlName(id);
         clientId = renderSupport.allocateClientId(id);
 
+        hiddenFieldPositioner = new HiddenFieldPositioner(writer, rules);
+
         Element element = writer.element("div", "id", clientId);
 
         resources.renderInformalParameters(writer);
@@ -148,21 +133,9 @@
             element.addClassName(CSSClassConstants.INVISIBLE);
 
 
-        writer.element("input",
-
-                       "type", "hidden",
-
-                       "name", controlName,
-
-                       "id", clientId + ":hidden",
-
-                       "value", String.valueOf(visible));
-        writer.end();
-
-
         clientBehaviorSupport.addFormFragment(clientId, show, hide);
 
-        componentActions = CollectionFactory.newList();
+        componentActions = new ComponentActionSink();
 
         // Here's the magic of environmentals ... we can create a wrapper around
         // the normal FormSupport environmental that intercepts some of the behavior.
@@ -174,15 +147,13 @@
             @Override
             public <T> void store(T component, ComponentAction<T> action)
             {
-                Component asComponent = Defense.cast(component, Component.class, "component");
-
-                componentActions.add(new WrappedComponentAction(asComponent, action));
+                componentActions.store(component, action);
             }
 
             @Override
             public <T> void storeAndExecute(T component, ComponentAction<T> action)
             {
-                store(component, action);
+                componentActions.store(component, action);
 
                 action.execute(component);
             }
@@ -203,11 +174,20 @@
      */
     void afterRender(MarkupWriter writer)
     {
+
+        hiddenFieldPositioner.getElement().attributes(
+                "type", "hidden",
+
+                "name", Form.FORM_DATA,
+
+                "id", clientId + ":hidden",
+
+                "value", componentActions.toBase64()
+        );
+
         writer.end(); // div
 
         environment.pop(FormSupport.class);
-
-        environment.peek(FormSupport.class).store(this, new HandleSubmission(controlName, componentActions));
     }
 
     public String getClientId()

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormInjector.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormInjector.java?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormInjector.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/components/FormInjector.java Thu May 22 15:35:30 2008
@@ -19,11 +19,12 @@
 import org.apache.tapestry5.annotations.Parameter;
 import org.apache.tapestry5.annotations.SupportsInformalParameters;
 import org.apache.tapestry5.corelib.data.InsertPosition;
+import org.apache.tapestry5.corelib.internal.ComponentActionSink;
 import org.apache.tapestry5.corelib.internal.FormSupportImpl;
+import org.apache.tapestry5.corelib.internal.HiddenFieldPositioner;
 import org.apache.tapestry5.internal.services.ClientBehaviorSupport;
 import org.apache.tapestry5.internal.services.ComponentResultProcessorWrapper;
 import org.apache.tapestry5.internal.services.PageRenderQueue;
-import org.apache.tapestry5.internal.util.Base64ObjectOutputStream;
 import org.apache.tapestry5.ioc.annotations.Inject;
 import org.apache.tapestry5.ioc.internal.util.IdAllocator;
 import org.apache.tapestry5.runtime.RenderCommand;
@@ -139,6 +140,15 @@
     }
 
     /**
+     * Used during Ajax partial rendering to identify the hidden field that will hold the form data (component actions,
+     * used when processing the form submission) for the injection.
+     */
+    private HiddenFieldPositioner hiddenFieldPositioner;
+
+    @Inject
+    private HiddenFieldLocationRules rules;
+
+    /**
      * Invoked via an Ajax request.  Triggers an action event and captures the return value. The return value from the
      * event notification is what will ultimately render (typically, its a Block).  However, we do a <em>lot</em> of
      * tricks to provide the desired FormSupport around the what renders.
@@ -158,41 +168,33 @@
 
         final String formId = request.getParameter(FORMID_PARAMETER);
 
-        final Base64ObjectOutputStream actions = new Base64ObjectOutputStream();
+        final ComponentActionSink actionSink = new ComponentActionSink();
 
         final RenderCommand cleanup = new RenderCommand()
         {
             public void render(MarkupWriter writer, RenderQueue queue)
             {
-                try
-                {
-                    actions.close();
-                }
-                catch (IOException ex)
-                {
-                    throw new RuntimeException(ex);
-                }
-
                 environment.pop(ValidationTracker.class);
 
                 FormSupportImpl formSupport = (FormSupportImpl) environment.pop(FormSupport.class);
 
                 formSupport.executeDeferred();
 
-                writer.element("input",
+                hiddenFieldPositioner.getElement().attributes(
+                        "type", "hidden",
 
-                               "type", "hidden",
+                        "name", Form.FORM_DATA,
 
-                               "name", Form.FORM_DATA,
-
-                               "value", actions.toBase64());
+                        "value", actionSink.toBase64());
             }
         };
 
         final RenderCommand setup = new RenderCommand()
         {
-            public void render(MarkupWriter writer, RenderQueue queue)
+            public void render(final MarkupWriter writer, RenderQueue queue)
             {
+                hiddenFieldPositioner = new HiddenFieldPositioner(writer, rules);
+
                 // Kind of ugly, but the only way to ensure we don't have name collisions on the
                 // client side is to force a unique id into each name (as well as each id, but that's
                 // RenderSupport's job).  It would be nice if we could agree on the uid, but
@@ -202,11 +204,10 @@
 
                 IdAllocator idAllocator = new IdAllocator(":" + uid);
 
-                FormSupportImpl formSupport = new FormSupportImpl(formId, actions, clientBehaviorSupport, true,
+                FormSupportImpl formSupport = new FormSupportImpl(formId, actionSink, clientBehaviorSupport, true,
                                                                   idAllocator);
-                environment.push(FormSupport.class, formSupport);
-
 
+                environment.push(FormSupport.class, formSupport);
                 environment.push(ValidationTracker.class, new ValidationTrackerImpl());
 
                 // Queue up the root render command to execute first, and the cleanup

Added: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/ComponentActionSink.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/ComponentActionSink.java?rev=659289&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/ComponentActionSink.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/ComponentActionSink.java Thu May 22 15:35:30 2008
@@ -0,0 +1,78 @@
+// Copyright 2008 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.corelib.internal;
+
+import org.apache.tapestry5.ComponentAction;
+import org.apache.tapestry5.internal.util.Base64ObjectOutputStream;
+import org.apache.tapestry5.ioc.internal.util.Defense;
+import org.apache.tapestry5.runtime.Component;
+
+import java.io.IOException;
+
+/**
+ * Used to collection component actions, with the ultimate goal being the creation of a MIME-encoded string of the
+ * serialization of those actions.
+ */
+public class ComponentActionSink
+{
+    private final Base64ObjectOutputStream stream;
+
+    public ComponentActionSink()
+    {
+        try
+        {
+            stream = new Base64ObjectOutputStream();
+        }
+        catch (IOException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+    }
+
+    public <T> void store(T component, ComponentAction<T> action)
+    {
+        Component castComponent = Defense.cast(component, Component.class, "component");
+        Defense.notNull(action, "action");
+
+        String completeId = castComponent.getComponentResources().getCompleteId();
+
+        try
+        {
+            // Writing the complete id is not very efficient, but the GZip filter
+            // should help out there.
+            stream.writeUTF(completeId);
+            stream.writeObject(action);
+        }
+        catch (IOException ex)
+        {
+            throw new RuntimeException(InternalMessages.componentActionNotSerializable(completeId, ex), ex);
+        }
+    }
+
+
+    public String toBase64()
+    {
+        try
+        {
+            stream.close();
+        }
+        catch (IOException ex)
+        {
+            throw new RuntimeException(ex);
+        }
+
+        return stream.toBase64();
+    }
+}

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/FormSupportImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/FormSupportImpl.java?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/FormSupportImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/FormSupportImpl.java Thu May 22 15:35:30 2008
@@ -20,11 +20,8 @@
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
 import org.apache.tapestry5.ioc.internal.util.Defense;
 import org.apache.tapestry5.ioc.internal.util.IdAllocator;
-import org.apache.tapestry5.runtime.Component;
 import org.apache.tapestry5.services.FormSupport;
 
-import java.io.IOException;
-import java.io.ObjectOutputStream;
 import java.util.List;
 
 /**
@@ -43,7 +40,7 @@
 
     private final String clientId;
 
-    private final ObjectOutputStream actions;
+    private final ComponentActionSink actionSink;
 
     private List<Runnable> commands;
 
@@ -60,20 +57,20 @@
     /**
      * Constructor used when rendering.
      */
-    public FormSupportImpl(String clientId, ObjectOutputStream actions, ClientBehaviorSupport clientBehaviorSupport,
+    public FormSupportImpl(String clientId, ComponentActionSink actionSink, ClientBehaviorSupport clientBehaviorSupport,
                            boolean clientValidationEnabled)
     {
-        this(clientId, actions, clientBehaviorSupport, clientValidationEnabled, new IdAllocator());
+        this(clientId, actionSink, clientBehaviorSupport, clientValidationEnabled, new IdAllocator());
     }
 
     /**
      * Full constructor.
      */
-    public FormSupportImpl(String clientId, ObjectOutputStream actions, ClientBehaviorSupport clientBehaviorSupport,
+    public FormSupportImpl(String clientId, ComponentActionSink actionSink, ClientBehaviorSupport clientBehaviorSupport,
                            boolean clientValidationEnabled, IdAllocator idAllocator)
     {
         this.clientId = clientId;
-        this.actions = actions;
+        this.actionSink = actionSink;
         this.clientBehaviorSupport = clientBehaviorSupport;
         this.clientValidationEnabled = clientValidationEnabled;
         this.idAllocator = idAllocator;
@@ -91,27 +88,12 @@
 
     public <T> void store(T component, ComponentAction<T> action)
     {
-        Component castComponent = Defense.cast(component, Component.class, "component");
-        Defense.notNull(action, "action");
-
-        String completeId = castComponent.getComponentResources().getCompleteId();
-
-        try
-        {
-            // Writing the complete id is not very efficient, but the GZip filter
-            // should help out there.
-            actions.writeUTF(completeId);
-            actions.writeObject(action);
-        }
-        catch (IOException ex)
-        {
-            throw new RuntimeException(InternalMessages.componentActionNotSerializable(completeId, ex), ex);
-        }
+        actionSink.store(component, action);
     }
 
     public <T> void storeAndExecute(T component, ComponentAction<T> action)
     {
-        store(component, action);
+        actionSink.store(component, action);
 
         action.execute(component);
     }

Added: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/HiddenFieldPositioner.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/HiddenFieldPositioner.java?rev=659289&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/HiddenFieldPositioner.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/corelib/internal/HiddenFieldPositioner.java Thu May 22 15:35:30 2008
@@ -0,0 +1,81 @@
+package org.apache.tapestry5.corelib.internal;
+
+import org.apache.tapestry5.MarkupWriter;
+import org.apache.tapestry5.MarkupWriterListener;
+import org.apache.tapestry5.dom.Element;
+import org.apache.tapestry5.ioc.internal.util.OneShotLock;
+import org.apache.tapestry5.services.HiddenFieldLocationRules;
+
+/**
+ * Used to position a hidden field (as part of a form-related component).  Hidden fields are not allowed to go just
+ * anywhere, there are rules, dicatated by the (X)HTML schema, about where they are allowed. We use the {@link
+ * org.apache.tapestry5.MarkupWriterListener} interface to monitor elements as they are started and ended to find a
+ * place to put content.
+ */
+public class HiddenFieldPositioner
+{
+    /**
+     * The type of element to create.
+     */
+    private static final String ELEMENT = "input";
+
+    private final MarkupWriter writer;
+
+    private final HiddenFieldLocationRules rules;
+
+    private final OneShotLock lock = new OneShotLock();
+
+    private Element hiddenFieldElement;
+
+    private final MarkupWriterListener listener = new MarkupWriterListener()
+    {
+        public void elementDidStart(Element element)
+        {
+            if (rules.placeHiddenFieldInside(element))
+            {
+                hiddenFieldElement = element.element(ELEMENT);
+                writer.removeListener(this);
+            }
+        }
+
+        public void elementDidEnd(Element element)
+        {
+            if (rules.placeHiddenFieldAfter(element))
+            {
+                hiddenFieldElement = element.getParent().element(ELEMENT);
+                writer.removeListener(this);
+            }
+        }
+    };
+
+    public HiddenFieldPositioner(MarkupWriter writer, HiddenFieldLocationRules rules)
+    {
+        this.writer = writer;
+        this.rules = rules;
+
+        this.writer.addListener(listener);
+    }
+
+    /**
+     * Returns the hidden field element, which can have its attributes filled in.
+     *
+     * @return the element
+     * @throws IllegalStateException if the element was not placed.
+     */
+    public Element getElement()
+    {
+        lock.lock();
+
+        // Remove the listener if it hasn't been removed already.
+
+        writer.removeListener(listener);
+
+        if (hiddenFieldElement == null)
+            throw new IllegalStateException(
+                    "The rendered content did not include any elements that allow for the positioning of the hidden form field's element.");
+
+
+        return hiddenFieldElement;
+    }
+
+}

Added: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/HiddenFieldLocationRulesImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/HiddenFieldLocationRulesImpl.java?rev=659289&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/HiddenFieldLocationRulesImpl.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/HiddenFieldLocationRulesImpl.java Thu May 22 15:35:30 2008
@@ -0,0 +1,41 @@
+package org.apache.tapestry5.internal.services;
+
+import org.apache.tapestry5.dom.Element;
+import org.apache.tapestry5.ioc.internal.util.Defense;
+import org.apache.tapestry5.services.HiddenFieldLocationRules;
+import org.apache.tapestry5.services.RelativeElementPosition;
+
+import java.util.Map;
+
+public class HiddenFieldLocationRulesImpl implements HiddenFieldLocationRules
+{
+    private final Map<String, RelativeElementPosition> configuration;
+
+    public HiddenFieldLocationRulesImpl(Map<String, RelativeElementPosition> configuration)
+    {
+        this.configuration = configuration;
+    }
+
+    private boolean match(Element element, RelativeElementPosition position)
+    {
+        Defense.notNull(element, "element");
+
+        String key = element.getName();
+
+        RelativeElementPosition actual = configuration.get(key);
+
+        if (actual == null) return false;
+
+        return actual == position;
+    }
+
+    public boolean placeHiddenFieldInside(Element element)
+    {
+        return match(element, RelativeElementPosition.INSIDE);
+    }
+
+    public boolean placeHiddenFieldAfter(Element element)
+    {
+        return match(element, RelativeElementPosition.AFTER);
+    }
+}

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/MarkupWriterImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/MarkupWriterImpl.java?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/MarkupWriterImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/MarkupWriterImpl.java Thu May 22 15:35:30 2008
@@ -22,6 +22,7 @@
 import org.apache.tapestry5.ioc.internal.util.InternalUtils;
 
 import java.io.PrintWriter;
+import java.util.Collection;
 import java.util.List;
 
 public class MarkupWriterImpl implements MarkupWriter
@@ -32,7 +33,7 @@
 
     private Text currentText;
 
-    private final List<MarkupWriterListener> listeners = CollectionFactory.newList();
+    private List<MarkupWriterListener> listeners;
 
     public MarkupWriterImpl()
     {
@@ -200,7 +201,6 @@
 
         currentText = null;
 
-
         fireElementDidStart();
 
         return current;
@@ -210,25 +210,37 @@
     {
         Defense.notNull(listener, "listener");
 
+        if (listeners == null) listeners = CollectionFactory.newList();
+
         listeners.add(listener);
     }
 
     public void removeListener(MarkupWriterListener listener)
     {
-        listeners.remove(listener);
+        if (listeners != null)
+            listeners.remove(listener);
     }
 
     private void fireElementDidStart()
     {
-        for (MarkupWriterListener l : listeners)
+        if (isEmpty(listeners)) return;
+
+        for (MarkupWriterListener l : CollectionFactory.newList(listeners))
         {
             l.elementDidStart(current);
         }
     }
 
+    private static boolean isEmpty(Collection<?> collection)
+    {
+        return collection == null || collection.isEmpty();
+    }
+
     private void fireElementDidEnd()
     {
-        for (MarkupWriterListener l : listeners)
+        if (isEmpty(listeners)) return;
+
+        for (MarkupWriterListener l : CollectionFactory.newList(listeners))
         {
             l.elementDidEnd(current);
         }

Added: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/HiddenFieldLocationRules.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/HiddenFieldLocationRules.java?rev=659289&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/HiddenFieldLocationRules.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/HiddenFieldLocationRules.java Thu May 22 15:35:30 2008
@@ -0,0 +1,23 @@
+package org.apache.tapestry5.services;
+
+import org.apache.tapestry5.dom.Element;
+
+/**
+ * Provides some assistance in determining <em>where</em> to place a hidden field based on standard (X)HTML elements.
+ * <p/>
+ * <p/>
+ * The service works based on a mapped service contribution; keys are the element names, values area {@link
+ * org.apache.tapestry5.services.RelativeElementPosition}.
+ */
+public interface HiddenFieldLocationRules
+{
+    /**
+     * Checks the element to see if a hidden field may be placed inside the element.
+     */
+    boolean placeHiddenFieldInside(Element element);
+
+    /**
+     * Checks the element to see if a hidden field may be placed after the element (as a sibling element).
+     */
+    boolean placeHiddenFieldAfter(Element element);
+}

Added: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/RelativeElementPosition.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/RelativeElementPosition.java?rev=659289&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/RelativeElementPosition.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/RelativeElementPosition.java Thu May 22 15:35:30 2008
@@ -0,0 +1,18 @@
+package org.apache.tapestry5.services;
+
+/**
+ * Used by {@link org.apache.tapestry5.services.HiddenFieldLocationRules} to identify where a hidden field may be placed
+ * relative to a particular element.
+ */
+public enum RelativeElementPosition
+{
+    /**
+     * The hidden field may be placed inside the element, as a child.
+     */
+    INSIDE,
+
+    /**
+     * The hidden field may be placed after the element, as a sibling.
+     */
+    AFTER;
+}

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java Thu May 22 15:35:30 2008
@@ -40,13 +40,12 @@
 import org.apache.tapestry5.ioc.services.*;
 import org.apache.tapestry5.ioc.util.StrategyRegistry;
 import org.apache.tapestry5.ioc.util.TimeInterval;
+import org.apache.tapestry5.json.JSONObject;
 import org.apache.tapestry5.runtime.Component;
 import org.apache.tapestry5.runtime.ComponentResourcesAware;
 import org.apache.tapestry5.runtime.RenderCommand;
 import org.apache.tapestry5.util.StringToEnumCoercion;
 import org.apache.tapestry5.validator.*;
-import org.apache.tapestry5.json.JSONObject;
-import org.apache.tapestry5.services.*;
 import org.slf4j.Logger;
 
 import javax.servlet.ServletContext;
@@ -153,8 +152,6 @@
 
     public static void bind(ServiceBinder binder)
     {
-        // Public Services
-
         binder.bind(ClasspathAssetAliasManager.class, ClasspathAssetAliasManagerImpl.class);
         binder.bind(PersistentLocale.class, PersistentLocaleImpl.class);
         binder.bind(ApplicationStateManager.class, ApplicationStateManagerImpl.class);
@@ -187,8 +184,8 @@
         binder.bind(ContextValueEncoder.class, ContextValueEncoderImpl.class);
         binder.bind(BaseURLSource.class, BaseURLSourceImpl.class);
         binder.bind(BeanBlockOverrideSource.class, BeanBlockOverrideSourceImpl.class);
-
         binder.bind(AliasManager.class, AliasManagerImpl.class).withId("AliasOverrides");
+        binder.bind(HiddenFieldLocationRules.class, HiddenFieldLocationRulesImpl.class);
     }
 
     // ========================================================================
@@ -277,11 +274,11 @@
      * their values between requests</dd> <dt>Persist </dt> <dd>Allows fields to store their their value persistently
      * between requests</dd> <dt>Parameter </dt> <dd>Identifies parameters based on the {@link
      * org.apache.tapestry5.annotations.Parameter} annotation</dd> <dt>Component </dt> <dd>Defines embedded components
-     * based on the {@link org.apache.tapestry5.annotations.Component} annotation</dd> <dt>Mixin </dt> <dd>Adds a mixin as
-     * part of a component's implementation</dd> <dt>Environment </dt> <dd>Allows fields to contain values extracted
+     * based on the {@link org.apache.tapestry5.annotations.Component} annotation</dd> <dt>Mixin </dt> <dd>Adds a mixin
+     * as part of a component's implementation</dd> <dt>Environment </dt> <dd>Allows fields to contain values extracted
      * from the {@link org.apache.tapestry5.services.Environment} service</dd> <dt>Inject </dt> <dd>Used with the {@link
-     * org.apache.tapestry5.ioc.annotations.Inject} annotation, when a value is supplied</dd> <dt>InjectPage</dt> <dd>Adds
-     * code to allow access to other pages via the {@link org.apache.tapestry5.annotations.InjectPage} field
+     * org.apache.tapestry5.ioc.annotations.Inject} annotation, when a value is supplied</dd> <dt>InjectPage</dt>
+     * <dd>Adds code to allow access to other pages via the {@link org.apache.tapestry5.annotations.InjectPage} field
      * annotation</dd> <dt>InjectBlock </dt> <dd>Allows a block from the template to be injected into a field</dd>
      * <dt>IncludeStylesheet </dt> <dd>Supports the {@link org.apache.tapestry5.annotations.IncludeStylesheet}
      * annotation</dd> <dt>IncludeJavaScriptLibrary </dt> <dd>Supports the {@link org.apache.tapestry5.annotations.IncludeJavaScriptLibrary}
@@ -289,9 +286,9 @@
      * for meta data and adds it to the component model</dd> <dt>ApplicationState </dt> <dd>Converts fields that
      * reference application state objects <dt>UnclaimedField </dt> <dd>Identifies unclaimed fields and resets them to
      * null/0/false at the end of the request</dd> <dt>RenderCommand </dt> <dd>Ensures all components also implement
-     * {@link org.apache.tapestry5.runtime.RenderCommand}</dd> <dt>SetupRender, BeginRender, etc. </dt> <dd>Correspond to
-     * component render phases and annotations</dd> <dt>InvokePostRenderCleanupOnResources </dt> <dd>Makes sure {@link
-     * org.apache.tapestry5.internal.InternalComponentResources#postRenderCleanup()} is invoked after a component
+     * {@link org.apache.tapestry5.runtime.RenderCommand}</dd> <dt>SetupRender, BeginRender, etc. </dt> <dd>Correspond
+     * to component render phases and annotations</dd> <dt>InvokePostRenderCleanupOnResources </dt> <dd>Makes sure
+     * {@link org.apache.tapestry5.internal.InternalComponentResources#postRenderCleanup()} is invoked after a component
      * finishes rendering</dd> <dt>Secure</dt> <dd>Checks for the {@link org.apache.tapestry5.annotations.Secure}
      * annotation</dd> <dt>ContentType</dt> <dd>Checks for {@link org.apache.tapestry5.annotations.ContentType}
      * annotation</dd> <dt>ResponseEncoding</dt> <dd>Checks for the {@link org.apache.tapestry5.annotations.ResponseEncoding}
@@ -621,9 +618,10 @@
      * Adds coercions: <ul> <li>String to {@link org.apache.tapestry5.SelectModel} <li>String to {@link
      * org.apache.tapestry5.corelib.data.InsertPosition} <li>Map to {@link org.apache.tapestry5.SelectModel}
      * <li>Collection to {@link GridDataSource} <li>null to {@link org.apache.tapestry5.grid.GridDataSource} <li>String
-     * to {@link org.apache.tapestry5.corelib.data.GridPagerPosition} <li>List to {@link org.apache.tapestry5.SelectModel}
-     * <li>{@link org.apache.tapestry5.runtime.ComponentResourcesAware} (typically, a component) to {@link
-     * org.apache.tapestry5.ComponentResources} <li>String to {@link org.apache.tapestry5.corelib.data.BlankOption} </ul>
+     * to {@link org.apache.tapestry5.corelib.data.GridPagerPosition} <li>List to {@link
+     * org.apache.tapestry5.SelectModel} <li>{@link org.apache.tapestry5.runtime.ComponentResourcesAware} (typically, a
+     * component) to {@link org.apache.tapestry5.ComponentResources} <li>String to {@link
+     * org.apache.tapestry5.corelib.data.BlankOption} </ul>
      */
     public static void contributeTypeCoercer(Configuration<CoercionTuple> configuration)
     {
@@ -1293,9 +1291,9 @@
      * The MasterDispatcher is a chain-of-command of individual Dispatchers, each handling (like a servlet) a particular
      * kind of incoming request. <dl> <dt>RootPath</dt> <dd>Renders the start page for the "/" request</dd>
      * <dt>Asset</dt> <dd>Provides access to classpath assets</dd> <dt>PageRender</dt> <dd>Identifies the {@link
-     * org.apache.tapestry5.services.PageRenderRequestParameters} and forwards onto {@link PageRenderRequestHandler}</dd>
-     * <dt>ComponentEvent</dt> <dd>Identifies the {@link ComponentEventRequestParameters} and forwards onto the {@link
-     * ComponentEventRequestHandler}</dd> </dl>
+     * org.apache.tapestry5.services.PageRenderRequestParameters} and forwards onto {@link
+     * PageRenderRequestHandler}</dd> <dt>ComponentEvent</dt> <dd>Identifies the {@link ComponentEventRequestParameters}
+     * and forwards onto the {@link ComponentEventRequestHandler}</dd> </dl>
      */
     public void contributeMasterDispatcher(OrderedConfiguration<Dispatcher> configuration,
                                            ObjectLocator locator)
@@ -1367,8 +1365,8 @@
 
 
     /**
-     * Adds page render filters, each of which provides an {@link org.apache.tapestry5.annotations.Environmental} service.
-     * Filters often provide {@link Environmental} services needed by components as they render. <dl>
+     * Adds page render filters, each of which provides an {@link org.apache.tapestry5.annotations.Environmental}
+     * service. Filters often provide {@link Environmental} services needed by components as they render. <dl>
      * <dt>PageRenderSupport</dt>  <dd>Provides {@link org.apache.tapestry5.RenderSupport}</dd>
      * <dt>ClientBehaviorSupport</dt> <dd>Provides {@link org.apache.tapestry5.internal.services.ClientBehaviorSupport}</dd>
      * <dt>Heartbeat</dt> <dd>Provides {@link org.apache.tapestry5.services.Heartbeat}</dd>
@@ -1488,8 +1486,8 @@
     /**
      * Contributes {@link PartialMarkupRendererFilter}s used when rendering a partial Ajax response.  This is an analog
      * to {@link #contributeMarkupRenderer(org.apache.tapestry5.ioc.OrderedConfiguration, org.apache.tapestry5.Asset,
-     * org.apache.tapestry5.Asset, ValidationMessagesSource, org.apache.tapestry5.ioc.services.SymbolSource, AssetSource)}
-     * } and overlaps it to some degree. <dl> <dt>   PageRenderSupport     </dt> <dd>Provides {@link
+     * org.apache.tapestry5.Asset, ValidationMessagesSource, org.apache.tapestry5.ioc.services.SymbolSource,
+     * AssetSource)} } and overlaps it to some degree. <dl> <dt>   PageRenderSupport     </dt> <dd>Provides {@link
      * org.apache.tapestry5.RenderSupport}</dd> <dt>ClientBehaviorSupport</dt> <dd>Provides {@link
      * org.apache.tapestry5.internal.services.ClientBehaviorSupport}</dd> <dt>Heartbeat</dt> <dd>Provides {@link
      * org.apache.tapestry5.services.Heartbeat}</dd> <dt>DefaultValidationDecorator</dt> <dd>Provides {@link
@@ -2010,4 +2008,26 @@
         configuration.add("zero", new ZeroNullFieldStrategy());
     }
 
+
+    /**
+     * Determines positioning of hidden fields relative to other elements (this is needed by {@link
+     * org.apache.tapestry5.corelib.components.FormFragment} and others.
+     * <p/>
+     * For elements input, select, textarea and label the hidden field is positioned after.
+     * <p/>
+     * For elements p, div, li and td, the hidden field is positioned inside.
+     */
+    public static void contributeHiddenFieldLocationRules(
+            MappedConfiguration<String, RelativeElementPosition> configuration)
+    {
+        configuration.add("input", RelativeElementPosition.AFTER);
+        configuration.add("select", RelativeElementPosition.AFTER);
+        configuration.add("textarea", RelativeElementPosition.AFTER);
+        configuration.add("label", RelativeElementPosition.AFTER);
+
+        configuration.add("p", RelativeElementPosition.INSIDE);
+        configuration.add("div", RelativeElementPosition.INSIDE);
+        configuration.add("td", RelativeElementPosition.INSIDE);
+        configuration.add("li", RelativeElementPosition.INSIDE);
+    }
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tapestry.js
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tapestry.js?rev=659289&r1=659288&r2=659289&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tapestry.js (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/tapestry.js Thu May 22 15:35:30 2008
@@ -886,7 +886,11 @@
 
         $(this.hidden.form).observe(Tapestry.FORM_PREPARE_FOR_SUBMIT_EVENT, function()
         {
-            this.hidden.value = Tapestry.isDeepVisible(this.element);
+            // On a submission, if the fragment is not visible, then wipe out its
+            // form submission data, so that no processing or validation occurs on the server.
+
+            if (! Tapestry.isDeepVisible(this.element))
+                this.hidden.value = "";
         }.bind(this));
     },