Hi all, I'm terribly sorry for the late review, but here are some minor API/JavaDoc issues I found while locally integrating those change to MyFaces : ________________________________________________________________________ javax.faces.application.Application public abstract void addBehavior(String behaviorId, String behaviorClass); public abstract Behavior createBehavior(String behaviorId) throws FacesException; public abstract Iterator<String> getBehaviorIds(); All those methods should not be abstract and throw UnsupportedOperationException to be consistent with other methods added after JSF 1.1. On that UnsupportedOperationException, would it be possible to revisit that strategy for 2.1? I know we provide that for binary backward compatibility, but in real life usage it's only dust thrown in the eyes of users imho. In fact, it only delays the error until the application is executed and it's then harder to fix issues as you have to redeploy the application after every fixes, as opposed to compilation errors that all show up at the same time. For example, 2.0 API have UIViewRoot depend on PartialViewContext accessed through a new getPartialViewContext on the FacesContext. By default that method now throws an UnsupportedOperationException so if the user application use a FacesContext decorator, the application WILL crash at runtime while a real abstract method would have warned the user about the missing method that he could have redirected to the wrapped instance. Also, since 2.0 provides a lot of new Wrapper classes, assuming users will start using them, then adding new abstract methods should no longer be much of an issue since by default the provided Wrapper classes will already take care of redirecting those. _____________________________________________________________________________ javax.faces.component.behavior.AjaxBehavior public Set<BehaviorHint> getHints() public String getRendererType() Those methods are either not needed as they only call super (copied doc), or the JavaDoc is incorrect. _____________________________________________________________________________ javax.faces.component.UIComponent public void unsubscribeFromEvent(Class<? extends SystemEvent> eventClass, ComponentSystemEventListener componentListener) The JavaDoc says 'When doing the comparison to determine if an existing listener is equal to the argument componentListener (and thus must be removed), the equals() method on the existing listener must be invoked, passing the argument componentListener, rather than the other way around.' However equals should always be commutative so I don't really know if such really low and strange implementation detail should be part of the JavaDoc _____________________________________________________________________________ javax.faces.component.UIComponentBase public FacesListener[] getFacesListeners(Class clazz) Method is not generic enabled, it should be public FacesListener[] getFacesListeners(Class<? extends FacesListener> clazz) Also, on a slightly different but related note, BehaviorBase class also has the listener handling methods, but does not provide the getBehaviorsListeners method, should it be added for consistency purposes? _____________________________________________________________________________ javax.faces.model.DataModel public Iterator<E> iterator() The only way for the returned Iterator to get row data is to set the model's rowIndex (because there's no getRowData(int index) method on DataModel). Therefore, the model become inherently not thread safe. I believe such limitation should be mentioned in the class' documentation. Of course, DataModel were already not thread safe in nearly all implementation so it's only an extra documentation needed here for new users imho so they don't try to place those in application scoped beans. Also since Java 5.0 allows return type covariance for overridden methods I think we could change the various getWrapped() methods to return relevant type (E[]m List<E>, etc.) _____________________________________________________________________________ javax.faces.webapp.pdl.facelets.FaceletContext public static final String FACELET_CONTEXT_KEY = "com.sun.faces.facelets.FACELET_CONTEXT"; Can we get a "javax.faces.XYZ" constant value for that please? _____________________________________________________________________________ javax.faces.event.ExceptionQueuedEventContext public static final String IN_AFTER_PHASE_KEY; public static final String IN_BEFORE_PHASE_KEY; The spec does not define any value at all for those two constants. _____________________________________________________________________________ That's about it! Regards, ~ Simon ________________________________ From: JSR 314 Open Mailing list on behalf of Ed Burns Sent: Wed 3/11/2009 10:51 AM To: JSR-314-OPEN(a)JCP.ORG Subject: Re: [jsr-314-open] [ADMIN] JSF 2.0 Spec Snapshot PM> Ed, PM> Here are some questions: PM> * is delta state saving in this revision? The state saving stuff is PM> still in an appendix, and I find it hard to check. It's not in the revision. The stuff in the appendix is now quite stale due to the lifecycle changes introduced to accomodate Andy Schwartz and Dan Allen's concerns over the performance of view metada gathering. Ryan is currently reworking the state saving branch, including Martin's recent patches, but it's tricky. The state management revision may have to be slipped to 2.1. PM> * Issue 450 (https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=450 PM> ) still hasn't been merged into the spec - I've asked about this one PM> multiple times. Can we get this merged in? THANKS for your persistence. This email will include an action plan for Roger and I to follow, and this will be on it. PM> * the big changes due to view parameters don't appear to be merged PM> in? See action plan below. PM> Otherwise, the areas I have been involved in look good: PM> * exception handling PM> * bean validation Thanks for the oversight. MM> Hi Ed, MM> my feedback below. I already (wrongly) sent these using my Gmail-address, MM> but I guess (hope) these mails didn't make it... I've put these into the action plan. MM> also, should I send out the patch-proposal that I made and the MM> follow up discussions I had with Leonardo to the EG? Pete and Andy MM> seemed to be interested... AS> Is this regarding delta state saving? I am definitely curious to hear AS> where we ended up with this. As mentioned above, late breaking view parameter changes have impacted our ability to include the state saving changes into 2.0. PM> Another couple more after talking to Dan: In action plan PM> RegexValidator ACTION PLAN for Roger and Ed. These need to be done to the spec. rogerk * 10.3.1.1: in table 10.3, the f:ajax attribute is called event - in several paragraphs around this, events - which one is true? It is now "event". Roger should be taking care of this soon. edburns * Merge spec issue [SelectItemsCollection-450] into spec. edburns * Merge all view parameter changes into spec. edburns * 3.1.8: should the tree-visitor be mentioned here? edburns * 4.1.18.4: shouldn't this reference AfterAddToViewEvent? edburns * 5.4.1: seems to have old spec language (language on error handler says that only predestroy does not call through to it) edburns * 6.1.14: managed-bean-scope can now be view on top of none, request, session, application - can it also be flash? I don't see this mentioned anywhere. edburns * the spec language for the BeanValidator is light compared to other validators (see 3.5.5 - for example RegexValidator's description vs. BeanValidator Thanks for your feedback! Ed -- | ed.burns(a)sun.com | office: 408 884 9519 OR x31640 | homepage: | http://ridingthecrest.com/