Community
Participate
Working Groups
The WizardDialog always reserves space to display a progressbar, even if the wizard method needsProgressMonitor()returns false. The empty space looks strange, if a dialog page contains many elements.
You probably meant to open this against the platform.
fixed in HEAD for build > 20060628.
verified in I20060809-0010
This fix is making all WizardDialogs huge on GTK. See bug 166544.
Please revert this patch until a fix can be found that does not cause regressions.
Lets revert as suggested
Created attachment 61592 [details] Patch to roll back changes
Fixed in build >20070326
Verified in I20070430-0010
This issue has not been fixed. This problem is still present in build M20080911. It seems like the roll back was verified after the fix that failed on GTK. However, the original issue of space being reserved for the progress bar still persists. Can we re-open this bug?
(In reply to comment #10) > This issue has not been fixed. This problem is still present in build > M20080911. > > It seems like the roll back was verified after the fix that failed on GTK. > However, the original issue of space being reserved for the progress bar still > persists. > > Can we re-open this bug? Sounds reasonable to me.
Prakash is now responsible for watching bugs in the [Wizards] component area.
*** Bug 180311 has been marked as a duplicate of this bug. ***
I use Eclipse Classic release (Helios, up to date) on Mac OS X 10.6.8 and have the same problem.
We need a new patch that doesn't break GTK+ http://wiki.eclipse.org/Platform_UI/How_to_Contribute PW
This hasn't been fixed since 11 years!!! Is this: funny, ridiculous or pathetic? I am solving this issue by copy-pasting entire WizardDialog class into my project and overriding method createDialogArea. Cheers, MK
(In reply to comment #16) > Is this: funny, ridiculous or pathetic? See above ... it's waiting for a contribution. PW
(In reply to Paul Webster from comment #17) > See above ... it's waiting for a contribution. Proposed fix: https://git.eclipse.org/r/16661
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=612a50f73640e54157cd7213f1e867c65713e638 Thanks Robin. PW
(In reply to Paul Webster from comment #19) > Thanks Robin. Thanks for the review. @Michal: For the record, fixing this took about an hour, with testing and fiddling around a bit to try other things. It's pretty easy to get started, see here: https://wiki.eclipse.org/Platform_UI/How_to_Contribute
Thanks for that fix! @Robin, Yes I know, the fix is exactly the same with 'patch' in my project. Next time i'll try to contribute the solution. I was not really familiar with the process. Cheers.
I reverted the fix since it caused severe bug 418683.
(In reply to Dani Megert from comment #22) > I reverted the fix since it caused severe bug 418683. So we need a progress monitor no matter what wizard.needsProgressMonitor() says? PW
(In reply to Paul Webster from comment #23) > (In reply to Dani Megert from comment #22) > > I reverted the fix since it caused severe bug 418683. > > So we need a progress monitor no matter what wizard.needsProgressMonitor() > says? > > PW Yes, eventually it will be required when calling ModalContext#run, where the monitor cannot be 'null'. At this point it's probably safer that #getProgressMonitor() always returns a monitor as it did for years, rather than creating one only when we execute the operation.
OK, maybe the way to approach this is to use gridData.exclude = true when the progress monitor is visible==false and set exclude to false when visible==true. PW
(In reply to Paul Webster from comment #25) > OK, maybe the way to approach this is to use gridData.exclude = true when > the progress monitor is visible==false and set exclude to false when > visible==true. Maybe I don't have the full picture, but why not change WizardDialog#getProgressMonitor to this?: protected IProgressMonitor getProgressMonitor() { if (progressMonitorPart != null) return progressMonitorPart; else return new NullProgressMonitor(); }
(In reply to Robin Stocker from comment #26) > (In reply to Paul Webster from comment #25) > > OK, maybe the way to approach this is to use gridData.exclude = true when > > the progress monitor is visible==false and set exclude to false when > > visible==true. > > Maybe I don't have the full picture, but why not change > WizardDialog#getProgressMonitor to this?: > > protected IProgressMonitor getProgressMonitor() { > if (progressMonitorPart != null) > return progressMonitorPart; > else > return new NullProgressMonitor(); > } The main reason is that this could break subclasses who (wrongly) assumed that they get a valid (and so far visible) progress monitor. A second reason is, that it would look a bit ugly, given the Javadoc: * @return the progress monitor, or <code>null</code> if this wizard * dialog does not have one
(In reply to Dani Megert from comment #27) > The main reason is that this could break subclasses who (wrongly) assumed > that they get a valid (and so far visible) progress monitor. Well, that would also be the case for Paul's proposed fix in comment 25. Or should we make it visible the first time someone calls getProgressMonitor? > A second reason > is, that it would look a bit ugly, given the Javadoc: > * @return the progress monitor, or <code>null</code> if this wizard > * dialog does not have one Yes, that would have to be changed. But as can be seen from the problem which caused the revert, callers already relied on the method never returning null.
(In reply to Robin Stocker from comment #28) > (In reply to Dani Megert from comment #27) > > The main reason is that this could break subclasses who (wrongly) assumed > > that they get a valid (and so far visible) progress monitor. > > Well, that would also be the case for Paul's proposed fix in comment 25. Or > should we make it visible the first time someone calls getProgressMonitor? using gridData.exclude it would be available just not visible. There is code in specific places already to set the progress monitor to visible:true and to set it back to visible:false. I was proposing we add the gridData.exclude changes in those locations, but really we should just need something like: if (!wizard.needsProgressMonitor()) { gridData.exclude = true; } PW
(In reply to Robin Stocker from comment #28) > > A second reason > > is, that it would look a bit ugly, given the Javadoc: > > * @return the progress monitor, or <code>null</code> if this wizard > > * dialog does not have one > > Yes, that would have to be changed. I guess you already know my answer to this ;-). Changing this contract after the fact is a no go as because this would break subclasses that currently return 'null'.
I've pushed up the fix suggested in comment #29 which seems to remove the progress monitor from the new workingset wizard without causing the same NPE problem. PW
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=045aaed115c36464369b61197b8d619970c106f9 PW
The fix looks good.
(In reply to Robin Stocker from comment #18) > > Proposed fix: https://git.eclipse.org/r/16661 Thanks for your fix, Robin. PW
I verified that the bug in 418683 no longer occurs. And this bug is also fixed with this. (In reply to Dani Megert from comment #30) > I guess you already know my answer to this ;-). Changing this contract after > the fact is a no go as because this would break subclasses that currently > return 'null'. You're right, I didn't consider that it is protected.
In 4.4.0.I20131028-2000 PW
The simplest fix for this issue that works with eclipse 3.x also is: class CustomWizardDialog extends WizardDialog { public CustomWizardDialog(Shell parentShell, IWizard newWizard) { super(parentShell, newWizard); } @Override protected ProgressMonitorPart createProgressMonitorPart( Composite composite, GridLayout pmlayout) { return super.createProgressMonitorPart( new Composite(getParentShell(), SWT.NONE), pmlayout); } }
