I need to look better on which core components are relying on an updated
visibility/enabled state at the process event time, and why the last
rendered state wouldn't be enough to them to work nicely.
On Tue, Nov 30, 2010 at 3:19 PM, Igor Vaynberg <igor.vaynb...@gmail.com>wrote:
> currently we only invoke configure before the render. this would mean
> we would have to invoke it before processing a listener, clearing the
> cache, and then invoking it again before render. i wonder if that is
> enough places to invoke it....
>
> -igor
>
> On Tue, Nov 30, 2010 at 9:15 AM, Pedro Santos <pedros...@gmail.com> wrote:
> > If user click an link, it will change the value of some property at the
> > process_event request cycle step. Then the processor will go to the
> respond
> > step, will invoke every component before render method which will end up
> > invoking the Component#configure and updating the visibility/enabled
> state
> > (even if it changes, we are able to work with the updated state). So when
> > the this component has the opportunity to render it self, it will be
> aware
> > its update state.
> >
> > On Tue, Nov 30, 2010 at 2:39 PM, Igor Vaynberg <igor.vaynb...@gmail.com
> >wrote:
> >
> >> there are other places that should be checked though. for example
> >> before we invoke a listener on the component we should check again to
> >> make sure that visibility hasnt changed. eg if visibility depends on
> >> some property of the user clicking the link that changed between
> >> render and clicking the link.
> >>
> >> -igor
> >>
> >> On Tue, Nov 30, 2010 at 8:30 AM, Pedro Santos <pedros...@gmail.com>
> wrote:
> >> > An implementation idea:
> >> >
> >> > Component {
> >> > public final void configure()
> >> > {
> >> > if (!getFlag(FLAG_CONFIGURED))
> >> > {
> >> > setVisible_NoClientCode(isVisible()); //we only check the
> user
> >> > isVisible in here
> >> > onConfigure();
> >> > setFlag(FLAG_CONFIGURED, true);
> >> > }
> >> > }
> >> > }
> >> >
> >> > On Tue, Nov 30, 2010 at 2:16 PM, Igor Vaynberg <
> igor.vaynb...@gmail.com
> >> >wrote:
> >> >
> >> >> so how is it different if they can still override something that
> needs
> >> >> to be checked all the time?
> >> >>
> >> >> -igor
> >> >>
> >> >> On Tue, Nov 30, 2010 at 8:02 AM, Pedro Santos <pedros...@gmail.com>
> >> wrote:
> >> >> > I understand the concern about possible isVisible implementations
> like
> >> >> >
> >> >> > isVisible(return currentlyTime < 10:00:00;) //imagine this
> component
> >> >> being
> >> >> > rendered at 09:59:59
> >> >> > isVisible(return dao.list().size() > 0);// performance issues
> >> >> >
> >> >> > But maybe we can have the best from both approaches. This is an
> >> >> copy/paste
> >> >> > from java.awt.Component:
> >> >> >
> >> >> > public boolean isVisible() {
> >> >> > return isVisible_NoClientCode();
> >> >> > }
> >> >> > final boolean isVisible_NoClientCode() {
> >> >> > return visible;
> >> >> > }
> >> >> >
> >> >> > There are some points in the awt framework were the isVisible
> method
> >> is
> >> >> not
> >> >> > used in benefit of isVisible_NoClientCode
> >> >> > I'm in favor of create an final isVisible/Enabled version and
> change
> >> the
> >> >> > Wicket core to use it. Also maintain the hotspot to users provide
> >> their
> >> >> > isVisible/Enable implementations that will serve to feed the core
> >> >> component
> >> >> > state.
> >> >> >
> >> >> > On Mon, Nov 29, 2010 at 4:56 PM, Igor Vaynberg <
> >> igor.vaynb...@gmail.com
> >> >> >wrote:
> >> >> >
> >> >> >> ive run into plenty of weird problems with overrides, but maybe
> >> >> >> because this was in a high concurrency app where data changed
> >> >> >> frequently. the problems arise from the fact that the value
> returned
> >> >> >> from isvisible() can change while we are doing traversals, etc.
> >> >> >>
> >> >> >> eg we run a traversal for all visible components and put them in a
> >> >> >> list. later we iterate over the list and try to render these
> >> >> >> components. the render function also checks their visibility and
> if
> >> >> >> they are no longer visible it throws an exception.
> >> >> >>
> >> >> >> if isvisible() override depends on some external factor like the
> >> >> >> database there is a small window where the value can change and
> now
> >> >> >> you can have a weird exception: such as "tried to invoke a
> listener
> >> on
> >> >> >> a component that is not visible or not enabled". these are very
> >> >> >> intermittent and damn near impossible to reproduce.
> >> >> >>
> >> >> >> another problem is performance. isvisible() is called multiple
> times
> >> >> >> during the request and if it depends on the database it can be a
> >> >> >> performance problem. in fact a couple of users have complained
> about
> >> >> >> this on the list in the past. at least now we have an easy
> solution
> >> >> >> for them - use onconfigure().
> >> >> >>
> >> >> >> so as of right now the developers have two choices: override
> >> >> >> isvisible() and potentially suffer the consequences. or, override
> >> >> >> onconfigure() and set visibility there in a more deterministic
> >> >> >> fashion.
> >> >> >>
> >> >> >> -igor
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> On Mon, Nov 29, 2010 at 10:21 AM, Eelco Hillenius
> >> >> >> <eelco.hillen...@gmail.com> wrote:
> >> >> >> > To expand, unless I'm missing something (new?), things are
> really
> >> only
> >> >> >> > problematic when both the mutable value and the override are
> mixed.
> >> In
> >> >> >> > a way, I think that using the override is 'more pure', as it's a
> >> >> >> > simple function that is executed when needed, whereas mutable
> state
> >> >> >> > can be harder to deal with when trying to figure out how it got
> to
> >> be
> >> >> >> > in that state. So, sorry Igor, but we disagree on this one.
> >> >> >> >
> >> >> >> > Eelco
> >> >> >> >
> >> >> >> >
> >> >> >> > On Mon, Nov 29, 2010 at 10:13 AM, Eelco Hillenius
> >> >> >> > <eelco.hillen...@gmail.com> wrote:
> >> >> >> >> Niether is evil. It has potential pitfalls, which you should
> just
> >> be
> >> >> >> >> aware of. We use such overrides all over the place and never
> have
> >> >> >> >> problems with them either. :-) Avoiding it is safer, but also
> more
> >> >> >> >> verbose (in 1.3.x at least).
> >> >> >> >>
> >> >> >> >> Eelco
> >> >> >> >>
> >> >> >> >> On Mon, Nov 29, 2010 at 9:49 AM, Igor Vaynberg <
> >> >> igor.vaynb...@gmail.com>
> >> >> >> wrote:
> >> >> >> >>> On Mon, Nov 29, 2010 at 9:35 AM, Sven Meier <s...@meiers.net>
> >> >> wrote:
> >> >> >> >>>> Hi Douglas,
> >> >> >> >>>>
> >> >> >> >>>> WICKET-3171 describes a problematic case, where visibility of
> a
> >> >> >> >>>> component changes while its form is being processed.
> >> >> >> >>>> In our projects we're overriding isVisible() where
> appropriate
> >> and
> >> >> >> never
> >> >> >> >>>> encountered a similar problem.
> >> >> >> >>>>
> >> >> >> >>>> I'd say WICKET-3171 is the rare 5% usecase. What's next, is
> >> >> overriding
> >> >> >> >>>> isEnabled() going to be declared evil too? ;)
> >> >> >> >>>
> >> >> >> >>> yes
> >> >> >> >>>
> >> >> >> >>> -igor
> >> >> >> >>>
> >> >> >> >>>>
> >> >> >> >>>> Sven
> >> >> >> >>>>
> >> >> >> >>>> On Mon, 2010-11-29 at 11:22 -0600, Douglas Ferguson wrote:
> >> >> >> >>>>
> >> >> >> >>>>> Can you explain why? We have done this all over the place.
> >> >> >> >>>>>
> >> >> >> >>>>> D/
> >> >> >> >>>>>
> >> >> >> >>>>> On Nov 29, 2010, at 10:00 AM, Martin Grigorov wrote:
> >> >> >> >>>>>
> >> >> >> >>>>> > The recommended way since a few 1.4 releases is to
> override
> >> >> >> onConfigure()
> >> >> >> >>>>> > and call setVisible(true|false) depending on your
> conditions.
> >> >> >> >>>>> >
> >> >> >> >>>>> > On Mon, Nov 29, 2010 at 4:49 PM, Douglas Ferguson <
> >> >> >> >>>>> > doug...@douglasferguson.us> wrote:
> >> >> >> >>>>> >
> >> >> >> >>>>> >> Igor posted a comment to this bug saying that overriding
> >> >> >> isVisible() is
> >> >> >> >>>>> >> "evil"
> >> >> >> >>>>> >>
> >> >> >> >>>>> >> https://issues.apache.org/jira/browse/WICKET-3171
> >> >> >> >>>>> >>
> >> >> >> >>>>> >> I was surprised by this and am curious to hear more.
> >> >> >> >>>>> >>
> >> >> >> >>>>> >> D/
> >> >> >> >>>>>
> >> >> >> >>>>
> >> >> >> >>>>
> >> >> >> >>>>
> >> >> >> >>>
> >> >> >> >>
> >> >> >> >
> >> >> >>
> >> >> >
> >> >> >
> >> >> >
> >> >> > --
> >> >> > Pedro Henrique Oliveira dos Santos
> >> >> >
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> > Pedro Henrique Oliveira dos Santos
> >> >
> >>
> >
> >
> >
> > --
> > Pedro Henrique Oliveira dos Santos
> >
>
--
Pedro Henrique Oliveira dos Santos
