Community
Participate
Working Groups
Build id: I20090325-1135 1. File > New > Other... 2. See that the 'Finish' button is at the end of the button bar. 3. Window > Preferences > Java > Installed JREs 4. Click the 'Add...' button. 5. See that the 'Finish' button is not the last button. 6. Close the dialog. 7. Select an existing JRE. 8. Click 'Edit...'. 9. See that the 'Finish' button is at the end of the button bar. I snooped around the code but couldn't quite figure out what was wrong. :o
It seems that the 'Next' button being enabled right off the bat is the cause behind this bug. The button is disabled when using wizards the "usual way" because we expect the user to choose a wizard before letting them select a page (or some other action such as filling in a project name). I presume this is a problem on Macs as well? Thread [main] (Suspended) Shell(Decorations).setDefaultButton(Button) line: 534 WizardDialog.updateButtons() line: 1256 WizardDialog.update() line: 1234 WizardDialog.showStartingPage() line: 1191 WizardDialog.createContents(Composite) line: 545 WizardDialog(Window).create() line: 431 WizardDialog(Dialog).create() line: 1088 WizardDialog(Window).open() line: 790 InstalledJREsBlock.addVM() line: 568 InstalledJREsBlock.access$10(InstalledJREsBlock) line: 565 InstalledJREsBlock$8.handleEvent(Event) line: 324 EventTable.sendEvent(Event) line: 84 Button(Widget).sendEvent(Event) line: 1161 Display.runDeferredEvents() line: 3480 Display.readAndDispatch() line: 3099 WorkbenchPreferenceDialog(Window).runEventLoop(Shell) line: 825 WorkbenchPreferenceDialog(Window).open() line: 801 OpenPreferencesAction.run() line: 65 OpenPreferencesAction(Action).runWithEvent(Event) line: 498 ActionContributionItem.handleWidgetSelection(Event, boolean) line: 584 ActionContributionItem.access$2(ActionContributionItem, Event, boolean) line: 501 ActionContributionItem$5.handleEvent(Event) line: 411
Pure JFace code to reproduce the problem. public static void main(String[] args) { Display display = new Display(); IWizard wizard = new Wizard() { public void addPages() { addPage(new WizardPageImpl()); addPage(new WizardPageImpl()); } public boolean performFinish() { return false; } public boolean canFinish() { return false; } }; new WizardDialog(null, wizard).open(); display.dispose(); } static class WizardPageImpl extends WizardPage { WizardPageImpl() { super(WizardPageImpl.class.getName()); } public void createControl(Composite parent) { setControl(parent); } }
I'm not sure how to get around this off-hand outside of doing something like attachment 108401 [details].
> I'm not sure how to get around this off-hand outside of doing something like > attachment 108401 [details]. That approach looks good to me. Since WizardDialog#createPreviousAndNextButtons(..) creates an additional Composite for the '< Back' and 'Next >' buttons, the special code for Mac and GTK in Dialog#initializeBounds() effectively becomes a no-op in this scenario. To avoid duplicating the button creation code, you could also do it like in OpenResourceDialog, i.e. add this to the end of WizardDialog#createButtonsForButtonBar(Composite): if (finishButton.getDisplay().getDismissalAlignment() == SWT.RIGHT) { // Make the default button the right-most button. // See also special code in org.eclipse.jface.dialogs.Dialog#initializeBounds() finishButton.moveBelow(null); } To solve this more generally, JFace would need to introduce another concept of a default button, which would only be used to determine the button to be moved to the right, regardless of the Shell's default button. But since the current approach of moving the Shell's default button works fine in most cases, I think it's OK to just fix problems like this where they show up.
Created attachment 130102 [details] Patch to modify createButtonsForButtonBar and test additions (In reply to comment #4) > To avoid duplicating the button creation code, you could also do it like in > OpenResourceDialog, i.e. add this to the end of > WizardDialog#createButtonsForButtonBar(Composite): Yes, I was planning to do that (if it came down to fixing it this way anyway).
Do you need more feedback on your patch, Remy?
(In reply to comment #6) > Do you need more feedback on your patch, Remy? No, I think we are good here. I will look into getting this in for M7.
Fix released to >20090414.
(In reply to comment #8) > Fix released to >20090414. Er...that should be >20090417. :O
Verified on I20090428-0100. Tests appear to have been added properly and being run also.
