Hi
This is re: http://issues.apache.org/jira/browse/TAPESTRY-517
I was able to track down the bug. (Please read the report at the above URL,
it's fairly long).
The first half of the bug was that when a FieldLabel is rendered /after/ its
related form component, its for="" attribute was off by one, because
AbstractFormComponent.renderIdAttribute(...) didn't cache the component's
cliendId correctly (created by a call to cycle.getUniqueId(), which was added
sometime during Tapestry 4.0 development).
I posted a simple patch for this bug in JIRA.
The second half involves the same conditions, but manifests itself in
duplicate entries in the "formids"-hidden parameter like so:
<input type="hidden" name="formids"
value="bookkeeperRCB,bookkeeperRCB$0,adminRCB,adminRCB$0" />
which of course breaks the rewind. This has been first reported as
TAPESTRY-409.
This happens, because there's a "rather strange" (for me) setter in
AbstractFormComponent:
protected void setName(IForm form)
{
form.getElementId(this);
}
This ends up at FormSupportImpl.getElementId(...), which in turn retrieves an
unique id using its own IdAllocator-instance (which I think is redundant, as
IRequestCycle now carries one) and then procedes to call
AbstractFormComponent.setName(String name).
Now, if the FieldLabel precedes the field component, then everything's fine,
as AbstractFormComponent.renderComponent(...) checks if the field was
prerendered, and if it was, just writes the prerendered mark-up.
But if the FieldLabel is rendered after the field component, setName(IForm)
gets called a second time during the (unnecessary) prerender, which
causes two calls to getElementId(IFormComponent) which in turn results in
another call to setName(String), overwriting the real elementId recorded
during the first rendering.
I have to say... a setter with that kind of side-effect does not exactly
make good coding practice, especially with the abundant use of Tapestry's
auto-implementing of abstract properties, which is documented here (and
/only/ here):
http://wiki.apache.org/jakarta-tapestry/Tapestry31Status
under "Oct 2004" it says:
* 3.1 will create a transient property if an abstract accessor exists
for a property, even if there is no matching <property> (this falls
under dont repeat yourself).
nooooow... after explaining all of this, I'd like to fix the bug :-), but
there's multiple ways to go about it.
1. The best way to me seems to rip out all redundant IdAllocators and just use
the one in IRequestCycle, as multiple components seem to have their own
instances which looks like a bug waiting to happen to me. This would certainly
clean up the id allocation code. Further, using a wasRendered() method instead
of wasPrerendered() you could cache any component once it was rendered. (This
might break some things of course and might be put on the roadmap for 4.1(?))
2. The quick and easy way seems to be to simply check in
AbstractFormComponent.renderComponent(...) if getName() is non-empty and avoid
the call to setName(IForm) if it is. This would keep the potentially
unnecessary prerender cycle, but it'd be an easy fix with minimal potential
to break things.
Unfortunately I have an exam coming up on friday, so I can't work on 1, but
I've attached a new patch to TAPESTRY-517 which works for me at a first
glance.
cheers,
Jonas Maurus
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
