ICEfaces
  1. ICEfaces
  2. ICE-5946

Regression Icefaces2 - ICE-1545 toggle button not working

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-Beta2
    • Fix Version/s: 2.0-Beta2, 2.0.0
    • Component/s: ICE-Components
    • Labels:
      None
    • Environment:
      Server: Tomcat6
      Browser: FF3.6, IE7 & Opera10.10

      Description

      Icefaces2 revision#22029

      1)load application, row with ids 1 & 2 are displayed
      2)navigate to last page, rows with id 5 & 6 are displayed
      3)click on toggle button, nothing happens ( it should have displayed first 5 rows, from ids1 to 5)

      Test case available at "C:\repo\qa\trunk\Regression-Icefaces2\Nightly\ICE-1545".

        Activity

        Hide
        Deryk Sinotte added a comment -

        It appears that one of the changes to fix a previous regression has fixed this case as well. It now appears to work as described in the case.

        Show
        Deryk Sinotte added a comment - It appears that one of the changes to fix a previous regression has fixed this case as well. It now appears to work as described in the case.
        Hide
        Deryk Sinotte added a comment -

        Re-opening. I was running an incorrect version of ICEfaces during testing. I can see the problem now.

        Show
        Deryk Sinotte added a comment - Re-opening. I was running an incorrect version of ICEfaces during testing. I can see the problem now.
        Hide
        Deryk Sinotte added a comment -

        Assigning to the component team. I've investigated this regression fairly thoroughly and found that the issue is related to changes in the dataTable component, not the toggle button.

        The behaviour of the test case changes in revision 22019, which is the checkin which re-synchronized the components in compat with the current 1.8 trunk improvements. I then narrowed it down to changes in the HtmlDataTable component which effect row calculations - specifically logic that was removed to fix a different problem in another case (http://jira.icefaces.org/browse/ICE-5252):

        [deryk] icefaces > svn diff -r22018:22019 ./compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java
        Index: compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java
        ===================================================================
        — compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java (revision 22018)
        +++ compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java (revision 22019)
        @@ -18,6 +18,16 @@
        *

        • Contributor(s): _____________________.
          *
          + * Alternatively, the contents of this file may be used under the terms of
          + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"
          + * License), in which case the provisions of the LGPL License are
          + * applicable instead of those above. If you wish to allow use of your
          + * version of this file only under the terms of the LGPL License and not to
          + * allow others to use your version of this file under the MPL, indicate
          + * your decision by deleting the provisions above and replace them with
          + * the notice and other provisions required by the LGPL License. If you do
          + * not delete the provisions above, a recipient may use your version of
          + * this file under either the MPL or the LGPL License."
          */

        package com.icesoft.faces.component.ext;
        @@ -61,7 +71,7 @@
        private String headerClasses = null;
        private Boolean clientOnly = null;
        private Boolean scrollFooter = null;

        • private int oldRows = -1;
          +
          public HtmlDataTable() {
          super();
          setRendererType(RENDERER_TYPE);
          @@ -97,11 +107,6 @@

        public void encodeBegin(FacesContext context) throws IOException {
        super.encodeBegin(context);

        • int currentRows = getRows();
        • if (oldRows != -1 && oldRows != currentRows) { - setFirst(0); - }
        • oldRows = currentRows;
          }

        /**
        @@ -157,7 +162,7 @@

        • Object.</p>
          */
          public Object saveState(FacesContext context) { - Object values[] = new Object[16]; + Object values[] = new Object[15]; values[0] = super.saveState(context); values[1] = renderedOnUserRole; values[2] = columnWidths; @@ -173,7 +178,6 @@ values[12] = new Integer(resizableTblColumnsWidthIndex); values[13] = scrollable; values[14] = Boolean.valueOf(isResizableColumnWidthsSet); - values[15] = new Integer(oldRows); return ((Object) (values)); }

        @@ -198,7 +202,6 @@
        resizableTblColumnsWidthIndex = ((Integer) values[12]).intValue();
        scrollable = (Boolean) values[13];
        isResizableColumnWidthsSet = ((Boolean) values[14]).booleanValue();

        • oldRows = ((Integer) values[15]).intValue();
          }

        public String getComponentType() {

        If I run from the ICEfaces 2 trunk but just back out this change, the regression test passes. There's a chance that this regression also exists on the 1.8 trunk if we assume that the same changes were made and the same test is available so we should verify that.

        Show
        Deryk Sinotte added a comment - Assigning to the component team. I've investigated this regression fairly thoroughly and found that the issue is related to changes in the dataTable component, not the toggle button. The behaviour of the test case changes in revision 22019, which is the checkin which re-synchronized the components in compat with the current 1.8 trunk improvements. I then narrowed it down to changes in the HtmlDataTable component which effect row calculations - specifically logic that was removed to fix a different problem in another case ( http://jira.icefaces.org/browse/ICE-5252): [deryk] icefaces > svn diff -r22018:22019 ./compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java Index: compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java =================================================================== — compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java (revision 22018) +++ compat/components/src/main/java/com/icesoft/faces/component/ext/HtmlDataTable.java (revision 22019) @@ -18,6 +18,16 @@ * Contributor(s): _____________________. * + * Alternatively, the contents of this file may be used under the terms of + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL" + * License), in which case the provisions of the LGPL License are + * applicable instead of those above. If you wish to allow use of your + * version of this file only under the terms of the LGPL License and not to + * allow others to use your version of this file under the MPL, indicate + * your decision by deleting the provisions above and replace them with + * the notice and other provisions required by the LGPL License. If you do + * not delete the provisions above, a recipient may use your version of + * this file under either the MPL or the LGPL License." */ package com.icesoft.faces.component.ext; @@ -61,7 +71,7 @@ private String headerClasses = null; private Boolean clientOnly = null; private Boolean scrollFooter = null; private int oldRows = -1; + public HtmlDataTable() { super(); setRendererType(RENDERER_TYPE); @@ -97,11 +107,6 @@ public void encodeBegin(FacesContext context) throws IOException { super.encodeBegin(context); int currentRows = getRows(); if (oldRows != -1 && oldRows != currentRows) { - setFirst(0); - } oldRows = currentRows; } /** @@ -157,7 +162,7 @@ Object.</p> */ public Object saveState(FacesContext context) { - Object values[] = new Object[16]; + Object values[] = new Object[15]; values[0] = super.saveState(context); values[1] = renderedOnUserRole; values[2] = columnWidths; @@ -173,7 +178,6 @@ values[12] = new Integer(resizableTblColumnsWidthIndex); values[13] = scrollable; values[14] = Boolean.valueOf(isResizableColumnWidthsSet); - values[15] = new Integer(oldRows); return ((Object) (values)); } @@ -198,7 +202,6 @@ resizableTblColumnsWidthIndex = ((Integer) values [12] ).intValue(); scrollable = (Boolean) values [13] ; isResizableColumnWidthsSet = ((Boolean) values [14] ).booleanValue(); oldRows = ((Integer) values [15] ).intValue(); } public String getComponentType() { If I run from the ICEfaces 2 trunk but just back out this change, the regression test passes. There's a chance that this regression also exists on the 1.8 trunk if we assume that the same changes were made and the same test is available so we should verify that.
        Hide
        yip.ng added a comment - - edited

        There is actually nothing wrong with the toggle button or the table or the paginator. See screenshot 1 and video at http://screencast.com/t/NzI5OWRjNj.

        As can be seen from the screehshot and the video, the rows property has been changed properly from 2 to 5. It just so happens that the first property remains the same at 4, therefore the last page is still displayed with just 2 records.

        Before the fix for ICE-5252, the first property would always be reset to 0, therefore the first page would always be displayed. This is no longer true. Here is what Adnan said about the fix for ICE-5252:

        "To fix this bug, we have added the knowledge to the datapaginator to know when the dataTable.rows property gets changed dynamically and also calculate a new value for the dataTable.first property. "

        Therefore, the test case needs to be adjusted for this fact. One way is to click the paginator previous button after clicking the toggle button. Another way is to click the toggle button without going to the last page first. (The sole purpose of the test app. is to test changing the rows property. It is not testing the toggle button, nor the table, nor the paginator.)

        Show
        yip.ng added a comment - - edited There is actually nothing wrong with the toggle button or the table or the paginator. See screenshot 1 and video at http://screencast.com/t/NzI5OWRjNj . As can be seen from the screehshot and the video, the rows property has been changed properly from 2 to 5. It just so happens that the first property remains the same at 4, therefore the last page is still displayed with just 2 records. Before the fix for ICE-5252 , the first property would always be reset to 0, therefore the first page would always be displayed. This is no longer true. Here is what Adnan said about the fix for ICE-5252 : "To fix this bug, we have added the knowledge to the datapaginator to know when the dataTable.rows property gets changed dynamically and also calculate a new value for the dataTable.first property. " Therefore, the test case needs to be adjusted for this fact. One way is to click the paginator previous button after clicking the toggle button. Another way is to click the toggle button without going to the last page first. (The sole purpose of the test app. is to test changing the rows property. It is not testing the toggle button, nor the table, nor the paginator.)

          People

          • Assignee:
            yip.ng
            Reporter:
            Mandeep Hayher
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: