[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [p2-dev] WizardWithLicenses enables finish before accepting license

Title: Re: [p2-dev] WizardWithLicenses enables finish before accepting license
Hi Susan,

Thanks! :)

I do want to let you know that this code is being used in the DSDP/Mobile Tools for Java-Pulsar project for release with the Galileo train, so it would be nice if it could be fixed for 3.5 given that this is MTJ’s first release. :)

TIA.

--David


On 5/14/09 12:50 PM, "ext Susan Franklin McCourt" <susan_franklin@xxxxxxxxxx> wrote:

Hi, David.
I opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=276356 to document this issue.
I'll check the SDK and admin agent scenarios during the RC1 test pass.  If there is indeed a problem in the SDK where licenses could be skipped, it will get addressed right away.  Otherwise, it becomes a 3.6 issue.

I agree that the page creation/update buttons sequence is brittle/delicate.
The dynamic page calculation is a bit (ahem) ad hoc right now and this needs to be worked out before the wizards become API...

susan
<David.Dubrow@xxxxxxxxx>






To: <p2-dev@xxxxxxxxxxx>
cc:
Subject: Re: [p2-dev] WizardWithLicenses enables finish before accepting license

Hi Susan,

My use of p2 is very limited to IUs with only native touchpoints and it would be difficult for me to setup a test case to use the standard p2 UIs since our IUs are not recognized by them. In any case, I don’t think we’re calling the PreselectedIUInstallWizard in any way that is causing this problem. In fact, the problem seems like it’s in there lurking even if not visible in all types of WizardWithLicenses given when the license page is added (i.e., as a side effect of getNextPage()).

I was able to fix it by just adding an asyncExec calling getContainer().updateButtons() just after the license page is added. It seems that adding a wizard page that can potentially change the enablement of the finish button as a side effect of Wizard.getNextPage() which can be called during the enablement of buttons seems like a very brittle coding pattern and bound to cause problems in the long run. The reason that adding the asyncExec call to update the buttons again fixes it is that it causes the enablement to be reevaluated after the current operation during which it’s too late to get the state correct. Since the page is only added once, it is guaranteed not to cause an endless loop. It’s not the best solution but is in the same spirit as adding a page as a side effect of returning state information. ;)


On 5/14/09 9:39 AM, "ext Susan Franklin McCourt" <susan_franklin@xxxxxxxxxx
<susan_franklin@xxxxxxxxxx> > wrote: