You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Pedro Santos <pe...@gmail.com> on 2010/11/24 11:46:37 UTC

isVisibleInHierarchy() possibly unnecessarily checks > children whose parents are invisible?

Hi Martin, I know your concern, but is the visitor responsibility to don't
go deeper in not visible components if your logic require such restriction.
I attached on the ticket an test case demonstrating how it can be done.
Basically you provide an implementation to the IFormVisitorParticipant
interface returning false when the component is not visible.

P.S. I just realise that I was voting in an not related voting thread.

> [WICKET-3166] - isVisibleInHierarchy() possibly unnecessarily checks
> children whose parents are invisible?
> I'm +1 to revert the change at this ticket because we start to make
> unnecessary visibility checks due an recursion that always stack calls to
> every component parent. I know it is important to respect any parent
> visibility restriction, but if the component has an restriction by itself,
> than we don't need to code such parent test.


If child is not designed to be visible it may throw exception at
child.isVisible. This fix was originally to prevent child.isVisible
being called when parent is not visible.

Reverting this fix might break applications and there is a test case
attached that will demonstrate this issue.

**
Martin



-- 
Pedro Henrique Oliveira dos Santos

Re: isVisibleInHierarchy() possibly unnecessarily checks > children whose parents are invisible?

Posted by Martin Makundi <ma...@koodaripalvelut.com>.
> Yes, would make more sense if the Form#inputChanged invoke an
> visitVisibleAndEnabledFormComponentsPostOrder method.
> Is ok to provide and use such API in the 1.4?
> But even without this visitor change in the 1.4, you can use the
> IFormVisitorParticipant interface to stop it from going deeper.

I don't want to do anything. I just want it to work. That's what this fix does.


**
Martin

>
> On Wed, Nov 24, 2010 at 8:48 AM, Martin Makundi <
> martin.makundi@koodaripalvelut.com> wrote:
>
>> Hi!
>>
>> > Hi Martin, I know your concern, but is the visitor responsibility to
>> don't
>> > go deeper in not visible components if your logic require such
>> restriction.
>> > I attached on the ticket an test case demonstrating how it can be done.
>> > Basically you provide an implementation to the IFormVisitorParticipant
>> > interface returning false when the component is not visible.
>> >
>> > P.S. I just realise that I was voting in an not related voting thread.
>>
>> That is true. But it is the built-in form visitor that has this issue.
>>
>> Please dont' revert this fix before fixing the formcomponent visitor ;]
>>
>> **
>> Martin
>>
>>
>> >
>> >> [WICKET-3166] - isVisibleInHierarchy() possibly unnecessarily checks
>> >> children whose parents are invisible?
>> >> I'm +1 to revert the change at this ticket because we start to make
>> >> unnecessary visibility checks due an recursion that always stack calls
>> to
>> >> every component parent. I know it is important to respect any parent
>> >> visibility restriction, but if the component has an restriction by
>> itself,
>> >> than we don't need to code such parent test.
>> >
>> >
>> > If child is not designed to be visible it may throw exception at
>> > child.isVisible. This fix was originally to prevent child.isVisible
>> > being called when parent is not visible.
>> >
>> > Reverting this fix might break applications and there is a test case
>> > attached that will demonstrate this issue.
>> >
>> > **
>> > Martin
>> >
>> >
>> >
>> > --
>> > Pedro Henrique Oliveira dos Santos
>> >
>>
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>

Re: isVisibleInHierarchy() possibly unnecessarily checks > children whose parents are invisible?

Posted by Pedro Santos <pe...@gmail.com>.
Yes, would make more sense if the Form#inputChanged invoke an
visitVisibleAndEnabledFormComponentsPostOrder method.
Is ok to provide and use such API in the 1.4?
But even without this visitor change in the 1.4, you can use the
IFormVisitorParticipant interface to stop it from going deeper.

On Wed, Nov 24, 2010 at 8:48 AM, Martin Makundi <
martin.makundi@koodaripalvelut.com> wrote:

> Hi!
>
> > Hi Martin, I know your concern, but is the visitor responsibility to
> don't
> > go deeper in not visible components if your logic require such
> restriction.
> > I attached on the ticket an test case demonstrating how it can be done.
> > Basically you provide an implementation to the IFormVisitorParticipant
> > interface returning false when the component is not visible.
> >
> > P.S. I just realise that I was voting in an not related voting thread.
>
> That is true. But it is the built-in form visitor that has this issue.
>
> Please dont' revert this fix before fixing the formcomponent visitor ;]
>
> **
> Martin
>
>
> >
> >> [WICKET-3166] - isVisibleInHierarchy() possibly unnecessarily checks
> >> children whose parents are invisible?
> >> I'm +1 to revert the change at this ticket because we start to make
> >> unnecessary visibility checks due an recursion that always stack calls
> to
> >> every component parent. I know it is important to respect any parent
> >> visibility restriction, but if the component has an restriction by
> itself,
> >> than we don't need to code such parent test.
> >
> >
> > If child is not designed to be visible it may throw exception at
> > child.isVisible. This fix was originally to prevent child.isVisible
> > being called when parent is not visible.
> >
> > Reverting this fix might break applications and there is a test case
> > attached that will demonstrate this issue.
> >
> > **
> > Martin
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>



-- 
Pedro Henrique Oliveira dos Santos

Re: isVisibleInHierarchy() possibly unnecessarily checks > children whose parents are invisible?

Posted by Martin Makundi <ma...@koodaripalvelut.com>.
Hi!

> Hi Martin, I know your concern, but is the visitor responsibility to don't
> go deeper in not visible components if your logic require such restriction.
> I attached on the ticket an test case demonstrating how it can be done.
> Basically you provide an implementation to the IFormVisitorParticipant
> interface returning false when the component is not visible.
>
> P.S. I just realise that I was voting in an not related voting thread.

That is true. But it is the built-in form visitor that has this issue.

Please dont' revert this fix before fixing the formcomponent visitor ;]

**
Martin


>
>> [WICKET-3166] - isVisibleInHierarchy() possibly unnecessarily checks
>> children whose parents are invisible?
>> I'm +1 to revert the change at this ticket because we start to make
>> unnecessary visibility checks due an recursion that always stack calls to
>> every component parent. I know it is important to respect any parent
>> visibility restriction, but if the component has an restriction by itself,
>> than we don't need to code such parent test.
>
>
> If child is not designed to be visible it may throw exception at
> child.isVisible. This fix was originally to prevent child.isVisible
> being called when parent is not visible.
>
> Reverting this fix might break applications and there is a test case
> attached that will demonstrate this issue.
>
> **
> Martin
>
>
>
> --
> Pedro Henrique Oliveira dos Santos
>