Bug 323570

Comment 1 Yury 2010-08-25 04:19:28 EDT

Created attachment 177391 [details]
patch to create snippet for the issue

Comment 2 Ivan Furnadjiev 2010-08-25 06:46:53 EDT

Hi Yury,
I've tried your snippet in SWT/JFace (Windows) and I found that when you close the MessageBox the active shell is the Wizard shell (press Enter will trigger Wizard next button) and not the ProgressDialog shell. display#getActiveShell() returns wizard shell too like in RAP. Ofcourse, in SWT/JFace it is possible to close the ProgressDialog with OK/Cancel button, because it makes the ProgressDialog shell active when you click on them. That's why I think that your code have problems with both RCP and RAP.
I will try to summarize the RAP problems:
1. Client and server are out of sync when you close the MessageBox - client shows ProgressDialog shell as active (I think this is correct), server - WizardDialog shell (but SWT behaves the same).
2. Because of 1, when you click on ProgressDialog OK/Cancel button ProgressDialog shell is not activated, as client believes it is already the current active shell.

Comment 3 Yury 2010-08-25 07:17:50 EDT

Hi, Ivan.

My suggestion is in following:

Display.setActiveShell(...) method should put the activate shell only on the top of NON MODELESS style shells in the array and keep MODELESS shells in order of their activation. 
Example:
NON MODELESS 1
NON MODELESS 2
MODELESS 1
MODELESS 2

Thread active the shell of NON MODELESS 1 by setFocus on its control.
NON MODELESS 2
NON MODELESS 1
MODELESS 1
MODELESS 2

So, we keep the last modeless shell active while it will be closed (removed from Display array).


Is this fix correct?

Comment 4 Yury 2010-08-25 07:26:30 EDT

in additional to my upper comment:
Probably there are not only shell style checking in Display.setActiveShell. Logic may be a bit more complicated - we can check parents of shells to choose the right their order.
Anyway, this code works in RCP and I think it should works in RAP.

Comment 5 Ivan Furnadjiev 2010-08-25 07:57:31 EDT

(In reply to comment #3)
I don't understand... we have three APPLICATION_MODAL shells: WizardDialog, ProgressDialog and MessageBox opened in that order. In WizardDialog#stopped() it restored the "saved" focused control (saved by WizardDialog#aboutToStart), which activate WizardDialog shell again (saved focus control is the "Run" button), regardless of existing ProgressDialog on top.
(In reply to comment #4)
Yes... you are right. There is a problem in RAP, that we have to solve somehow.

Comment 6 Yury 2010-08-25 09:31:55 EDT

Ivan, there was a common example, not from the snippet.
If we will keep modal shells in order that they appeared the problem will be resolved.
In our case the order of shells is:
1. WizardDialog opens ProgressDialog - Display#shells is "WizardShell, ProgressShell"
2. WizardDialog restore focus control - Display#shells is "ProgressShell, WizardShell"
3. WizardDialog opens MessageBox - Display#shells is "ProgressShell, WizardShell, MessageShell".

We close MessageBox, Display#removeShell() remove MessageShell from order and active the last one (viz WizardShell).
But if the order will be kept (in step 2) it will be ProgressShell in Display#shells and ProgressDialog will work.

That is what I suggest to fix.

Comment 7 Ivan Furnadjiev 2010-08-25 09:57:02 EDT

OK... Agree. Commenting focusControl.setFocus() in  WizardDialog#stopped() solves the problem.
WizardDialog#stopped() calls focusControl.setFocus(). This forces WizardShell to be activated. In the original SWT code ( Control#forceFocus ) there is a check if the control isActive(). This method ( Control#isActive() ) is missing in RAP. In other words, we should not set the focus/activate shell if the control shell is blocked by another modal shell. Implementing Control#isActive() should do the trick.

Comment 8 Yury 2010-08-26 00:17:48 EDT

Thank you, Ivan. This fix is nice for me.

Comment 9 Ivan Furnadjiev 2010-08-26 08:47:48 EDT

Created attachment 177523 [details]
Patch

Comment 10 Ivan Furnadjiev 2010-08-26 08:48:33 EDT

Applied patch to CVS HEAD.