I would slightly lean to this solution: Solution two:
Check if EL is set and ValueExpression.isReadOnly(ELContext context) returns false set EL else set local and fix the el-implementation if it doesn't do what it is expected to do. Second problem (resetting) is nothing we can do generally; we'd have to do it case-by-case. regards, Martin On Mon, Mar 24, 2008 at 7:04 PM, Andrew Robinson <[EMAIL PROTECTED]> wrote: > The bug https://issues.apache.org/jira/browse/TOMAHAWK-858 has made me > want to bring this up for discussion of the entire team. I really > don't like the solution they are posing as it will cause backward > compatibility problems and is not a full solution. This problem is not > unique to the Tomahawk tabbed pane, so I would love to see a MyFaces > wide (and maybe a JSF 2.0) fix (or approach) to the problem. > > The issue is that several components use a "setXxx(...)" method to > update components. This typically is done in renderer code. Here is a > short list of components that I know are affected: > > Tomahawk data scroller / data table and the first attribute > Tomahawk tabbed pane > Trinidad UIXShowDetail (I checked in a one component fix for this one, > but I am not 100% happy with it) > > I am sure there are more. The problem is that if I have this code (I > am picking on the data table it is the easiest example): > > <t:dataTable first="#{bean.firstRowIndex}"...> > <t:dataScroller ...> > ... > > The data scroller has the code UIData.setFirst(...) when the event is > broadcast. > > Typically all MyFaces getter code is written as: > > if there is a local value > return it > if not, get the value binding / value expression > if set, evaluate it > if the value is null or there was no EL, then return a default value > if available, or else null > > The setter code is: > > set the local value > > Even with Trinidad using a FacesBean, it still suffers from the local > vs. EL problem. What ends up being the major issue is that the local > value always takes precedence over the EL value. This is very rarely > the desired behavior. The local value is only really useful when > component binding is being used and the page author is not using EL to > set attributes. > > The solution is more complex. In issue TOMAHAWK-858, someone has > proposed to use EL if it is set, but they did not examine the problem > domain. For example: > > <t:dataTable first="#{bean.condition ? 0 : bean.firstRowIndex}" ... > > In this crude example, perhaps the hard-coded 0 is to reset the table > to the first record on another link (please do not over analyze the > example, I know it is far from perfect). The problem with this is that > in using a conditional EL expression, this is no longer set-able! So > if the data scroller code attempts to assign 20 (the next page lets > say) to the first EL, it will throw an exception, because a > conditional statement cannot be set. > > One solution to this is: > if EL is set, > try to set the EL > if that throws an exception, set the local value > > This would technically work, but I hate the code. It is unpredictable > and bad for performance (exceptions are expensive and should be > avoided). > > Solution two: > > Check if EL is set and ValueExpression.isReadOnly(ELContext context) > returns false > set EL > else set local > > Problem is that I do not think that this is always accurate. > > The other problem is that once the local value is set, it cannot > always be cleared: > > private Integer _first; > public int getFirst() { > if (_first != null) return _first.intValue(); > ValueBinding ... > } > public int setFirst(int first) { _first = new Integer(first); } > > Here, there is no way to set _first back to null. We could change the > APIs drastically of Tomahawk and Trinidad so that the generation > plugin always has to use objects and not primitives, but that would > break a lot of code and is not a nice API when null is never returned > for a getter when a default value is used. > > Another option is a case-by-case fix where the property can be made > "transient". Meaning that the set method does nothing and the get > method is always used. This is a partial fix, but is ugly and requires > that the component users always update the values behind the EL > manually based on events > > If EL allowed for different get & set that may work better: > > <t:dataTable first="#{get: bean.condition ? 0 : bean.firstRowIndex, > set: bean.firstRowIndex}" ... > > In this case the value expression would evaluate different expressions > for get vs. set. The problem here is that it is harder to write, > understand and would require an EL change that should be part of the > J2EE spec. which would be ugly. > > Another option is for the user to be able to specify which values > should be local and which should set via EL, but I don't see a clean > way of doing this without making an ugly API (way to many > attribute-metadata attributes). > > Before TOMAHAWK-858 is fixed, I would like to see a common approach > that we can take for all of the MyFaces projects so that it is easiest > for users to be familiar with. > > Opinions, solutions? > > Thanks, > Andrew > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces
