ICEfaces
  1. ICEfaces
  2. ICE-6884

When ACE component in iterative container, setting ace property during render affects all rows

          Details

          • Type: Bug Bug
          • Status: Closed
          • Priority: Major Major
          • Resolution: Fixed
          • Affects Version/s: 2.0.2
          • Fix Version/s: 3.0.RC1, 3.0
          • Component/s: ACE-Components
          • Labels:
            None
          • Environment:
            ACE
          • Assignee Priority:
            P2

            Description

            The ACE generator create special getter/setter methods for properties so that if a property is not bound via EL to a bean, it can still maintain distinct values for each row that the component is in. But if the component, or it's renderer, or app code via a component binding, call a property setter method during render, then that value will apply to all rows, since the generated setter method cannot differentiate between construction time during render phase, and rendering during render phase, and the code path treats the set as being for construction, so that it is applied to all rows.

              Issue Links

              blocks

              Improvement - An improvement or enhancement to an existing feature or task. ICE-6728 Add client-side cache support for ace:tabPane within a 'clientSide=false'

              • Major - Major loss of function.
              • Closed

                Activity

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

                Should investigate if the new isDisconnected() code could replace the phase specific checks.

                Show
                Mark Collette added a comment - Should investigate if the new isDisconnected() code could replace the phase specific checks.
                Hide
                Permalink
                Mark Collette added a comment -

                ICE-7321 inspired an idea that we could take what we added there, for detecting that a new MenuItem was having its properties set in bean code and know to set them in the section that applies to every row, and use that solely for .xhtml view construction, instead of using the phase heuristics.

                To test this, I added an ACE checkboxButton to a dataTable, and modified the CheckboxButtonRenderer so that when the value was true, it would could setStyle() on the component, during rendering. I was able to duplicate the error, where the CheckboxButtonBase.setStyle() code would set the new style value into the place for every row instead of just for the current row, because of the simplicity of the check for being in RenderView phase, as the heuristic for component tree construction. I then debugged what the value of isDisconnected() was, and found that in view construction for both RestoreView and RenderView, it was true, but was false when simply rendering and hitting my set of the style. Perfect. So, after removing the phase code, and fully relying on isDisconnected(), I verified that my test set of the style only went to the row specific part, and my updates after clicking the checkboxButton were now localised to just that one row, since it was no longer modifying the value for all rows.

                Development branch
                Subversion 26338
                icefaces2/ace/generator/src/org/icefaces/ace/generator/artifacts/ComponentArtifact.java

                Show
                Mark Collette added a comment - ICE-7321 inspired an idea that we could take what we added there, for detecting that a new MenuItem was having its properties set in bean code and know to set them in the section that applies to every row, and use that solely for .xhtml view construction, instead of using the phase heuristics. To test this, I added an ACE checkboxButton to a dataTable, and modified the CheckboxButtonRenderer so that when the value was true, it would could setStyle() on the component, during rendering. I was able to duplicate the error, where the CheckboxButtonBase.setStyle() code would set the new style value into the place for every row instead of just for the current row, because of the simplicity of the check for being in RenderView phase, as the heuristic for component tree construction. I then debugged what the value of isDisconnected() was, and found that in view construction for both RestoreView and RenderView, it was true, but was false when simply rendering and hitting my set of the style. Perfect. So, after removing the phase code, and fully relying on isDisconnected(), I verified that my test set of the style only went to the row specific part, and my updates after clicking the checkboxButton were now localised to just that one row, since it was no longer modifying the value for all rows. Development branch Subversion 26338 icefaces2/ace/generator/src/org/icefaces/ace/generator/artifacts/ComponentArtifact.java
                Hide
                Permalink
                Mark Collette added a comment -

                For testing, had this in ComponentArtifact, where it sets a property:

                writer.append("\n\t\t\tPhaseId pi = getFacesContext().getCurrentPhaseId();");
                writer.append("\n\t\t\tStateHelper sh = getStateHelper(); ");
                writer.append("\n\t\t\tif (isDisconnected(this)) {");
                writer.append("\nSystem.out.println(\"comp: \" + getClientId() + \" setter: "field.getName()" value: \" + "field.getName()" + \" phase: \" + pi + \" disconnected: \" + isDisconnected(this));");

                Had this in CheckBoxRenderer.encodeBegin():
                Object val = checkbox.getValue();
                if (val != null && (val instanceof Boolean) && ((Boolean)val).booleanValue())

                { System.out.println("CheckBoxButton true for: " + checkbox.getClientId(facesContext)); checkbox.setStyle("background-color: red;"); }

                Had this in comp-suite:

                src/main/webapp/resources/examples/compat/dataTable/dataTable.xhtml
                <ice:column>
                <f:facet name="header">
                ACE Button
                </f:facet>
                <ace:checkboxButton id="mark_cb" label="Mark CB" value="#

                {car.markBoolean}

                " singleSubmit="true"/>
                </ice:column>

                src/main/java/org/icefaces/samples/showcase/example/compat/dataTable/Car.java
                protected boolean markBoolean;
                public boolean isMarkBoolean()

                { return markBoolean; }

                public void setMarkBoolean(boolean mb)

                { this.markBoolean = mb; System.out.println("setMarkBoolean: " + mb); }
                Show
                Mark Collette added a comment - For testing, had this in ComponentArtifact, where it sets a property: writer.append("\n\t\t\tPhaseId pi = getFacesContext().getCurrentPhaseId();"); writer.append("\n\t\t\tStateHelper sh = getStateHelper(); "); writer.append("\n\t\t\tif (isDisconnected(this)) {"); writer.append("\nSystem.out.println(\"comp: \" + getClientId() + \" setter: " field.getName() " value: \" + " field.getName() " + \" phase: \" + pi + \" disconnected: \" + isDisconnected(this));"); Had this in CheckBoxRenderer.encodeBegin(): Object val = checkbox.getValue(); if (val != null && (val instanceof Boolean) && ((Boolean)val).booleanValue()) { System.out.println("CheckBoxButton true for: " + checkbox.getClientId(facesContext)); checkbox.setStyle("background-color: red;"); } Had this in comp-suite: src/main/webapp/resources/examples/compat/dataTable/dataTable.xhtml <ice:column> <f:facet name="header"> ACE Button </f:facet> <ace:checkboxButton id="mark_cb" label="Mark CB" value="# {car.markBoolean} " singleSubmit="true"/> </ice:column> src/main/java/org/icefaces/samples/showcase/example/compat/dataTable/Car.java protected boolean markBoolean; public boolean isMarkBoolean() { return markBoolean; } public void setMarkBoolean(boolean mb) { this.markBoolean = mb; System.out.println("setMarkBoolean: " + mb); }

                  People

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

                    Dates

                    • Created:
                      Updated:
                      Resolved: