You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by Jesse Kuhnert <jk...@gmail.com> on 2008/01/10 16:54:10 UTC
[discuss T4] Multiple form support && generated client/form element ids
I've finally been bitten by this bug and would like to work on a fix
"now". Some background on the problem in question can be found in
https://issues.apache.org/jira/browse/TAPESTRY-1278 and
https://issues.apache.org/jira/browse/TAPESTRY-1274.
My only problem with the current MultipleFormSupport implementation is
that it would unnecessarily lengthen generated client id values and
almost never be able to honor the original component ID specified for
a component.
It would also require enough Selenium test changes on my current
project to make it almost unbearable to deal with.
I would like to propose that we:
-) Remove MultipleFormSupport (or mark it as deprecated if it's too
late) and handle this issue directly once in the core FormSupport
implementation.
-) Use the same basic logic that MultipleFormSupport implements but
instead of prefixing ids with form id values use the knowledge of
existing forms in the render cycle to re-create the internal state of
IdAllocator such that it is seeded in exactly the same state as that
used by previous forms so ID generation noise is kept as minimal as
possible.
For example, the following form layout:
<form jwcid="first@Form"><input jwcid="name@Textfield" /></form>
<form jwcid="second@Form"><input jwcid="name@Textfield" /></form>
would result in a more predictable html output of:
<form name="first" id="first"><input type="text" name="name" id="name" /></form>
<form name="second" id="second"><input type="text" name="name_0"
id="name_0" /></form>
I think this is what Andy wanted to move towards doing already but I
wanted to bring it up for discussion as I need to solve the problem
now-ish.
--
Jesse Kuhnert
Tapestry / OGNL / Dojo team member/developer
Open source based consulting work centered around
dojo/tapestry/tacos/hivemind. http://blog.opencomponentry.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: [discuss T4] Multiple form support && generated client/form element ids
Posted by Andreas Andreou <an...@gmail.com>.
- MultipleFormSupport isn't directly used by libraries AFAIK, but it's been
around since 4.1.1. I guess it can easily become an empty+deprecated
subclass of FormSupport if the new implementation just works.
- the FormSupportFactory concept simply allows pluggable FormSupport
and I'd keep it
- As for 'complicating things', I guess it just depends on the available
alternatives/solutions + I didn't mind having users do some
book-keeping on their own (relating to ids)
On Jan 10, 2008 6:52 PM, Jesse Kuhnert <jk...@gmail.com> wrote:
> Responses inlined.
>
> On Jan 10, 2008 11:26 AM, Andreas Andreou <an...@gmail.com> wrote:
> > > My only problem with the current MultipleFormSupport implementation is
> > > that it would unnecessarily lengthen generated client id values and
> > > almost never be able to honor the original component ID specified for
> > > a component.
> >
> > Yep, it just does formId:componentId everywhere... why the need to honor
> > the original id? clientId or peekClientId just works
>
> It only just works if you use it. In many instances people write
> javascript for their apps without using a Tapestry javascript template
> (myself included). It's also not really practical when writing
> Selenium based integration tests.
>
> I think trying to honor the original ID as much as possible is very
> important from a predictability / API intuitiveness point of view.
> The more the framework generates and changes the rendered content
> "automatically" the more some kinds of developers can feel like
> they've lost control and think the framework is messy. Look at a JSF
> generated page for a good example of this.
>
> >
> > > -) Remove MultipleFormSupport (or mark it as deprecated if it's too
> > > late) and handle this issue directly once in the core FormSupport
> > > implementation.
> > > -) Use the same basic logic that MultipleFormSupport implements but
> > > instead of prefixing ids with form id values use the knowledge of
> > > existing forms in the render cycle to re-create the internal state of
> > > IdAllocator such that it is seeded in exactly the same state as that
> > > used by previous forms so ID generation noise is kept as minimal as
> > > possible.
> >
> > Since it's configurable, why not just do an EnhancedFormSupport and make it the
> > default... If it works ok, we can then remoce/deprecate MultipleFormSupport
> >
>
> I'd like to try and reduce the complexity of the system where
> possible. As far as I can tell the only utility that the FormSupport
> factory currently provides is related to ID generation. Since the
> current FormSupport implementation is also responsible for managing
> this I think it makes sense to just "fix it" once and remove any
> concepts that are no longer needed.
>
> Makes the code base easier to read and understand imho. Did you have
> other plans for FormSupport that I'm not aware of?
>
> >
> > > For example, the following form layout:
> > >
> > > <form jwcid="first@Form"><input jwcid="name@Textfield" /></form>
> > > <form jwcid="second@Form"><input jwcid="name@Textfield" /></form>
> > >
> > > would result in a more predictable html output of:
> > >
> > > <form name="first" id="first"><input type="text" name="name" id="name" /></form>
> > > <form name="second" id="second"><input type="text" name="name_0"
> > > id="name_0" /></form>
> >
> >
> > I think there's still a problem with that approach - imagine the first
> > form initially hidden
> > and then rendered on an ajax request... since second hasn't already rendered,
> > idAllocator will give ids that already exist in the document...
> > And i'm sure there are variations of this problem floating around.
> >
> >
> > MultipleFormSupport doesn't solve this case either - what we use is a
> > further subclass
> > that just stores itself in the current cycle.
> >
> > This allows a custom NamespaceForm component to grab that instance and
> > configure its prefix to a used defined value (usually the id of the
> > entity to be processed)
> >
> > It's important that NamespaceForm sets the prefix in prepareForRender
> > - but that's all there is to it.
> >
> >
> Yeah, I know. This is a tough problem to solve. I think my current
> thinking is that I'll create an ultra slim text protocol for
> re-creating IdAllocator state so that the id seed values can be
> embedded as hidden form input data.
>
> Not to pick on the first point about code complexity too much, but
> already it looks like MultipleFormSupport and NamespaceForm are
> starting to complicate things some. Howard introduced the concept of
> namespaces in to the id allocation process already when he did the
> portlet work I think.
>
> My intention with this stuff isn't to pick on the current
> MultipleFormSupport, that work is obviously a great starting point
> for things and I'm sure has done a fine job solving some of these
> issues so far. I just hadn't really dug in to the details of how it
> works until now and so finally have some thoughts on how to attack it.
>
> Nothing I'm doing is set in stone of course. I'll start a branch for
> my work to try and minimize the impact on existing snapshot
> applications.
>
> Just for clarification though - is the MultipleFormSupport logic
> something that has gone out in a release or is being used by other
> libraries (like Tacos) ? Ideally I'd still like to try removing it in
> the branch (unless you have additional functionality you intended to
> implement that relies on MultipleFormSupport existing) unless it's
> something that is already released - in which case that will require a
> different approach.
>
>
> --
> Jesse Kuhnert
> Tapestry / OGNL / Dojo team member/developer
>
> Open source based consulting work centered around
> dojo/tapestry/tacos/hivemind. http://blog.opencomponentry.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>
--
Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
Tapestry / Tacos developer
Open Source / JEE Consulting
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: [discuss T4] Multiple form support && generated client/form element ids
Posted by Jesse Kuhnert <jk...@gmail.com>.
Responses inlined.
On Jan 10, 2008 11:26 AM, Andreas Andreou <an...@gmail.com> wrote:
> > My only problem with the current MultipleFormSupport implementation is
> > that it would unnecessarily lengthen generated client id values and
> > almost never be able to honor the original component ID specified for
> > a component.
>
> Yep, it just does formId:componentId everywhere... why the need to honor
> the original id? clientId or peekClientId just works
It only just works if you use it. In many instances people write
javascript for their apps without using a Tapestry javascript template
(myself included). It's also not really practical when writing
Selenium based integration tests.
I think trying to honor the original ID as much as possible is very
important from a predictability / API intuitiveness point of view.
The more the framework generates and changes the rendered content
"automatically" the more some kinds of developers can feel like
they've lost control and think the framework is messy. Look at a JSF
generated page for a good example of this.
>
> > -) Remove MultipleFormSupport (or mark it as deprecated if it's too
> > late) and handle this issue directly once in the core FormSupport
> > implementation.
> > -) Use the same basic logic that MultipleFormSupport implements but
> > instead of prefixing ids with form id values use the knowledge of
> > existing forms in the render cycle to re-create the internal state of
> > IdAllocator such that it is seeded in exactly the same state as that
> > used by previous forms so ID generation noise is kept as minimal as
> > possible.
>
> Since it's configurable, why not just do an EnhancedFormSupport and make it the
> default... If it works ok, we can then remoce/deprecate MultipleFormSupport
>
I'd like to try and reduce the complexity of the system where
possible. As far as I can tell the only utility that the FormSupport
factory currently provides is related to ID generation. Since the
current FormSupport implementation is also responsible for managing
this I think it makes sense to just "fix it" once and remove any
concepts that are no longer needed.
Makes the code base easier to read and understand imho. Did you have
other plans for FormSupport that I'm not aware of?
>
> > For example, the following form layout:
> >
> > <form jwcid="first@Form"><input jwcid="name@Textfield" /></form>
> > <form jwcid="second@Form"><input jwcid="name@Textfield" /></form>
> >
> > would result in a more predictable html output of:
> >
> > <form name="first" id="first"><input type="text" name="name" id="name" /></form>
> > <form name="second" id="second"><input type="text" name="name_0"
> > id="name_0" /></form>
>
>
> I think there's still a problem with that approach - imagine the first
> form initially hidden
> and then rendered on an ajax request... since second hasn't already rendered,
> idAllocator will give ids that already exist in the document...
> And i'm sure there are variations of this problem floating around.
>
>
> MultipleFormSupport doesn't solve this case either - what we use is a
> further subclass
> that just stores itself in the current cycle.
>
> This allows a custom NamespaceForm component to grab that instance and
> configure its prefix to a used defined value (usually the id of the
> entity to be processed)
>
> It's important that NamespaceForm sets the prefix in prepareForRender
> - but that's all there is to it.
>
>
Yeah, I know. This is a tough problem to solve. I think my current
thinking is that I'll create an ultra slim text protocol for
re-creating IdAllocator state so that the id seed values can be
embedded as hidden form input data.
Not to pick on the first point about code complexity too much, but
already it looks like MultipleFormSupport and NamespaceForm are
starting to complicate things some. Howard introduced the concept of
namespaces in to the id allocation process already when he did the
portlet work I think.
My intention with this stuff isn't to pick on the current
MultipleFormSupport, that work is obviously a great starting point
for things and I'm sure has done a fine job solving some of these
issues so far. I just hadn't really dug in to the details of how it
works until now and so finally have some thoughts on how to attack it.
Nothing I'm doing is set in stone of course. I'll start a branch for
my work to try and minimize the impact on existing snapshot
applications.
Just for clarification though - is the MultipleFormSupport logic
something that has gone out in a release or is being used by other
libraries (like Tacos) ? Ideally I'd still like to try removing it in
the branch (unless you have additional functionality you intended to
implement that relies on MultipleFormSupport existing) unless it's
something that is already released - in which case that will require a
different approach.
--
Jesse Kuhnert
Tapestry / OGNL / Dojo team member/developer
Open source based consulting work centered around
dojo/tapestry/tacos/hivemind. http://blog.opencomponentry.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org
Re: [discuss T4] Multiple form support && generated client/form element ids
Posted by Andreas Andreou <an...@gmail.com>.
> My only problem with the current MultipleFormSupport implementation is
> that it would unnecessarily lengthen generated client id values and
> almost never be able to honor the original component ID specified for
> a component.
Yep, it just does formId:componentId everywhere... why the need to honor
the original id? clientId or peekClientId just works
> -) Remove MultipleFormSupport (or mark it as deprecated if it's too
> late) and handle this issue directly once in the core FormSupport
> implementation.
> -) Use the same basic logic that MultipleFormSupport implements but
> instead of prefixing ids with form id values use the knowledge of
> existing forms in the render cycle to re-create the internal state of
> IdAllocator such that it is seeded in exactly the same state as that
> used by previous forms so ID generation noise is kept as minimal as
> possible.
Since it's configurable, why not just do an EnhancedFormSupport and make it the
default... If it works ok, we can then remoce/deprecate MultipleFormSupport
> For example, the following form layout:
>
> <form jwcid="first@Form"><input jwcid="name@Textfield" /></form>
> <form jwcid="second@Form"><input jwcid="name@Textfield" /></form>
>
> would result in a more predictable html output of:
>
> <form name="first" id="first"><input type="text" name="name" id="name" /></form>
> <form name="second" id="second"><input type="text" name="name_0"
> id="name_0" /></form>
I think there's still a problem with that approach - imagine the first
form initially hidden
and then rendered on an ajax request... since second hasn't already rendered,
idAllocator will give ids that already exist in the document...
And i'm sure there are variations of this problem floating around.
MultipleFormSupport doesn't solve this case either - what we use is a
further subclass
that just stores itself in the current cycle.
This allows a custom NamespaceForm component to grab that instance and
configure its prefix to a used defined value (usually the id of the
entity to be processed)
It's important that NamespaceForm sets the prefix in prepareForRender
- but that's all there is to it.
--
Andreas Andreou - andyhot@apache.org - http://blog.andyhot.gr
Tapestry / Tacos developer
Open Source / JEE Consulting
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org