You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by simon <si...@chello.at> on 2008/01/26 09:51:46 UTC

Re: svn commit: r615200 - /myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java

I haven't analysed this in depth, but this patch doesn't look like a
good idea to me.

Everywhere else in renderers we create components via the normal
factories.

I'm concerned that this will fix one particular user's issue, and break
many other situations.

Regards,
Simon

On Fri, 2008-01-25 at 12:20 +0000, tomsp@apache.org wrote:
> Author: tomsp
> Date: Fri Jan 25 04:20:00 2008
> New Revision: 615200
> 
> URL: http://svn.apache.org/viewvc?rev=615200&view=rev
> Log:
> TOMAHAWK-117 resolved, applied patch from Martin Haimberger
> 
> Modified:
>     myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java
> 
> Modified: myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java
> URL: http://svn.apache.org/viewvc/myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java?rev=615200&r1=615199&r2=615200&view=diff
> ==============================================================================
> --- myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java (original)
> +++ myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java Fri Jan 25 04:20:00 2008
> @@ -344,7 +344,7 @@
>          if(ondblclick != null){
>          	link.setOndblclick(ondblclick);
>          }
> -    	
> +
>          link.encodeBegin(facesContext);
>          facetComp.encodeBegin(facesContext);
>          if (!facetComp.getRendersChildren())
> @@ -412,7 +412,7 @@
>  
>          String onclick = scroller.getOnclick();
>     	    String ondblclick = scroller.getOndblclick();
> -        
> +
>          for (int i = start, size = start + pages; i < size; i++)
>          {
>              int idx = i + 1;
> @@ -495,10 +495,13 @@
>                                        String text, int pageIndex)
>      {
>          String id = HtmlDataScrollerRenderer.PAGE_NAVIGATION + Integer.toString(pageIndex);
> +
>          Application application = facesContext.getApplication();
>  
> -        HtmlCommandLink link = (HtmlCommandLink) application
> -                        .createComponent(HtmlCommandLink.COMPONENT_TYPE);
> +	    // See Jira Issue TOMAHAWK-117 http://issues.apache.org/jira/browse/TOMAHAWK-117
> +        //     and http://issues.apache.org/jira/browse/MYFACES-1809
> +        HtmlCommandLink link = new org.apache.myfaces.component.html.ext.HtmlCommandLink();
> +
>          link.setId(scroller.getId() + id);
>          link.setTransient(true);
>          UIParameter parameter = (UIParameter) application
> @@ -526,8 +529,10 @@
>      {
>          Application application = facesContext.getApplication();
>  
> -        HtmlCommandLink link = (HtmlCommandLink) application
> -                        .createComponent(HtmlCommandLink.COMPONENT_TYPE);
> +	    // See Jira Issue TOMAHAWK-117 http://issues.apache.org/jira/browse/TOMAHAWK-117
> +        //     and http://issues.apache.org/jira/browse/MYFACES-1809
> +        HtmlCommandLink link = new org.apache.myfaces.component.html.ext.HtmlCommandLink();
> +
>          link.setId(scroller.getId() + facetName);
>          link.setTransient(true);
>          UIParameter parameter = (UIParameter) application
> 
> 


Re: svn commit: r615200 - /myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java

Posted by Thomas Spiegl <th...@gmail.com>.
The scroller implementation assumes that a commandLink renders it's
children nested between the link tag. Trinidad's commandLink for
example deviates from this behaviour & renders the <a> start tag
within encodeEnd of the renderer. Any other component library might do
the same.
Indeed it's not nice to create the commandLink without using the
factory. A better solution might be to check the commandLink
implementation to render the link's children correctly.

Regards,
Thomas

On Jan 26, 2008 9:51 AM, simon <si...@chello.at> wrote:
> I haven't analysed this in depth, but this patch doesn't look like a
> good idea to me.
>
> Everywhere else in renderers we create components via the normal
> factories.
>
> I'm concerned that this will fix one particular user's issue, and break
> many other situations.
>
> Regards,
> Simon
>
>
> On Fri, 2008-01-25 at 12:20 +0000, tomsp@apache.org wrote:
> > Author: tomsp
> > Date: Fri Jan 25 04:20:00 2008
> > New Revision: 615200
> >
> > URL: http://svn.apache.org/viewvc?rev=615200&view=rev
> > Log:
> > TOMAHAWK-117 resolved, applied patch from Martin Haimberger
> >
> > Modified:
> >     myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java
> >
> > Modified: myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java
> > URL: http://svn.apache.org/viewvc/myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java?rev=615200&r1=615199&r2=615200&view=diff
> > ==============================================================================
> > --- myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java (original)
> > +++ myfaces/tomahawk/trunk/core/src/main/java/org/apache/myfaces/custom/datascroller/HtmlDataScrollerRenderer.java Fri Jan 25 04:20:00 2008
> > @@ -344,7 +344,7 @@
> >          if(ondblclick != null){
> >               link.setOndblclick(ondblclick);
> >          }
> > -
> > +
> >          link.encodeBegin(facesContext);
> >          facetComp.encodeBegin(facesContext);
> >          if (!facetComp.getRendersChildren())
> > @@ -412,7 +412,7 @@
> >
> >          String onclick = scroller.getOnclick();
> >           String ondblclick = scroller.getOndblclick();
> > -
> > +
> >          for (int i = start, size = start + pages; i < size; i++)
> >          {
> >              int idx = i + 1;
> > @@ -495,10 +495,13 @@
> >                                        String text, int pageIndex)
> >      {
> >          String id = HtmlDataScrollerRenderer.PAGE_NAVIGATION + Integer.toString(pageIndex);
> > +
> >          Application application = facesContext.getApplication();
> >
> > -        HtmlCommandLink link = (HtmlCommandLink) application
> > -                        .createComponent(HtmlCommandLink.COMPONENT_TYPE);
> > +         // See Jira Issue TOMAHAWK-117 http://issues.apache.org/jira/browse/TOMAHAWK-117
> > +        //     and http://issues.apache.org/jira/browse/MYFACES-1809
> > +        HtmlCommandLink link = new org.apache.myfaces.component.html.ext.HtmlCommandLink();
> > +
> >          link.setId(scroller.getId() + id);
> >          link.setTransient(true);
> >          UIParameter parameter = (UIParameter) application
> > @@ -526,8 +529,10 @@
> >      {
> >          Application application = facesContext.getApplication();
> >
> > -        HtmlCommandLink link = (HtmlCommandLink) application
> > -                        .createComponent(HtmlCommandLink.COMPONENT_TYPE);
> > +         // See Jira Issue TOMAHAWK-117 http://issues.apache.org/jira/browse/TOMAHAWK-117
> > +        //     and http://issues.apache.org/jira/browse/MYFACES-1809
> > +        HtmlCommandLink link = new org.apache.myfaces.component.html.ext.HtmlCommandLink();
> > +
> >          link.setId(scroller.getId() + facetName);
> >          link.setTransient(true);
> >          UIParameter parameter = (UIParameter) application
> >
> >
>
>



-- 
http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces