Re: [jsr-314-open] PostAddToViewEvent publishing conditions still inconsistent

Hi Some days ago, I tried to check if publishing conditions for PostAddToViewEvent were fixed, but unfortunately it was not. These issues: - PostAddToViewEvent delivery specification needs clarification http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-805 (for 2.0 rev A) - Fix PostAddToViewEvent and PreRemoveFromViewEvent publishing conditions http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-782 (for 2.1_gf31_m5) were marked as fixed. This is what javadoc for JSF 2.1 says about it (I put the javadoc to make it more easier to understand): - (see UIComponent.getChildren method, the part in orange) "... After the child component has been added to the view, Application.publishEvent(javax.faces.context.FacesContext, java.lang.Class, java.lang.Object) must be called, passing PostAddToViewEvent.class as the first argument and the newly added component as the second argument if any the following cases are true. * FacesContext.getCurrentPhaseId() returns PhaseId.RESTORE_VIEW and partial state saving is enabled. * FacesContext.isPostback() returns false and FacesContext.getCurrentPhaseId() returns something other than PhaseId.RESTORE_VIEW..." - Another reference to the previous point (see PostAddToViewEvent class description) "... The implementation must guarantee that Application.publishEvent(javax.faces.context.FacesContext, java.lang.Class, java.lang.Object) is called, immediately after any UIComponent instance is added to the view hierarchy except in the case where ResponseStateManager.isPostback(javax.faces.context.FacesContext) returns true at the same time as FacesContext.getCurrentPhaseId() returns PhaseId.RESTORE_VIEW. When both of those conditions are met, Application.publishEvent(javax.faces.context.FacesContext, java.lang.Class, java.lang.Object) must not be called. ..." - (see UIComponent.setParent method) "... Set the parent UIComponent of this UIComponent. If parent.isInView() returns true, calling this method will first cause a PreRemoveFromViewEvent to be published, for this node, and then the children of this node. Then, once the re-parenting has occurred, a PostAddToViewEvent will be published as well, first for this node, and then for the node's children, but only if any of the following conditions are true. * FacesContext.getCurrentPhaseId() returns PhaseId.RESTORE_VIEW and partial state saving is enabled. * FacesContext.isPostback() returns false and FacesContext.getCurrentPhaseId() returns something other than PhaseId.RESTORE_VIEW ..." The first thing to notice is the description here is not what is happening right now in Mojarra (2.0 and 2.1). The description found in UIComponent.setParent is more close to the reality (the conditions related to parent.isInView() ) but the part that says "... but only if any of the following conditions are true ..." is not being preserved. The intention of those conditions are clear. See this coment from Andy Schwartz (JAVASERVERFACES_SPEC_PUBLIC-805): "... 2. We don't actually want to deliver PostAddToViewEvents during render response when Facelets temporarily removes/re-adds existing components from the tree. ..." Unfortunately, in practice the previous two conditions does not warrant that. Its more, there are valid scenarios in facelets when it is required to throw PostAddToViewEvent (for example, an ui:include src="#{someEL}" should propagate that event so the view could be restored). - (see PostAddToViewEvent class description) "... When an instance of this event is passed to SystemEventListener.processEvent(javax.faces.event.SystemEvent) or ComponentSystemEventListener.processEvent(javax.faces.event.ComponentSystemEvent), the listener implementation may assume that the source of this event instance is a UIComponent instance and that either that instance or an ancestor of that instance was just added to the view. Therefore, the implementation may assume it is safe to call UIComponent.getParent(), UIComponent.getClientId(), and other methods that depend upon the component instance being added into the view. ..." In practice, that's not true in Mojarra, but it is on MyFaces. The reason is Mojarra still uses a Listener attached to PostAddToViewEvent, making UIComponent.getClientId() "unstable". If the developer has a listener that is executed before the one doing relocation, UIComponent.getClientId() call will not work correctly. The central point of this mail is how should PostAddToViewEvent should really behave? In theory, and according to the latest JSF 2.1 spec, the original intention must be preserved. That means: - Deliver PostAddToViewEvents during restore view when partial state saving is enabled. - Don't deliver PostAddToViewEvents during render response when Facelets temporarily removes/re-adds existing components from the tree, but propagate it when the component is removed or added permanently. The current behavior of Mojarra, from the point of view of the spec should be consider a bug. How to do fix it? I propose do the following for Mojarra: 1. Check when a component is temporally removed/added on ComponentTagHandlerDelegate implementation. If that so, disable event processing using FacesContext.setProcessingEvents() method temporally while removal and addition is done. 2. Propagate another internal event (for example PostAddToViewOnRefreshBuild or something like that) to notify the listeners used for relocation (cc:insertChildren, cc:insertFacet, h:outputScript, h:outputStylesheet) to do their job like before. Anyway, it could be good to know how this should really work, and fix the relevant points on the spec. Suggestions are welcome. regards, Leonardo Uribe