5376 - Modal dialogs don't work in embedding applications

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P2)

Description

[Pierre Saslawsky]

Modal dialogs rely on code that has been implemented in xpfe/appshell/src/
nsWebShellWindow.cpp and in the various platform-specific implementations of
widget/src/nsAppShell.cpp.

These 2 classes - nsWebShellWindow and nsAppShell - are not used by embedding
applications. The modal dialogs must be implemented differently.

Comment 1

[Peter Trudelle]

reassigning to danm.  Who is putting up these modal dialogs?

Comment 2

[Peter Trudelle]

These classes are used by some embedding applications, such as
Navigator, Messenger and Composer.  The problem is that they aren't
part of the Gecko embedding package, and should not be required for
all embedding apps.  reassigning to rickg. Rick, we already implement the
nsINetSupports interface for our dialogs.  It seems to us like embedding
applications would need to support this so that embedded components like Gecko
& Necko can put up such UI. Since we are explicitly not embeddable, I don't see
why we would own this.  I don't know who in your area should.  cc'ing dp and
jevering

Comment 3

[Pierre Saslawsky]

The problem is broader than just nsINetSupport, I think, but the way
nsINetSupport has been implemented now makes it an absolute requirement to find a
fix.

To whomever ends up with this bug: please get in touch with me for the Mac
implementation, Ramiro for Unix and Nisheeth (webshell), Chris Karnaze (views) or
Tom Pixley (events) for Windows. You can also contact David Hyatt and Dan Matejka
from XPToolkit who have done the current implementation.

Comment 4

[rickg]

Sorry this has sat so long -- I've been buried under 100's of bugs while Kipp
and Troy were away. Peter: if your team owns the code, then I suggest your guys
work with Nisheeth -- but we don't (and shouldn't) own dialogs.

Comment 5

[Peter Trudelle]

reassigning to pinkerton as part of his work on embedding APIs in M9

Comment 6

[Mike Pinkerton (not reading bugmail)]

accepting bugs in bulk.

Comment 7

[Mike Pinkerton (not reading bugmail)]

reassigning to scc, per all of our discussions. Marking M15 to get them off our
M9 radar.

Comment 8

[Paul MacQuiddy]

dividing up phillips qa contact bugs, he no longer works here

Comment 9

[Scott Collins]

Hmmm.  This seems to belong to embedding issues.  Re-assigning accordingly.

Comment 10

[don]

Move to M16 for now ...

Comment 11

[travis]

I believe I've fixed all these occurances.

Comment 12

[Eli Goldberg]

pierre, could you possibly verify this bug, or suggest how it could be verified 
on a UI level?

Thanks!

Comment 13

[Pierre Saslawsky]

Reopening. Travis: to consider this bug closed, we need more explanations on how 
modal dialog events are received and processed. Do you have a documentation about 
the Embedding APIs? Any sample code about what an app needs to do to embed a 
browser shell?

Comment 14

[travis]

No I don't have documentation, but that's a different bug.  Every place in code 
where we put up a prompt we do so by getting a prompt off the docShell.  The 
embedder simply implements an nsIPrompt.  This is how the design works.  
Marking fixed again.

Comment 15

[Pierre Saslawsky]

On the base of these explanations, I still can't mark this bug as verified.

The nsIPrompt is obtained from nsDocShell which itself gets it from mTreeOwner 
which is declared as a nsIDocShellTreeOwner. I see 2 problems here:
- Nowhere in nsIDocShellTreeOwner we require a, or inherit from, nsIPrompt. Is 
the implementation of nsIDocShellTreeOwner by the embedding application supposed 
to implement a nsIPrompt as well?
- nsIDocShellTreeOwner defines ShowModal(). How is it supposed to work in 
embedding apps? Remember: in a system like MacOS, we can have only one event loop 
in the app. Unlike Win32, the events are not propagated directly to the windows 
or child-windows that belong to the app.

I saw an additional problem where nsIDocShellTreeOwner defines 
GetNewWindow(aChromeFlags, aDocShellTreeItem) but it looks like these flags 
should be identical to those defined in nsIWebBrowserChrome and one of these is 
'modal'. Here again I don't see how it can work in an OS like the Mac. A last 
question: is the window returned by GetNewWindow() supposed to implement 
nsIWebBrowserChrome as well? In that case, we would have the same problem as 
above to implement ShowAsModal().

Reopening. Maybe we should talk.

Comment 16

[travis]

Well first off this bug according to the description was about not relying on 
the implementation of nsIPrompt found in xpfe/appshell.  There are a number of 
interfaces the embedder can or will possibly implement.  Some are required and 
some aren't.  If the embedder wants to provide prompting, then it has to 
implement nsIPrompt interface obtainable via a GetInterface off the tree 
owner.  DocShellTreeOwner does define ShowModal and it's suppose to work 
however the embedder wishes to implement it.  Basically it signifies the 
callers desire for the window to be modal.  We certainly manage to implement it 
in an XP way.

No the returned nsIDocShellTreeItem is not required to implement 
nsIWebBrowserChrome.  At some level it is a new window, that does have 
nsIWebBrowserChrome, but not that particular object.

If you would like to come by sometime so I can explain how embedding is all 
setup then I can do that.

Comment 17

[Pierre Saslawsky]

That's fine: marking fixed/verified.

Comment 18

[Pierre Saslawsky]

Verified