[jira] Created: (MYFACES-2374) UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW

[My Faces - Dev mailing list]

UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
------------------------------------------------------------------------------------------------------------------

                 Key: MYFACES-2374
                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
             Project: MyFaces Core
          Issue Type: Task
          Components: JSR-314
            Reporter: Leonardo Uribe


Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12769164#action_12769164 ]

Jakob Korherr commented on MYFACES-2374:
----------------------------------------

Leonardo just found out the official statement about this at http://wiki.java.net/bin/view/Projects/Jsf2MR1ChangeLog.

C015 Errata UIViewRoot?.setBeforePhaseListeners() removed the statement that all phases including RestoreView? will have their beforePhaseListeners called. Reverted to the way it was in 1.2

But this means that we still have to call UIViewRoot.getAfterPhaseListener() after RESTORE_VIEW, right?
And what about the phaselisteners of getPhaseListeners(). Should they also only be called for after restore view? This would not make much sence, because the spec says that no afterPhase method should be invoked without a clean run of beforePhase before.

So I think the right thing to do is to only call UIViewRoot.getAfterPhaseListener() after RESTORE_VIEW.

Any other opinions?

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12769248#action_12769248 ]

Leonardo Uribe commented on MYFACES-2374:
-----------------------------------------

I think the best we can do is make a black box test of ri, see how this one is called and maybe use Thread.dumpStack to see from which part each call comes from, and replicate the same behavior on myfaces.

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12770633#action_12770633 ]

Jakob Korherr commented on MYFACES-2374:
----------------------------------------

The RI 2.0.1 does not call afterPhase on RESTORE_VIEW, but its javadocs says it does. I will write a comment to the EG and ask about this.

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12772086#action_12772086 ]

Jakob Korherr commented on MYFACES-2374:
----------------------------------------

posted mojarra issue: https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1391

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

     [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jakob Korherr updated MYFACES-2374:
-----------------------------------

    Status: Patch Available  (was: Open)

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch, restore_view_phaselistener_newest.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773623#action_12773623 ]

Leonardo Uribe commented on MYFACES-2374:
-----------------------------------------

The latest patch is incomplete by two reasons:

1. When partial state saving is used, UIViewRoot.processRestoreState is not called. Look how PostRestoreStateEvent is triggered.
2. The patch remove valid code calling PhaseListeners. Look how UIViewRoot.notifyListeners is called.

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch, restore_view_phaselistener_newest.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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

[My Faces - Dev mailing list]

    [ https://issues.apache.org/jira/browse/MYFACES-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773628#action_12773628 ]

Jakob Korherr commented on MYFACES-2374:
----------------------------------------

ad 1. Thanks - I did not know that
ad 2. It removes invalid code, because the PhaseListeners from getPhaseListeners() don't need to be called any more. They changed this behavior, see javadoc of UIViewRoot:

The default implementation must call UIComponentBase.processRestoreState(javax.faces.context.FacesContext, java.lang.Object) from within a try block. The try block must have a finally block that ensures that no FacesEvents remain in the event queue, and that the this.UIComponent.visitTree(javax.faces.component.visit.VisitContext, javax.faces.component.visit.VisitCallback) is called, passing a ContextCallback that takes the following action: call the UIComponent.processEvent(javax.faces.event.ComponentSystemEvent) method of the current component. The argument event must be an instance of PostRestoreStateEvent whose component property is the current component in the traversal.
--> They deleted the following: "that any PhaseListeners in getPhaseListeners() are invoked as appropriate"

Besides that, this implementation does not check, if the PhaseListener is registered for PhaseId.RESTORE_VIEW or PhaseId.ANY_PHASE.

> UIViewRoot.getBeforePhaseListener() and UIViewRoot.getAfterPhaseListener() could be called on PhaseId.RESTORE_VIEW
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2374
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2374
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>            Reporter: Leonardo Uribe
>         Attachments: restore_view_phaselistener.patch, restore_view_phaselistener_newest.patch
>
>
> Note that on jsf 1.2 this is not true. The problem with this one is how call UIViewRoot beforePhaseListener before PhaseId.RESTORE_VIEW, because in theory we need to "restore it" before call it. Maybe the solution is call it from the place where the state is restored (JspStateManagerImpl).

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