Hi Leonardo, yes, certainly this needs to be doable as well. For performance reasons - is it really necessary to compare the full client-id? best regards, Martin On Fri, Apr 23, 2010 at 7:52 PM, Leonardo Uribe <lu4242(a)gmail.com&gt; wrote: > Hi > > Looking some jsf 2.0 code, I notice the current algorithm of > UIData.invokeOnComponent cannot handle the case when the target is a > UIColumn instance and no rowIndex is on clientId. Is that ok? > > Checking the new visit tree api, UIData.visitTree cleary says: > > "....If this component has children, for each UIColumn child: > > Call VisitContext.invokeVisitCallback(javax.faces.component.UIComponent, > javax.faces.component.visit.VisitCallback) on that UIColumn instance. If > such a call returns true, terminate visiting and return true from this > method. > > If the child UIColumn has facets, call > UIComponent.visitTree(javax.faces.component.visit.VisitContext, > javax.faces.component.visit.VisitCallback) on each one....." > > I think we should do something like this: > > for (Iterator<UIComponent> itChildren = > this.getChildren().iterator(); > !returnValue && itChildren.hasNext();) > { > UIComponent child = itChildren.next(); > if (child instanceof UIColumn && > clientId.equals(child.getClientId())) > { > try { > callback.invokeContextCallback(context, child); > } catch (Exception e) { > throw new FacesException(e); > } > returnValue = true; > } > // process the child's facets > for (Iterator<UIComponent> itChildFacets = > child.getFacets().values().iterator(); > !returnValue && itChildFacets.hasNext();) > { > //recursive call to find the component > returnValue = > itChildFacets.next().invokeOnComponent(context, clientId, callback); > } > } > > Does that sounds reasonable? Should be fixed on 1.2.x branch too? > > regards, > > Leonardo Uribe > > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces

Hi Leonardo, yes, ok - it is the same as the existing implementation, you are right - but still, I think we would only need to compare the part of the client-id which has not already been checked cause we have moved down the component tree already. But I see that this performance enhancement is not in the existing code, so forget my objection (or implement it and make it a little faster ;) Back to your original issue: I definitely agree. best regards, Martin On Mon, Apr 26, 2010 at 7:31 AM, Leonardo Uribe <lu4242(a)gmail.com&gt; wrote: > Hi Martin > > Yes, it is necessary to compare the full client id. Really that > part of the algorithm is the same as UIComponent.invokeOnComponent, > but without propagate the callback to the children. > > regards, > > Leonardo > > 2010/4/25 Martin Marinschek <mmarinschek(a)apache.org&gt; >> >> Hi Leonardo, >> >> yes, certainly this needs to be doable as well. For performance >> reasons - is it really necessary to compare the full client-id? >> >> best regards, >> >> Martin >> >> On Fri, Apr 23, 2010 at 7:52 PM, Leonardo Uribe <lu4242(a)gmail.com&gt; wrote: >> > Hi >> > >> > Looking some jsf 2.0 code, I notice the current algorithm of >> > UIData.invokeOnComponent cannot handle the case when the target is a >> > UIColumn instance and no rowIndex is on clientId. Is that ok? >> > >> > Checking the new visit tree api, UIData.visitTree cleary says: >> > >> > "....If this component has children, for each UIColumn child: >> > >> > Call VisitContext.invokeVisitCallback(javax.faces.component.UIComponent, >> > javax.faces.component.visit.VisitCallback) on that UIColumn instance. If >> > such a call returns true, terminate visiting and return true from this >> > method. >> > >> > If the child UIColumn has facets, call >> > UIComponent.visitTree(javax.faces.component.visit.VisitContext, >> > javax.faces.component.visit.VisitCallback) on each one....." >> > >> > I think we should do something like this: >> > >> > for (Iterator<UIComponent> itChildren = >> > this.getChildren().iterator(); >> > !returnValue && itChildren.hasNext();) >> > { >> > UIComponent child = itChildren.next(); >> > if (child instanceof UIColumn && >> > clientId.equals(child.getClientId())) >> > { >> > try { >> > callback.invokeContextCallback(context, >> > child); >> > } catch (Exception e) { >> > throw new FacesException(e); >> > } >> > returnValue = true; >> > } >> > // process the child's facets >> > for (Iterator<UIComponent> itChildFacets = >> > child.getFacets().values().iterator(); >> > !returnValue && itChildFacets.hasNext();) >> > { >> > //recursive call to find the component >> > returnValue = >> > itChildFacets.next().invokeOnComponent(context, clientId, callback); >> > } >> > } >> > >> > Does that sounds reasonable? Should be fixed on 1.2.x branch too? >> > >> > regards, >> > >> > Leonardo Uribe >> > >> > >> >> >> >> -- >> >> http://www.irian.at >> >> Your JSF powerhouse - >> JSF Consulting, Development and >> Courses in English and German >> >> Professional Support for Apache MyFaces > > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces

Hi Leonardo, IMHO, you can commit it on MyFaces - cause this is really a defect-fix, not an incomplicant feature. @Ed: can you please comment on the spec related change necessary? best regards, Martin On 4/28/10, Leonardo Uribe <lu4242(a)gmail.com&gt; wrote: > Hi > > Should I create an issue on spec issue tracker for this one? > > Any chance we can fix it for 1.2? > > Can I commit this fix on myfaces or I have to wait? > > regards, > > Leonardo Uribe > > 2010/4/26 Martin Marinschek <mmarinschek(a)apache.org&gt; > >> Hi Leonardo, >> >> yes, ok - it is the same as the existing implementation, you are right >> - but still, I think we would only need to compare the part of the >> client-id which has not already been checked cause we have moved down >> the component tree already. But I see that this performance >> enhancement is not in the existing code, so forget my objection (or >> implement it and make it a little faster ;) >> >> Back to your original issue: I definitely agree. >> >> best regards, >> >> Martin >> >> On Mon, Apr 26, 2010 at 7:31 AM, Leonardo Uribe <lu4242(a)gmail.com&gt; wrote: >> > Hi Martin >> > >> > Yes, it is necessary to compare the full client id. Really that >> > part of the algorithm is the same as UIComponent.invokeOnComponent, >> > but without propagate the callback to the children. >> > >> > regards, >> > >> > Leonardo >> > >> > 2010/4/25 Martin Marinschek <mmarinschek(a)apache.org&gt; >> >> >> >> Hi Leonardo, >> >> >> >> yes, certainly this needs to be doable as well. For performance >> >> reasons - is it really necessary to compare the full client-id? >> >> >> >> best regards, >> >> >> >> Martin >> >> >> >> On Fri, Apr 23, 2010 at 7:52 PM, Leonardo Uribe <lu4242(a)gmail.com&gt; >> wrote: >> >> > Hi >> >> > >> >> > Looking some jsf 2.0 code, I notice the current algorithm of >> >> > UIData.invokeOnComponent cannot handle the case when the target is a >> >> > UIColumn instance and no rowIndex is on clientId. Is that ok? >> >> > >> >> > Checking the new visit tree api, UIData.visitTree cleary says: >> >> > >> >> > "....If this component has children, for each UIColumn child: >> >> > >> >> > Call >> VisitContext.invokeVisitCallback(javax.faces.component.UIComponent, >> >> > javax.faces.component.visit.VisitCallback) on that UIColumn instance. >> If >> >> > such a call returns true, terminate visiting and return true from >> >> > this >> >> > method. >> >> > >> >> > If the child UIColumn has facets, call >> >> > UIComponent.visitTree(javax.faces.component.visit.VisitContext, >> >> > javax.faces.component.visit.VisitCallback) on each one....." >> >> > >> >> > I think we should do something like this: >> >> > >> >> > for (Iterator<UIComponent> itChildren = >> >> > this.getChildren().iterator(); >> >> > !returnValue && itChildren.hasNext();) >> >> > { >> >> > UIComponent child = itChildren.next(); >> >> > if (child instanceof UIColumn && >> >> > clientId.equals(child.getClientId())) >> >> > { >> >> > try { >> >> > callback.invokeContextCallback(context, >> >> > child); >> >> > } catch (Exception e) { >> >> > throw new FacesException(e); >> >> > } >> >> > returnValue = true; >> >> > } >> >> > // process the child's facets >> >> > for (Iterator<UIComponent> itChildFacets = >> >> > child.getFacets().values().iterator(); >> >> > !returnValue && itChildFacets.hasNext();) >> >> > { >> >> > //recursive call to find the component >> >> > returnValue = >> >> > itChildFacets.next().invokeOnComponent(context, clientId, callback); >> >> > } >> >> > } >> >> > >> >> > Does that sounds reasonable? Should be fixed on 1.2.x branch too? >> >> > >> >> > regards, >> >> > >> >> > Leonardo Uribe >> >> > >> >> > >> >> >> >> >> >> >> >> -- >> >> >> >> http://www.irian.at >> >> >> >> Your JSF powerhouse - >> >> JSF Consulting, Development and >> >> Courses in English and German >> >> >> >> Professional Support for Apache MyFaces >> > >> > >> >> >> >> -- >> >> http://www.irian.at >> >> Your JSF powerhouse - >> JSF Consulting, Development and >> Courses in English and German >> >> Professional Support for Apache MyFaces >> > -- http://www.irian.at Your JSF powerhouse - JSF Consulting, Development and Courses in English and German Professional Support for Apache MyFaces