66809 - New MODAL_SUBSET windows are not created

Status: RESOLVED FIXED

(MailNews Core :: Filters, defect, P1)

Description

[vargol]

On the BeOS build dated 27/01/2001 you can not add message filters as the 'New'
Filter button does absolutely nothing.
The other enabled buttons work fine.

Comment 1

[Gervase Markham [:gerv]]

Can't reproduce on 2001-01-26 or 2001-02-05 on Win95...

Gerv

Comment 2

[kurros (Wade Majors)]

Gervase: this is a BeOS only bug.This problem isn't just with this particular button, and can be seen throughout various dialogs. But is the primary thing that makes MailNews unusable for me under BeOS.The buttons, when clicked, will cause moz to print two lines to the Terminal:nsWindow::ShowMenuBar - FIXME: not implemented!nsWindow::ConstrainPosition - not implementedA new bug should be created for these two messages, and this one marked as a dupe, or the Summary of this bug should be changed.

Comment 3

[kurros (Wade Majors)]

Augh. Silly NetPositive. Sorry about the formatting on the entry above.

Comment 4

[Makoto Hamanaka]

I know that this bug can be solved by changing the window 'feel' of dialogs 
from modal to normal one. ( in nsWindow.cpp ) But if you do so, dialogs will 
not come in front of other windows, I didn't test it recently, though.

I could't find the reason why these dialogs do not appear when 
the dialogs have 'modal' feel.

Comment 5

[kurros (Wade Majors)]

Ah hah.
You hit the nail on the head Ham.

The problem is that when you click these buttons, they want to throw up another
window (an alert actually) that is a MODAL_SUBSET of the current BWindow, which
is already a MODAL_SUBSET of another BWindow. The problem is that under BeOS you
can't have a BWindow be a MODAL_SUBSET of another MODAL_SUBSET BWindow. 

Three possible solutions I can think of:
1) Don't use (evil) modal windows. Of course this would have adverse
side-effects when the new window is an alert that you want to be blocking. Eg:
"Please enter an email address before clicking next". We'll just have to trust
the user (hah!). Easiest solution and it would get us running at least.

2) Figure out how to only make them MODAL_SUBSET windows if they are an
important alert/requestor (like in the example above). Harder and might require
updates to alot of dialogs.

3) Keep a "MODAL_SUBSET"-type bool privatly to each window, as well as maybe a
pointer to any sub windows it might spawn. If we spawn a new window, set the
bool to true and update the pointer to point to the new (non modal) window. Now,
if the user activates the window prematurely (B_WINDOW_ACTIVATED, bool active =
true) while the modal-subset bool is true and the pointer is valid, do
BWindow::Activate() to the "modal" dialog that is supposed to be up. When the
"modal" dialog is closed and sends its message(?) back to the "parent" window,
unset the bool and pointer.

Methods 2 and 3 would require work. Work I don't think I'd be able to handle.

What does everyone think? I'm curious if other platforms have hit this
limitation we have, and what their workaround was.

Comment 6

[kurros (Wade Majors)]

Updating summary and reassinging to Yannick.

Comment 7

[kurros (Wade Majors)]

Attachment: diff -u nsWindow.cpp

Comment 8

[kurros (Wade Majors)]

This patch makes the Window Feel of Dialogs B_MODAL_APP_WINDOW always. This gets
us going, but dialogs will block access to the entire app, not just its
"parent". Unless someone has a better idea, I think this patch should go in so
these dialogs can be useable and prevent crashes.

Comment 9

[Makoto Hamanaka]

I tested to change the dialog's feel to B_MODAL_APP_WINDOW_FEEL, then this bug 
was solved, but on the other hand, pull-down-menus on dialogs stopped 
working properly. ( ex: Preference->Appearance->Fonts->Language encoding)

I also tested to set B_NORMAL_WINDOW_FEEL, and there is no such problem.
But then, dialogs can be behind the other windows.

So, I think B_NORMAL_WINDOW_FEEL is the best so far, because it 
disables nothing. But we should find some way to order windows.

Comment 10

[kurros (Wade Majors)]

Attachment: diff -u nsWindow.cpp (revised per last comment)

Comment 11

[kurros (Wade Majors)]

i've checked in the last attachment. open to suggestions on managing the order of windows

Comment 12

[Yannick Koehler]

No more working on Bezilla

Comment 13

[Dennis Jakobsen]

I don't know if these MODEL_SUBSET's are created or not, but it works the way it is supposed to now. marking FIXED.

Comment 14

[laurel]

marking verified worksforme.

Comment 15

[Sergei Dolgov]

Dialogs' behaviour is still far from being perfect in BeOS.
E.g. Find dialog should be permanently frontmost, when arised, but should still
allow access to main page. Now, with activating main window, find dialogs, as
password dialogs are going to background, and it is total pain to find them
sometimes among other apps and windows.
NS_METHOD nsWindow::Enable() function which should add proper behaviour for
dialogs, does really nothing in our case.

Comment 16

[Sergei Dolgov]

Attachment: Patch for dialogs

Creates dialog windows with B_FLOATING_APP_WINDOW_FEEL (instead MODAL or NORMAL
as it was before), changes Enable() method

Comment 17

[Sergei Dolgov]

Still needs additional work on allowing drop-down boxes in filter field

Comment 18

[Sergei Dolgov]

Attachment: improved patch

eWindowType_popup is now also permanently B_FLOATING_APP_FEEL.
It allows not only working "New" button for creating message filters, but also
working drop-down boxes in this filter.

Comment 19

[Sergei Dolgov]

adding myself to CC

Comment 20

[Sergei Dolgov]

reassigning to myself

Comment 21

[Paul]

ok, it seems a bit better than before.  however, shouldn't the dialog only be on
top of the window that openned it.  i.e. if I have two browser windows, one
opens a dialog window, I switch to the other browser window, it overlaps the
dialog window.  I switch back, the dialog comes forward.  this is, at least,
what happens under linux.

as for the patch, I would remove the window_feel variable, and all the comments
around it, and just put the feel type in the constructor.  the comments aren't
really needed, as, cvs gives us history, and comments are added with changes to cvs.

Comment 22

[Sergei Dolgov]

HYeah, Paul, it should, but problem is here: nsresult
nsWindow::StandardWindowCreate 

parent = ((aParent) ? (BView *)aParent->GetNativeData(NS_NATIVE_WINDOW) : nsnull);
In case of dialogs' tree (see beginning of this bug) this parent should point on
previous dialog, not on first window. In such case we'll be able to use
B_FLOATING_SUBSET_WINDOW_FEEL without problem (with adding secondary dialog to
subset of primary dialog window, not to subset of main window).
But i don't know how to do it.

Comment 23

[Sergei Dolgov]

opps, regarding previous comment... this gets just previous dialog as parent.
so thesis should be inverted, sorry.

Comment 24

[Sergei Dolgov]

Attachment: Dialogs now belongs to window, not app

Now dialogs "belongs" to window, not whole app (following Paul's request).
In reality there is something "GetTopWindow" now in code, so in future it may
be separated as function/metod, if needed.
bug 72488 depends a little bit on checkin of this (as it affects also popups),
so i'm on hold.

Comment 25

[Sergei Dolgov]

has little issue. If you close secondary dialog, e.g. Alert "Not Found" for
"Find" dialog, focus don't go automatically to primary ("Find") dialog, but to
main window.
But it seems rather subject to further SetFocus()/Show() improvements, than for
this bug.

Comment 26

[Sergei Dolgov]

Attachment: Same as previous, but fixed problem with "child" of parentless dialogs

Now parentless dialogs have, as it should be, B_NORMAL_WINDOW_FEEL flag, so
those form itslef "root" for subsets.

Comment 27

[Sergei Dolgov]

Comment on attachment 89263 [details] [diff] [review]
Dialogs now belongs to window, not app

obsolete

Comment 28

[Paul]

much better!
is there a reason that you create subsets as floating, and not modal? (line 579)
 I've changed it to B_MODAL_SUBSET_WINDOW_FEEL, and, personally, I like the way
it works a bit better.  For one thing, if you focus on another window that does
not have a dialog, the warning/dialog still stays on top of the window it is
"attached" to.

Comment 29

[Sergei Dolgov]

About MODAL.
There are 2 reasons. 
1)For "Find" dialog i prefer that main window stays still accessbile, 
when dialog is raised. As far as i understand, it should be availble in case
 of other dialogs too, except cases, when Enable() function was called explicitly
2) setting it as MODAL brings us to "roots" of this bug - drop-down lists in
"New" mail filter creation dialog don't work. So it needs bothering with flags
and handling of eWindowType_popup widgets.

If you agree with argument 1) i can try to fix problem with alert box in
Destroy() method.
Btw, maybe this bug needs some discussion outside bugzilla.

Comment 30

[Sergei Dolgov]

Attachment: patch with modal dialogs

Added implementation of NS_METHOD SetModal().

Comment 31

[Sergei Dolgov]

Attachment: patch

Fixes stupid hiding of Find box when right-click popup is called from its
text-input field, and also hiding of primary message filter dialog, when "New
Filter" dialog rises.

Comment 32

[Sergei Dolgov]

hehe, there is funny side effect with popups being floating:
with enabled tooltips "mouse over" on tooltip place brings whole mozilla
frontmost, if it was in background/non-active.
Though, remedy should be in another bug/patch - related to popups and mose
events handling.

Comment 33

[Paul]

Comment on attachment 89727 [details] [diff] [review]
patch

in nsWindow::Enable, there is a chance that
the Looper may be locked, without being Unlocked if the window type is a popup.

Comment 34

[Sergei Dolgov]

removed Enable function from my code long ago - it rather stays on the way
everywhere than really does any job in BeOS. Here it immediatelly returns NS_OK;
So patch should be updated

Comment 35

[Sergei Dolgov]

Attachment: Patch

Enable is placeholder now.
Addition in Show() prevents from driving window frontmost by tooltips.

Comment 36

[Paul]

Comment on attachment 93048 [details] [diff] [review]
Patch

You like to create extra variables, don't you Sergei :)

>+NS_METHOD nsWindow::SetModal(PRBool aModal)
>+{
>+	window_feel feel, newfeel;

You can get rid of the above two vars

>+	if(mView && mView->Window())
>+	{
>+		feel= mView->Window()->Feel();

remove above line

>+		if(aModal)
>+		{
>+			switch(feel)

change to switch(mView->Window()->Feel())

>+			{
>+			case B_FLOATING_SUBSET_WINDOW_FEEL:
>+				newfeel = B_MODAL_SUBSET_WINDOW_FEEL;

change to mView->Window()->SetFeel(B_MODAL_SUBSET_WINDOW_FEEL);
do this for all the others as well



Beyond that, tooltips will still bring windows forward, though, not the window
the tooltip is being shown for.  ex:
open two mozilla windows, open a non mozilla window, put both mozilla windows
behind the non-mozilla window.	tooltip from one window, will bring the other
one forward over the non-window.

solution: don't display tooltips for non-active windows

Comment 37

[Sergei Dolgov]

Attachment: patch

-

Comment 38

[Sergei Dolgov]

Comment on attachment 93092 [details] [diff] [review]
patch

outdated. working on new patch

Comment 39

[Paul]

taking

Comment 40

[Paul]

fix dependency.  This depends on bug#134837, it does not block it

Comment 41

[Paul]

Due to fix of Bug134837, marking fixed

Comment 42

[Sergei Dolgov]

We still don't have proper modality and floating handling...
Maybe i should open bug with more exact problem desription.

Comment 43

[Sergei Dolgov]

Ok, it seems that fix for bug 247700 (forcing message loop start for invisble
window) which fixed a bit message chain, has effect also on this problem.

Instead a tricky solution of combination several floatong and modal modes i
tried before to put it working (there were unofficial builds with working
floating and modal windows available 2 or 3 years ago), much simpler solutions
seem working.