Bug 147762 - [Wizards] Give Wizards an a-modality flag
Summary: [Wizards] Give Wizards an a-modality flag
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: All All
: P3 blocker (vote)
Target Milestone: 4.12 M3   Edit
Assignee: Wim Jongman CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 260633 272095 (view as bug list)
Depends on:
Blocks: 363481 260633 272095 277169 547501
  Show dependency tree
 
Reported: 2006-06-19 14:15 EDT by Gerd Castan CLA
Modified: 2019-05-21 05:10 EDT (History)
16 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gerd Castan CLA 2006-06-19 14:15:41 EDT
I found myself thinking about how to create a new resource in a plugin of mine.

A wizard came to my mind as the easiest way to do this. For no different reason.

I Opened one of my new favourite books "Building Commercial-Quality Plug-Ins" and this chapter starts with a mind-boggling sentence: "[Wizard] is used when a modal operation requires a particular sequence..."

A particular sequence is useful, but what I want to do is not necesarily modal at all. So I looked at editors. And found a completely different programming model. So different that I wanted to go back to Wizards.

Then I looked at the standard "New [whatever]" wizards that come with eclipse: New file? not necesarily modal. New class? Not modal. The only reason why they use Wizard is that wizards are easy to program [and editors have been hard to write when there is no existing file/resource].

I see no reason why I have to use/learn a completely different programming model when I don't need to make it modal.

What I'd like to do is to implement my IWizardPage and whatever I have to implement. And at runtime I want to set a flag that decides whether the wizard appears in a dialog or as an editor, enabling the user to look up things he needs to know when using the wizard.

What do you think about it?
Comment 1 Benjamin Cabé CLA 2008-04-09 17:35:21 EDT
The problem might be a bit hard to address even if adding the flag "isModal" to the wizards' extension points and interpreting its value during WizardDialog creation is quite straightforward. Imagine that, for example, the user deletes resources in its workspace while a "New Resource Wizard" is opened ...

Fixing that bug would probably require an API change, at least on IWizard (+isModal()) ? 

Can someone (Paul ? Karice ?) please release this bug to the platform-ui-inbox ?
Comment 2 Tod Creasey CLA 2008-04-11 11:46:29 EDT
It would also break a lot of wizards that expect to be run in a modal way - this would require explicit by in. Also consider user experience - if they are used to modal wizards a non modal one could catch them by surprise.

Take an Import Wizard for instance - if you delete a folder while it's files are being added it will fail all over the place.

Many products use a forms based editor to do this non modal wizard behaviour - you could consider the same approach.
Comment 3 Gerd Castan CLA 2008-05-23 10:40:07 EDT
See http://wiki.eclipse.org/E4/Eclipse_Application_Model#Common_UI_Elements

in e4, where a split between server and client is discussed, the assumptions mentioned in Comment #1 and Comment #2 will break anyway. You can't make this client server split without thinking about the topic of this bug.

I would be happy if this is tackled in e4. Probably too hard for Eclipse 3.x.

For e4 I suggest to think about making wizards not modal at all.
Comment 4 Paul Webster CLA 2008-05-26 15:53:00 EDT
Would one option not be to provide a WizardView (it seems to me that it would need to provide IWizardContainer/2 to host wizards)?

Running a normal wizard *might* lead to unexpected errors ... but I think in most cases it wouldn't.  Also, if you put them in a view, the visual cues would be different enough that users shouldn't be surprised by behavioural changes.

PW
Comment 5 Boris Bokowski CLA 2008-05-26 16:42:48 EDT
If your wizard does not appear in one of our wizard dialogs, you can open your own wizard dialog and make it non-modal:

Write a (trivial) subclass of WizardDialog that overrides getShellStyle() with:
  return getShellStyle() & ~SWT.APPLICATION_MODAL;

Create a new instance of this subclass, call setBlockOnOpen(false) on it, add your wizard pages, and call open().
Comment 6 Missing name CLA 2009-01-11 09:06:52 EST
see also http://en.wikipedia.org/wiki/Modal_window#Criticisms, and bug 260633
Comment 7 Boris Bokowski CLA 2009-11-26 16:20:34 EST
Prakash is now responsible for watching bugs in the [Wizards] component area.
Comment 8 Wim Jongman CLA 2019-05-10 07:33:59 EDT
This is the one I'm going to fix. There are many bugs open with this request. I will try to gather all of them.
Comment 9 Eclipse Genie CLA 2019-05-11 16:32:25 EDT
New Gerrit change created: https://git.eclipse.org/r/142020
Comment 10 Wim Jongman CLA 2019-05-11 16:36:03 EDT
Made setShellStyle public in WizardDialog. I have added a temporary WizardDialogTester class with main method for easy testing.

The idea is that people can do this:

WizardDialog d = new WizardDialog(shell, w);
d.setShellStyle((WizardDialog.getDefaultShellStyle() & ~SWT.APPLICATION_MODAL));
d.open();
	
I have also made an option to make all Eclipse wizards non-modal. You can activate this by specifying -Dnonmodal=true in your eclipse launch configuration. 

We can move this into a preference later.
Comment 11 Thomas Wolf CLA 2019-05-11 17:12:38 EDT
(In reply to Wim Jongman from comment #10)

> [...] an option to make all Eclipse wizards non-modal.

Does that really make sense? A non-modal dialog must react in meaningful ways if the context changes. Dialogs, whether wizards or not, written to be modal are unlikely to work as expected when they are suddenly non-modal and the user makes changes outside of the dialog that affect the dialog's context.
Comment 12 Wim Jongman CLA 2019-05-12 08:02:17 EDT
(In reply to Thomas Wolf from comment #11)
> (In reply to Wim Jongman from comment #10)
> 
> > [...] an option to make all Eclipse wizards non-modal.
> 
> Does that really make sense? A non-modal dialog must react in meaningful
> ways if the context changes. Dialogs, whether wizards or not, written to be
> modal are unlikely to work as expected when they are suddenly non-modal and
> the user makes changes outside of the dialog that affect the dialog's
> context.

Thanks, Thomas. I am familiar with this argument but it is very unlikely to ever happen. Modal windows have their place but only when critical confirmation from the user is required.

I think we need to reverse the paradigm. A wizard is not modal unless the developer explicitly makes it modal.
Comment 13 Andrey Loskutov CLA 2019-05-12 08:10:27 EDT
(In reply to Wim Jongman from comment #12)
> I think we need to reverse the paradigm. A wizard is not modal unless the
> developer explicitly makes it modal.

Would be OK if we would start with it at very beginning. Please note that there are custom applications based on Eclipse which would break if Eclipse would switch the modal wizard behavior. I assume most of *our* wizards do not want to be a-modal and that we would have lot of work to "restore" the previous state. I would be fine with new *option* to make a wizard a-modal, but not with an unconditional change.
Comment 14 Wim Jongman CLA 2019-05-12 11:00:35 EDT
(In reply to Andrey Loskutov from comment #13)

> 
> Would be OK if we would start with it at very beginning. Please note that
> there are custom applications based on Eclipse which would break if Eclipse
> would switch the modal wizard behavior. 
> but not with an unconditional change.

Yes, I was not going to change this. I'm only proposing an expert option.


> I would be fine with new *option* to make a wizard a-modal,

This is what I did. Please see [1]

[1] https://git.eclipse.org/r/142020
Comment 15 Dani Megert CLA 2019-05-12 12:31:56 EDT
(In reply to Wim Jongman from comment #14)
> (In reply to Andrey Loskutov from comment #13)
> 
> > 
> > Would be OK if we would start with it at very beginning. Please note that
> > there are custom applications based on Eclipse which would break if Eclipse
> > would switch the modal wizard behavior. 
> > but not with an unconditional change.
> 
> Yes, I was not going to change this. I'm only proposing an expert option.
What do you mean by "expert" option? There is no such concept today in our UI.

I'm with Thomas here. This will can lead to lots of problems. I'm fine as a first step to add this as preference that can be set via INI file. So "experts" can play around with it.
Comment 16 Wim Jongman CLA 2019-05-12 13:17:21 EDT
(In reply to Dani Megert from comment #15)

> I'm fine as a
> first step to add this as preference that can be set via INI file. So
> "experts" can play around with it.

To be clear, this bug is not about making all wizards non-modal. The default modality will remain the same.

This fix allows people to make a wizard dialog modeless without having to subclass WizardDialog.

While fixing this I have also added the "expert" option.

It is activated *ONLY* when you run Eclipse with the following system property

-Djface.modelessWizards=true
Comment 17 Dani Megert CLA 2019-05-13 02:56:19 EDT
(In reply to Wim Jongman from comment #16)
> (In reply to Dani Megert from comment #15)
> 
> > I'm fine as a
> > first step to add this as preference that can be set via INI file. So
> > "experts" can play around with it.
> 
> To be clear, this bug is not about making all wizards non-modal. The default
> modality will remain the same.
> 
> This fix allows people to make a wizard dialog modeless without having to
> subclass WizardDialog.
OK, that sounds reasonable.

 
> While fixing this I have also added the "expert" option.
> 
> It is activated *ONLY* when you run Eclipse with the following system
> property
> 
> -Djface.modelessWizards=true
This option would make ALL dialogs non-modal? If so, I would rename it to
-Djface.allWizardsNonModal
Comment 18 Wim Jongman CLA 2019-05-13 04:08:46 EDT
(In reply to Dani Megert from comment #17)

> > 
> > -Djface.modelessWizards=true
> This option would make ALL dialogs non-modal? If so, I would rename it to
> -Djface.allWizardsNonModal

Yes, that explains it better.

I believe the correct term is Modeless [1] but NonModal may fit better in the way the term is used. Any English native speakers with opinions about this?

[1] https://en.wikipedia.org/wiki/Modal_window
Comment 19 Paul Pazderski CLA 2019-05-13 04:37:20 EDT
(In reply to Wim Jongman from comment #18)
> I believe the correct term is Modeless but NonModal may fit better in
> the way the term is used.

+1 for non-modal from a non-native speaker.
Comment 20 Wim Jongman CLA 2019-05-14 05:29:38 EDT
Ok, I am about to push but there is still some API strategy that I like to discuss.

I have introduced two methods that allow altering the modal state.

set/getNonModal -> setNonModal(true);

    and (pulled up from Window)

setShellStyle(int) -> setShellStyle(getShellStyle() & ~SWT.APPLICATION_MODAL));

setShellStyle is a powermethod because I can also use it to alter other shell attributes like MIN and MAX.  However, its intention is not immediately clear.

Should I get rid of get/setNonModal? WDYT?
Comment 21 Thomas Wolf CLA 2019-05-14 05:44:18 EDT
I have no opinion on that. However, doesn't this also need setBlockOnOpen(false)?
Comment 22 Wim Jongman CLA 2019-05-14 06:15:11 EDT
(In reply to Thomas Wolf from comment #21)
> However, doesn't this also need setBlockOnOpen(false)?

Apologies for not replying on your Gerrit comment Thomas. 


As you know, but for others to understand, setBlockOnOpen(true) is meant to not return from the (wizard)dialog.open() call but wait until the wizard has finished. This is the default.

If an application is not relying on the result of the wizard (e.g. to refresh the workspace after a new project is created), one can use setBlockOnOpen(false).

Now we are non-modal, you're worried about the following scenario when setBlockOnOpen(true):

1. User opens the wizard dialog in the UI thread
2. UI Thread is waiting for wizard dialog to return
3. User does something outside of the wizard (because now it is non-modal)
4. UI thread tries to process that request but is blocked by 2.

After your Gerrit comment, I have tried to lock the UI like this but I did not succeed. My guess was that the dialog is also doing a readanddispatch.

I will dig a little further to make sure I understand.
Comment 23 Wim Jongman CLA 2019-05-14 06:44:40 EDT
(In reply to Wim Jongman from comment #22)

>> However, doesn't this also need setBlockOnOpen(false)? 
> I will dig a little further to make sure I understand.


This code is inside Window.open:

		// run the event loop if specified
		if (block) {
			runEventLoop(shell);
		}

I'm pretty sure that therefore setBlockOnOpen(false) is not needed for nonmodal wizards.
Comment 24 Thomas Wolf CLA 2019-05-14 15:47:50 EDT
I still don't understand what this -Djface.allWizardsNonModal flag is supposed to be for. Just gave this a try, and I have no problems at all to make various parts fail if WizardDialogs are non-modal:

* Choose in the package explorer "Select working set", click "New...". Move the wizard aside, click "OK" in the parent dialog. Gives an NPE.

* Go to "Preferences->Plug-in Developement->Target Platform", click "Add...". Move the wizard aside, click "OK" in the preferences dialog. Gives an SWTException.

While these two cases would be easy to fix, they show what I meant above: code that was written for a modal WizardDialog won't work in all cases if the WizardDialog suddenly isn't modal anymore. And thus I don't understand what this flag is for. Testing?
Comment 25 Andrey Loskutov CLA 2019-05-14 15:56:18 EDT
(In reply to Wim Jongman from comment #20)
> Ok, I am about to push but there is still some API strategy that I like to
> discuss.
> 
> I have introduced two methods that allow altering the modal state.
> 
> set/getNonModal -> setNonModal(true);

Usually we have the "positive" setters/getters, also

set/getModal -> setModal(false), but see below

>     and (pulled up from Window)
> 
> setShellStyle(int) -> setShellStyle(getShellStyle() &
> ~SWT.APPLICATION_MODAL));
> 
> setShellStyle is a powermethod because I can also use it to alter other
> shell attributes like MIN and MAX.  However, its intention is not
> immediately clear.
> 
> Should I get rid of get/setNonModal? WDYT?

Yes. Which one will win if called with opposite arguments?
Comment 26 Wim Jongman CLA 2019-05-14 17:27:46 EDT
(In reply to Thomas Wolf from comment #24)
> I still don't understand what this -Djface.allWizardsNonModal flag is
> supposed to be for. Just gave this a try, and I have no problems at all to
> make various parts fail if WizardDialogs are non-modal:

Yes, I was too optimistic in comment #12. I did understand that you meant this class of errors. 

When I said "unlikely to ever happen" I meant the following class of errors:

* User starts the new file wizard on a project  
* User leaves the modal window and deletes the project
* User presses finish on the new file wizard.

So the global flag is only for people that know what they are doing and are tired of modal wizards. In essence, it is indeed a proof of concept and a test.
Comment 27 Wim Jongman CLA 2019-05-14 17:29:02 EDT
(In reply to Andrey Loskutov from comment #25)

> Yes. Which one will win if called with opposite arguments?

As always, the last call wins.
Comment 28 Wim Jongman CLA 2019-05-16 05:52:25 EDT
If we are all good with this then I will make API documentation and push.

Summary:

1. Pulled up protected get/setShellStyle and made them public
2. Added setModal(boolean) to set/unset application model shell style
3. Added getModal() to return if the wizard is application modal
4. Global "expert" -D flag to make all wizards non modal
Comment 29 Eclipse Genie CLA 2019-05-20 11:09:35 EDT
New Gerrit change created: https://git.eclipse.org/r/142446
Comment 30 Eclipse Genie CLA 2019-05-20 11:39:57 EDT
New Gerrit change created: https://git.eclipse.org/r/142452
Comment 34 Wim Jongman CLA 2019-05-20 11:45:59 EDT
*** Bug 260633 has been marked as a duplicate of this bug. ***
Comment 35 Wim Jongman CLA 2019-05-20 11:46:46 EDT
*** Bug 272095 has been marked as a duplicate of this bug. ***
Comment 36 Sarika Sinha CLA 2019-05-21 01:47:05 EDT
This has created a blocker issue in Build id: I20190520-1805.
1.Create a Java Project from any perspective other than Java Perspective in a new Workspace
2. On Finish, Project is created but dialog does not go off as the "Switch Perspective" dialog is behind the dialog but not visible.
Comment 38 Jay Arthanareeswaran CLA 2019-05-21 01:57:57 EDT
I am also affected by this and looks like most if not all the scenarios that open a file/folder dialog are affected as well.
Comment 39 Lakshmi P Shanmugam CLA 2019-05-21 02:01:30 EDT
(In reply to Eclipse Genie from comment #31)
> Gerrit change https://git.eclipse.org/r/142020 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5aec0a7c94146fd40a4ef085b1b3212c71b717a2
> 

Looks like setting the SWT.ON_TOP style is causing the problems. The wizard dialog always remains on top and any new dialog opened when the wizard is open goes behind the wizard dialog.
Comment 40 Andrey Loskutov CLA 2019-05-21 02:12:05 EDT
Looking on the number of complains, should we revert?
Comment 41 Lakshmi P Shanmugam CLA 2019-05-21 02:38:35 EDT
I verified that removing the SWT.ON_TOP style from setShellStyle() fixes the problem.
Comment 42 Eclipse Genie CLA 2019-05-21 02:51:11 EDT
New Gerrit change created: https://git.eclipse.org/r/142473
Comment 43 Lakshmi P Shanmugam CLA 2019-05-21 03:09:45 EDT
(In reply to Eclipse Genie from comment #42)
> New Gerrit change created: https://git.eclipse.org/r/142473

The SWT.ON_TOP style was not part of the shell's style before the commit. Also, removing this style doesn't affect the rest of the change.
Comment 44 Wim Jongman CLA 2019-05-21 03:42:16 EDT
Apologies. I'm on it.
Comment 45 Wim Jongman CLA 2019-05-21 04:21:14 EDT
(In reply to Wim Jongman from comment #44)
> Apologies. I'm on it.

Thanks people for creating the patch and the reviews. The javadoc was also fixed with this change. 

I decided that the SWT.ON_TOP flag was needed because otherwise the nonmodal wizards will be hidden on focus lost but I failed to check any continuation wizard. An embarrassing mistake.

If the programmer wants to have the non-modal dialog on top then this can still be achieved by setShellStyle which was also already added for purposes like this.

I will wait a little while and then release the patch.
Comment 46 Dani Megert CLA 2019-05-21 04:41:54 EDT
(In reply to Wim Jongman from comment #45)
Thanks Wim.

> I will wait a little while and then release the patch.
Please make sure to merge before noon European time. We want the fix in the next build (06:00 EDT).