ICEfaces
  1. ICEfaces
  2. ICE-3154

Clean up improper 'encodeChildren' implementations in a few components

          Details

          • Type: Bug Bug
          • Status: Closed
          • Priority: Minor Minor
          • Resolution: Fixed
          • Affects Version/s: 1.7
          • Fix Version/s: 1.7.1
          • Component/s: ICE-Components
          • Labels:
            None
          • Environment:
            All

            Description

            Because of ICE-3147, I did a little impromptu audit of encodeChildren() methods in our components. The following classes use non-standard implementations of encodeChildren(-), which may cause problems:

            com.icesoft.faces.component.gmap.GMapLatLngs
            com.icesoft.faces.component.gmap.GMapMarker
            com.icesoft.faces.component.gmap.GMapRenderer
            com.icesoft.faces.component.menubar.MenuItemRenderer
               Lines 658, 704 :: Link rendering. The way we hard-code the links, it
            should be ok
            com.icesoft.faces.component.menubar.MenuItemRendererBase
               Omits check for rendered that MenuItemsRenderer does
            com.icesoft.faces.component.panellayout.PanelLayoutRenderer

              Activity

              Ascending order - Click to sort in descending order
              Mark Collette created issue -
              Hide
              Permalink
              Mark Collette added a comment -

              I should check the Canvas code too, since I found:

              canvas\src\com\icesoft\faces\canvas\Canvas.java
              Method: render(UIComponent)

              Show
              Mark Collette added a comment - I should check the Canvas code too, since I found: canvas\src\com\icesoft\faces\canvas\Canvas.java Method: render(UIComponent)
              Hide
              Permalink
              Mark Collette added a comment -

              Canvas.encodeChildren(FacesContext)
              DataPaginatorRenderer.renderFacet(FacesContext, DataPaginator, UIComponent, String)

              Show
              Mark Collette added a comment - Canvas.encodeChildren(FacesContext) DataPaginatorRenderer.renderFacet(FacesContext, DataPaginator, UIComponent, String)
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16837 Mon Jun 09 16:31:07 MDT 2008 adnan.durrani Fix for ICE-3154 (Clean up improper 'encodeChildren' implementations in a few components)
              Files Changed
              Commit graph MODIFY /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/gmap/GMapMarker.java
              Commit graph MODIFY /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/gmap/GMapLatLngs.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16838 Mon Jun 09 16:31:52 MDT 2008 adnan.durrani Fix for ICE-3154 (Clean up improper 'encodeChildren' implementations in a few components)
              Files Changed
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/gmap/GMapMarker.java
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/gmap/GMapLatLngs.java
              Ken Fyten made changes -
              Field Original Value New Value
              Summary Improper encodeChildren Clean up improper 'encodeChildren' implementations in a few components
              Fix Version/s 1.7.1 [ 10122 ]
              Security Private [ 10001 ]
              Priority Major [ 3 ] Minor [ 4 ]
              Ken Fyten made changes -
              Affects Version/s 1.7 [ 10080 ]
              Affects Version/s 1.7.1 [ 10122 ]
              Hide
              Permalink
              Adnan Durrani added a comment -

              trunk : revision 16837
              branch 1.7: 16838

              Changes made to the both GmapLatLngs and GMapMarker.
              It doesn't apply on GMapRenderer as it takes only designated children and it maps the javascript visual elements with the JSF component, so even on render=false on any of its children, GMapRenderer delegates the encoding to it children, where it children decides to send remove command for appropriate visual element to the GMap library.

              Show
              Adnan Durrani added a comment - trunk : revision 16837 branch 1.7: 16838 Changes made to the both GmapLatLngs and GMapMarker. It doesn't apply on GMapRenderer as it takes only designated children and it maps the javascript visual elements with the JSF component, so even on render=false on any of its children, GMapRenderer delegates the encoding to it children, where it children decides to send remove command for appropriate visual element to the GMap library.
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16841 Mon Jun 09 17:51:41 MDT 2008 yip.ng ICE-3154
              Fixed potential logic flaws in encodeChildren() method.
              Files Changed
              Commit graph MODIFY /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/menubar/MenuItemRenderer.java
              Commit graph MODIFY /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/panellayout/PanelLayoutRenderer.java
              Commit graph MODIFY /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/menubar/MenuItemRendererBase.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16842 Mon Jun 09 17:53:39 MDT 2008 yip.ng ICE-3154
              Fixed potential logic flaws in encodeChildren() method.
              Files Changed
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/panellayout/PanelLayoutRenderer.java
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/menubar/MenuItemRenderer.java
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/menubar/MenuItemRendererBase.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16844 Mon Jun 09 18:15:23 MDT 2008 adnan.durrani changes made to fix ICE-3154, has been reverted. Don't apply on it
              Files Changed
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/gmap/GMapMarker.java
              Hide
              Permalink
              yip.ng added a comment -

              Fixed com.icesoft.faces.component.menubar.MenuItemRenderer, com.icesoft.faces.component.menubar.MenuItemRendererBase and com.icesoft.faces.component.panellayout.PanelLayoutRenderer as suggested by Mark.

              Show
              yip.ng added a comment - Fixed com.icesoft.faces.component.menubar.MenuItemRenderer, com.icesoft.faces.component.menubar.MenuItemRendererBase and com.icesoft.faces.component.panellayout.PanelLayoutRenderer as suggested by Mark.
              Hide
              Permalink
              Adnan Durrani added a comment -

              changes made to the GMapMarker also reverted as the encodeBegin of it using the renderer flag to send remove command to the google map library.

              Show
              Adnan Durrani added a comment - changes made to the GMapMarker also reverted as the encodeBegin of it using the renderer flag to send remove command to the google map library.
              Hide
              Permalink
              Mark Collette added a comment -

              Here's an example of a test case for panelLayout:

              <ice:panelLayout layout="flow">
              <ice:outputText value="This text should display"/>
              <br/>
              <ice:outputLink value="http://www.icefaces.org" title="There should be an image in here">
              <ice:graphicImage alt="Home" url="http://www.icefaces.org/main/css/images/tools_home.gif"/>
              </ice:outputLink>
              </ice:panelLayout>

              Before Yip's fix, the image would not show up, and afterwards, it now should.

              Show
              Mark Collette added a comment - Here's an example of a test case for panelLayout: <ice:panelLayout layout="flow"> <ice:outputText value="This text should display"/> <br/> <ice:outputLink value="http://www.icefaces.org" title="There should be an image in here"> <ice:graphicImage alt="Home" url="http://www.icefaces.org/main/css/images/tools_home.gif"/> </ice:outputLink> </ice:panelLayout> Before Yip's fix, the image would not show up, and afterwards, it now should.
              Ken Fyten made changes -
              Status Open [ 1 ] Resolved [ 5 ]
              Resolution Fixed [ 1 ]
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16849 Tue Jun 10 16:35:09 MDT 2008 mark.collette ICE-3154 : Clean up improper 'encodeChildren' implementations in a few components
              Files Changed
              Commit graph MODIFY /icefaces/trunk/icefaces/component/src/com/icesoft/faces/component/datapaginator/DataPaginatorRenderer.java
              Repository Revision Date User Message
              ICEsoft Public SVN Repository #16850 Tue Jun 10 16:35:38 MDT 2008 mark.collette ICE-3154 : Clean up improper 'encodeChildren' implementations in a few components
              Files Changed
              Commit graph MODIFY /icefaces/branches/icefaces-1.7/icefaces/component/src/com/icesoft/faces/component/datapaginator/DataPaginatorRenderer.java
              Hide
              Permalink
              Mark Collette added a comment -

              TRUNK
              Subversion 16849
              icefaces\component\src\com\icesoft\faces\component\datapaginator\DataPaginatorRenderer.java

              ICEfaces 1.7 branch
              Subversion 16850
              icefaces\component\src\com\icesoft\faces\component\datapaginator\DataPaginatorRenderer.java

              Show
              Mark Collette added a comment - TRUNK Subversion 16849 icefaces\component\src\com\icesoft\faces\component\datapaginator\DataPaginatorRenderer.java ICEfaces 1.7 branch Subversion 16850 icefaces\component\src\com\icesoft\faces\component\datapaginator\DataPaginatorRenderer.java
              Hide
              Permalink
              Ken Fyten added a comment -

              Changes caused regression issue: ICE-3174.

              Show
              Ken Fyten added a comment - Changes caused regression issue: ICE-3174.
              Ken Fyten made changes -
              Resolution Fixed [ 1 ]
              Status Resolved [ 5 ] Reopened [ 4 ]
              Assignee Priority P1
              Assignee Yip Ng [ yip.ng ]
              Hide
              Permalink
              Ken Fyten added a comment -

              False alarm.

              Show
              Ken Fyten added a comment - False alarm.
              Ken Fyten made changes -
              Status Reopened [ 4 ] Resolved [ 5 ]
              Assignee Priority P1
              Resolution Fixed [ 1 ]
              Assignee Yip Ng [ yip.ng ]
              Ken Fyten made changes -
              Status Resolved [ 5 ] Closed [ 6 ]

                People

                • Assignee:
                  Unassigned
                  Reporter:
                  Mark Collette
                • Votes:
                  0 Vote for this issue
                  Watchers:
                  0 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved: