izpack/izpack — pull request #187 by mtjandra (2014-03-27 14:13:57+00:00)
Title: [IZPACK-1074]

This is to address http://jira.codehaus.org/browse/IZPACK-1074

1. Null values of a comboBox or radioButton can be set in the installer.
If the associated variable for the comboBox or radioButton has not been defined, or if the choice fields do not contain the "set" attribute. This is a problem as it can produce unexpected NPE during the installation process.

To reproduce just add a comboBox or radioButton without defining a "set" attribute and without setting its variable in the tags of the install.xml

Solution: In the ComboFieldReader and RadioFieldReader set the "selected" variable to 0 rather than -1

2. During console installation the radioButton field does not remember what the user has selected. If radioButton has default value of A you have choices A,B,C and choose B, then at the end of the panel hit rediplay, you will see that the radioButton is still marked as A instead of B. Problem lies in the RadioFieldReader.java file, where it does not compute the correctly selected radio button.

To reproduce follow the workflow as described above.

Solution: Compute the selected radio like how the selected comboBox is computed in the ComboFieldReader

3. Notice high similarity between ComboField and RadioField, code duplication can lead to inconsistent changes towards choice fields.

Solution: Create a SimpleChoiceReader (Similar to the idea of the SimpleFileReader) to abstract common features between the ComboFieldReader and RadioFieldReader. Later completely generalize and remove the ComboFieldReader and RadioFieldReader so that choices use the SimpleChoiceReader.

4. Notice that the RadioChoice IS a Choice but with the additional feature of revalidation. Swapping between choices and with the notion of revalidation it should be intuitive that you may also want to revalidate a panel based on the selection of a comboBox.

Solution: Remove the radioChoice and just add the revalidation features to the Choice. Now we can think of comboBoxes and radioButtons consisting of "choices" rather than two seperate types of choices.

5. Currently I believe its intuitive to developers and users that their applications are reactive and should respond immediately. So I would suggest that the revalidation feature not be an option but rather just part of the installer and on by default. Having the revalidation on does not hurt users that do not require it, and helps other produce less bug prone installers. (This can always be reverted if you don't agree)

Solution: Add the actionListeners to the components by default

6. If the a checkboxes variable isn't set, set it to the initialValue. This will ensure that if some condition depends on a checkbox, and a checkbox is "set" to true, that any component that depends on this condition will be shown when on the same panel. Previously a component wasn't shown if the above situation was true.

To reproduce "set" checkbox to true.
Have a component on the same panel of the checkbox with a conditionID which is based on the checkbox being true.
Notice the component is not shown.

Solution: Set the variable when checkbox is decided to set itself as selected or not.