[jira] Commented: (TOMAHAWK-1332) UISchedule uses event queue for its decoding

["Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>]

    [ https://issues.apache.org/jira/browse/TOMAHAWK-1332?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12638702#action_12638702 ] 

Leonardo Uribe commented on TOMAHAWK-1332:
------------------------------------------

After doing several tests other bugs where detected. Since many bugs are present at the same time and t:schedule is a complex component, the explanation is long but I'll try to be as concrete as possible:

1. The constructor of UISchedule looks like this:

    public UISchedule()
    {
        super();
        _scheduleListener = new ScheduleActionListener();
        addActionListener(_scheduleListener); 
    }

The intention of the private ScheduleActionListener is update the property getSelectedEntry with the submitted value. If the component has immediate=true, the mouse and action events are handled on APPLY_REQUEST_VALUES phase, otherwise (default) on INVOKE_APPLICATION phase.

Add _scheduleListener to facesListeners is bad practice (since in fact this listener is an implementation detail non visible by the user). It causes a lot of problems, since the save and restore state methods should remove and add it, for avoid save it on the state. This hacks on save and restore state methods causes possible incompatibilities with trinidad optimized state saving (like unavoidable t:saveState problem)

To complicate the things more, listeners added with f:actionListener tag are executed before scheduleListener (due to restoreState hack), so they don't see which selectedEntry was clicked.

2. The code of  processAction of ScheduleActionListener looks like this:

        public void processAction(ActionEvent event)
                throws AbortProcessingException
        {
            UISchedule schedule = (UISchedule) event.getComponent();
            ScheduleEntry entry = schedule.getSubmittedEntry();
            schedule.getModel().setSelectedEntry(entry);
            schedule.setSubmittedEntry(null);

            if (schedule.getAction() != null)
            {
                getFacesContext().getApplication().getActionListener()
                        .processAction(event);
            }
        }

Suppose no action is defined for the component but the application has registered an actionListener. The event never reaches that listener. 

Since UISchedule implements ActionSource, it must check and call the application actionListener from its broadcast() method and not from some inner listener.

3. tomahawk12 UISchedule must implement ActionSource2, since it has getActionExpression and setActionExpression methods.

4. actionListener method of UISchedule is broken, since no ActionEvent cast is done before call, so if a MouseEvent is trigged, it fails. Code is show below.

    public void broadcast(FacesEvent event) throws AbortProcessingException
    {

        //.................
        super.broadcast(event);

        MethodBinding actionListener = getActionListener();

        if (actionListener != null)
        {
            actionListener.invoke(context, new Object[] { event });
        }
    }

Finally any solution must call _scheduleListener first before any other actionListener but after any mouseListener.



> UISchedule uses event queue for its decoding
> --------------------------------------------
>
>                 Key: TOMAHAWK-1332
>                 URL: https://issues.apache.org/jira/browse/TOMAHAWK-1332
>             Project: MyFaces Tomahawk
>          Issue Type: Bug
>          Components: Schedule
>    Affects Versions: 1.1.7
>            Reporter: Kennard Consulting
>
> org.apache.myfaces.custom.schedule.AbstractScheduleRenderer does most of the decoding for the UISchedule, but then relies on a deferred step (via the Event queue) to finish the job...
>    schedule.setSubmittedEntry(entry);
>    queueAction = true;
>    if (queueAction)
>       schedule.queueEvent(new ActionEvent(schedule));
> ...org.apache.myfaces.custom.schedule.UISchedule has a processAction that actually updates the model with the selectedEntry...
>     private class ScheduleActionListener implements ActionListener {
>         public void processAction(ActionEvent event)
>         {
>             schedule.getModel().setSelectedEntry(entry);
> I am unclear as to what the need is for this deferred step, but at any rate it is very brittle. If any other ActionListeners get fired before the ScheduleActionListener, they will find the model isn't ready for them yet. This is happening for me using RichFace's jsFunction tag, but it doesn't seem like it would be specific to that.
> Can we move this last step of the decoding into somewhere more appropriate? Ideally in the decode() method itself, but at least as a Listener during the APPLY_REQUEST_VALUES phase? Having it as a listener during the INVOKE_APPLICATION phase, when it could get ordered arbitrarily with respect to other ActionListeners, seems like asking for trouble.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.