You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Juergen Donnerstag <ju...@gmail.com> on 2008/04/01 20:45:01 UTC

Changing tag from open-close to open-body-close

wicket 1458 is about wicket not performing completely well if tags are
changed from open-close to open-body-close, e.g. like this

			protected void onComponentTag(ComponentTag tag)
			{
				if (tag.isOpenClose())
				{
					tag.setType(XmlTag.OPEN);
				}
				super.onComponentTag(tag);
			}

what happens is that though the original tag is an open-close, and
thus doesn't have a body, renderComponentTagBody() gets called (it
tests the modified tag not the original one). We could fix that in
Component.renderComponent() but obviously some components e.g. Border
are relying on that behavior. I would therefor suggest to make the
following change to MarkupContainer.renderComponentTagBody().

	protected final void renderComponentTagBody(final MarkupStream markupStream,
		final ComponentTag openTag)
	{
		if ((markupStream != null) && (markupStream.getCurrentIndex() > 0))
		{
			// If the original tag has been changed from open-close to open-body-close,
			// than historically renderComponentTagBody gets called, but actually
			// it shouldn't do anything since there is no body for that tag.
			ComponentTag origOpenTag =
(ComponentTag)markupStream.get(markupStream.getCurrentIndex() - 1);
			if (origOpenTag.isOpenClose())
			{
				return;
			}
		}

                .....
       }

It works well for all our test cases but I wonder whether this change
should go in 1.3.3 (to be released this week) without a lot of testing
from people using trunk. The change looks straight forward, but you
never know, especially since this is at the very core of rendering.

Juergen

Re: Changing tag from open-close to open-body-close

Posted by Johan Compagner <jc...@gmail.com>.
doesn't sound to dangerous no.
There isn't a overridable method called (that they could have overridden)
right?

the only thing is that now it tries to render a child component (that isnt
there?)

then it sounds fine with me

johan

On Tue, Apr 1, 2008 at 8:45 PM, Juergen Donnerstag <
juergen.donnerstag@gmail.com> wrote:

> wicket 1458 is about wicket not performing completely well if tags are
> changed from open-close to open-body-close, e.g. like this
>
>                        protected void onComponentTag(ComponentTag tag)
>                        {
>                                if (tag.isOpenClose())
>                                {
>                                        tag.setType(XmlTag.OPEN);
>                                }
>                                super.onComponentTag(tag);
>                        }
>
> what happens is that though the original tag is an open-close, and
> thus doesn't have a body, renderComponentTagBody() gets called (it
> tests the modified tag not the original one). We could fix that in
> Component.renderComponent() but obviously some components e.g. Border
> are relying on that behavior. I would therefor suggest to make the
> following change to MarkupContainer.renderComponentTagBody().
>
>        protected final void renderComponentTagBody(final MarkupStream
> markupStream,
>                final ComponentTag openTag)
>        {
>                if ((markupStream != null) && (markupStream.getCurrentIndex()
> > 0))
>                {
>                        // If the original tag has been changed from
> open-close to open-body-close,
>                        // than historically renderComponentTagBody gets
> called, but actually
>                        // it shouldn't do anything since there is no body
> for that tag.
>                        ComponentTag origOpenTag =
> (ComponentTag)markupStream.get(markupStream.getCurrentIndex() - 1);
>                        if (origOpenTag.isOpenClose())
>                        {
>                                return;
>                        }
>                }
>
>                .....
>       }
>
> It works well for all our test cases but I wonder whether this change
> should go in 1.3.3 (to be released this week) without a lot of testing
> from people using trunk. The change looks straight forward, but you
> never know, especially since this is at the very core of rendering.
>
> Juergen
>