Hi Martin,
Thanks for merging that.
I have been working on a patch for this, but got stuck with some errors
introduced in HtmlHeaderContainer, Border and a few others. Will
continue working on it this weekend.
Thanks,
Jesse
On 26/05/2014 21:51, Martin Grigorov wrote:
Hi Jesse,
I've applied your patches for WICKET-5569 and WICKET-5580.
Please create a new ticket with this improvement + a test.
Martin Grigorov
Wicket Training and Consulting
On Mon, Apr 28, 2014 at 11:07 PM, Martin Grigorov <[email protected]>wrote:
Hi Jesse,
The improvement sounds good !
I won't be able to work on it this week but I'm sure you will create a
patch with a test by the time I'm back ;)
Martin Grigorov
Wicket Training and Consulting
On Sun, Apr 27, 2014 at 10:50 PM, Jesse Long <[email protected]> wrote:
Hi Wicket Devs,
ComponentResolvers#resolve() first tries ComponentResolvers#
resolveByComponentHierarchy().
This tries the immediate container going all the way up the hierarchy
attempting to resolve the component is the container is also a
IComponentResolver.
I'm worried that the wrong problem is being fixed in the wrong place here.
Basically, I understand the need for this code as:
TransparentWebMarkupContainer inside TransparentWebMarkupContainer would
not work, because TransparentWebMarkupContainer just calls get(String) on
its parent. If its parent is also transparent, the get(String) is not going
to return the component, instead, resolve() should be called.
Therefore, we must try the first one, and if that fails, continue up the
tree in case the parent is also a TransparentWebMarkupContainer.
This smells like the ComponentResolvers class fixing
TransparentWebMarkupContainer bugs.
Rather, we could just make TransparentWebMarkupContainer first try
get(String) on the parent, if that fails, check if the parent is a
IComponentResolver and if so, call resolve() on it.
Like:
@Override
public Component resolve(MarkupContainer container, MarkupStream
markupStream, ComponentTag tag)
{
MarkupContainer parent = getParent();
Component resolvedComponent = parent.get(tag.getId());
/*
* If get(String didn't work, parent may be a IComponentResolver,
in which case we must call
* resolve() on it.
*/
if (resolvedComponent == null && parent instanceof
IComponentResolver)
{
resolvedComponent = ((IComponentResolver)parent).resolve(container,
markupStream, tag);
}
if (resolvedComponent != null && getPage().wasRendered(
resolvedComponent))
{
/*
* Means that parent container has an associated homonymous
tag to this grandchildren
* tag in markup. The parent container wants render it and it
should be not resolved to
* their grandchildren.
*/
return null;
}
return resolvedComponent;
}
When using a TransparentWebMarkupContainer, it is reasonable to expect
the component to be resolved from the component bound to the grandparent of
the component tag in the markup hierarchy, but with the current plan, the
component could be resolved from the great great great grandparent.
Current behavior probably also leads to some confusing error messages
when a component cannot be resolved, where a component deep in the
hierarchy could "steal" a component which has not yet been rendered from an
ancestor, and the error message would be about how the ancestor is missing
a component or it was already rendered etc. User would have a hard time
figuring out that the problem is actually deeper inside the hierarchy.
Thoughts?
Thanks,
Jesse
