Hi,

We do this now in DefaultRequestTargetResolverStrategy.resolveListenerInterfaceTarget(....)

            final Component component = page.get(pageRelativeComponentPath);
            if (component == null)
            {
                throw new WicketRuntimeException(
                        "Attempt to call listener method on non-existant component: " + componentPath);
            }
            if (!component.isVisibleInHierarchy())
            {
                throw new WicketRuntimeException(
                        "Calling listener methods on components that are not visible is not allowed: "
                                + componentPath);
            }
            if (!component.isEnabled())
            {
                throw new WicketRuntimeException(
                        "Calling listener methods on components that are not enabled is not allowed: "
                                + componentPath);
            }
            if (!component.isEnableAllowed())
            {
                throw new UnauthorizedActionException(component,Component.ENABLE);
            }
           
            // Ask the request listener interface object to create a request target
            return listener.newRequestTarget(page, component, listener, requestParameters);


So we are throwing exceptions when the component is not visible or not enabled.

This did go wrong for example in our PagingNavigator next page link (IncrementLink)
Because that one didn't redirect (i just fixed that because a increment link should redirect else you go next next next)
so when you do refresh (before my fix) and hit the last page and then another refresh. You get an error.
Normally i guess in this situations you just want an ignore. (current page rendered again)

Only mayeb the really security check isEnabledAllowed() should really throw an exception.

So i guess the change i propose would be:

            final Component component = page.get(pageRelativeComponentPath);
            if (component == null || !component.isEnabled() || !component.isVisibleInHierarchy())
            {
                return new RedirectPageRequestTarget(page);
            }
            if (!component.isEnableAllowed())
            {
                throw new UnauthorizedActionException(component, Component.ENABLE);
            }
           
            // Ask the request listener interface object to create a request target
            return listener.newRequestTarget(page, component, listener, requestParameters);

So if you call on something that just isn't there/visible/enabled then just redirect to the current page and be done with it.
Only if it is a security overridden enable then a real unauthorized exception will be thrown.

This fixes for example our previous problem with our IncrementLink
Or for example if you have an remove or delete link on something but that remove or delete link is calculated on the fly
(so isEnabled() is overridden) then it could be that on render time it is enabled. But at the time he clicks on it
some condition is changed so that isEnabled now returns false. then currently a exception page is thrown
But that is not what you want in that case. I guess just rendering the page again is i guess the most logical thing.

johan

Reply via email to