History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: IZPACK-28
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Julien Ponge
Reporter: Brett Bergquist
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
IzPack

Logic to show the next panel can cause the current panel's isValidated method to be invoked more than once

Created: 11/Mar/08 08:38 PM   Updated: 23/Apr/08 02:02 PM
Component/s: Installer
Affects Version/s: 3.11.0
Fix Version/s: 4.0.0

Time Tracking:
Not Specified

File Attachments: 1. Text File IZPACK-28.patch (6 kb)
2. Text File IZPACK-28.patch (4 kb)

Environment: WinXP Pro


 Description  « Hide
If you have conditional panels (panels with conditions attached) and those panels are not going to be displayed, then the installer may invoke the isValidated method of the current panel multiple times. With a panel like TargetPanel followed by conditional panels, the popup that informs the user that the target directory will be created is shown multiple times. The logic in "navigateNext" method uses recursion is "canShow" returns 'false". Recursion is not really needed here. What is needed is to find out if there is a "next" panel that can be displayed depending on the current conditions for the successive panels.

In my case, I have a panel is conditional depending on the OS of the platform being installed on. These panels are after the TargetPanel, and when the first panel after the target panel is going to be skipped, the warning that the TargetPanel pops up is displayed twice.

In looking over the code to provide a patch, I see that "IzPanel" has a "hidden" property and that this property is used in "visiblePanelMapping" to figure out the panel steps and also to see if a panel is going to be visible. I cannot find any code that that ever calls "IzPanel.setHidden". This also seems a little flawed in that you can have panels displayed depending on the conditions that are associated with the panel. These conditions can be dynamic based on things that occurred earlier in the installer steps, so I don't see how this will ever be 100% accurate.

I have a patch that I am testing for the "navigateNext" that will not cause the "isValidate" to be called multiple times, but it will not fix the panel steps problem.



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Brett Bergquist - 11/Mar/08 09:40 PM
This patch changes adds two new methods "hasNavigateNext" and "hasNavigatePrevious" which when called with the current panel number return the next or previous panel number that can be shown with the current conditions in effect or -1 if there is no next or previous panel that can be show. The current "navigateNext" and "navigatePrevious" are changed from their recursive form to call these new methods.

Note that this patch is not against the latest version in the trunk as that has changed a bit since I pulled out the code.


Brett Bergquist - 14/Mar/08 08:40 AM
The previous patch file that I attached had a "off by 1 bug" in that you were not able to navigate to the last panel. This patch is the correct patch to apply.

Julien Ponge - 16/Mar/08 10:25 AM
Brett,

I could not apply your patch to the current trunk revision as there are some seriously conflicting changes.

Could you please check against http://svn.codehaus.org/izpack/izpack-src/trunk/ ?

Thank you very much!


Brett Bergquist - 20/Apr/08 12:00 PM
New patch against the latest trunk. This patch does away with the recursion to find the next or previous panel that can be navigated to and also takes into consideration of hidden (not visible) panels. It adds a couple of new methods (hasNavigateNext, hasNavigatePrevious) that are used as utility methods to determine if there is a next or previous panel to navigate to depending on the current conditions and panel visibility.

Julien Ponge - 20/Apr/08 12:17 PM
Works like a charm, thanks a lot for your work!

Julien Ponge - 23/Apr/08 02:02 PM
The issue is confirmed to be closed.