75538 - Add chrome support to embedding APIs

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[Adam Lock]

Add support to the embedding APIs to support chrome content.

Amongst the things to do:

1. nsIWebBrowserSetup should have a property to set a new webbrowser
   to host either chrome or content
2. Ensure it is possible to obtain the nsIWebNavigation interface for the
   content area through public means, e.g. GetInterface on the content
   DOM window
3. Add a "-chrome" arg to mfcEmbed that enables chrome support
4. Update mini-nav.xul to work and be part of embed.jar
5. Add some chrome tests to the smoke tests

Comment 1

[Conrad Carlen (not reading bugmail)]

What level of chrome support are we talking about? Beyond what's needed to show
chrome dialogs? In terms of those, I'm not sure why #1 is needed. PPEmbed
handles chrome dialogs without doing that.

Comment 2

[Adam Lock]

I think we need to add support to the APIs and smoke tests to raise the profile 
of our chrome support and ensure it is available to embedders through public 
means.

Comment 3

[Conrad Carlen (not reading bugmail)]

One way we could do that is to have some embedding apps use XUL dialogs for
prompts and some use native. That will also demonstate how you can choose
whether you want chrome or native for a given UI iface and how easy it is to go
either way :-)

Comment 4

[Michael Dunn]

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Comment 5

[Conrad Carlen (not reading bugmail)]

One thing we're missing for this is the ability to set the type of the
webBrowser to be chrome without using the nsIDocShellTreeItem iface. After
discussing with Adam, I added a way of doing this to nsIWebBrowserSetup.
This came up when using XUL dialogs for prompts in an embedded app. The XUL
dialogs which the embedding app made had no titles. Reason why is that
nsGlobalWindow::SetTitle will refuse to set the title is the docshell type is
not chrome. With this, patch, it works.

Comment 6

[Conrad Carlen (not reading bugmail)]

Attachment: Patch to nsIWebBrowserSetup to allow setup as chrome

Comment 7

[Conrad Carlen (not reading bugmail)]

Attachment: Patch to PPEmbed to make use of the setup feature

Comment 8

[Adam Lock]

r=[e-mail] for both patches.

Comment 9

[Judson Valeski]

wfm. r=valeski.

What about the nsIWebNavigation issue adam raises?

Comment 10

[Conrad Carlen (not reading bugmail)]

Alright, so this patch only covers point #1 that Adam made.
Adam , can you explain a bit more about point #2?
I don't think point #3 is needed. The embedding app should make the window a
chrome window if it is told to do so given the flags to
nsIWebBrowserChrome::CreateBrowserWindow.
In terms of smoketests, we are covered already here since the prompt dialogs
(part of current smoketests) are now chrome dialogs in mfcEmbed.

Comment 11

[Adam Lock]

Point 2 covers the fact that embedders will want to access nsIWebNavigation 
on the chrome and on the primary (and other?) content docshells that it 
contains. 

Therefore we have to ensure there is a public way that they can obtain the 
content docshells' nsIWebNavigation interfaces. It would be sufficient if they 
can be obtained by walking the DOM window/document hierarchy and doing a 
GetInterface for them. That just has to be verified.

Comment 12

[Adam Lock]

I did some testing and I don't think it's possible to obtain nsIWebNavigation 
without traversing the docshells at the moment.

Neither DOM windows nor documents implement nsIInterfaceRequestor and neither 
exposes nsIWebNavigation. So while you can traverse the window/frames with 
nsIDOMWindow and nsIDOMWindowCollection you can't obtain nsIWebNavigation for 
the docshell associated with the window/frame. Perhaps nsGlobalWindow should 
implement nsIInterfaceRequestor so interfaces on the docshell can be obtained 
with GetInterface?

Can anyone think of another way we might be able to get nsIWebNavigation?

Comment 13

[Adam Lock]

Attachment: Patch adds nsIInterfaceRequestor to nsGlobalWindow. GetInterface returns docshell's nsIWebNavigation

Comment 14

[Adam Lock]

Patch allows the nsIWebNavigation to be obtained for any frame. For example with 
the following pseudocode.

  nsCOMPtr<nsIDOMWindow> window;
  nsCOMPtr<nsIDOMWindowCollection> frames;
  nsCOMPtr<nsIDOMWindow> contentWindow;
  nsCOMPtr<nsIWebNavigation> conentNav;

  webBrowser->GetContentDOMWindow(getter_AddRefs(window));
  window->GetFrames(getter_AddRefs(frames));
  frames->NamedItem("content", getter_AddRefs(contentWindow));
  mContentNav = do_GetInterface(contentWindow);
  
  mContentNav->LoadURI("http://foo.com/", LOAD_FLAGS_NONE);

Comment 15

[kill this account]

FYI: as part of bug 79608 I added a GI to nsIDOMWindow

    Index: dom/src/base/nsGlobalWindow.cpp
    ===================================================================
    ...
    +  NS_INTERFACE_MAP_ENTRY(nsIInterfaceRequestor)
    ...
    ///***********************************************************************
    +// GlobalWindowImpl::nsIInterfaceRequestor
    //************************************************************************
    +NS_IMETHODIMP GlobalWindowImpl::GetInterface(const nsIID & aIID, void 
**aSink)
    +{
    +  NS_ENSURE_ARG_POINTER(aSink);
    +  *aSink = nsnull;
    +
    +  if (aIID.Equals(NS_GET_IID(nsIDocCharset))) {
    +    if (mDocShell) {
    +      nsCOMPtr<nsIDocCharset> docCharset(do_QueryInterface(mDocShell));
    +      *aSink = docCharset;
    +    }
    +  }
    +  else {
    +    return QueryInterface(aIID, aSink);
    +  }
    +
    +  NS_IF_ADDREF(((nsISupports *) * aSink));
    +  return NS_OK;
     }

Comment 16

[Judson Valeski]

-> 0.9.3

Comment 17

[Adam Lock]

Retargetting

Comment 18

[Adam Lock]

->Future

Comment 19

[Adam Lock]

Step 3 is done.

Probably still to do are:

1. Public mechanism for specifying chrome or content
2. More visibility for the embed-sample.jar & move it to embedding/samples

Comment 20

[Adam Lock]

Marking bug fixed. Conrad's patch for bug 129288 should take care of 1) in my
last comment and 2) can be added to bug 72851

Comment 21

[David Epstein]

Checking the patches against 3/15/02 Mozilla debug build.
Adam or Conrad, I looked at the source checkin. I know these patches are old and
changes have been made since, but have a few questions before verifying this:

1) WRT patch id=31829, Looks like SETUP_CONTENT_IS_CHROME flag is superceded by
 in SETUP_IS_CHROME_WRAPPER in the source: 
mozilla/embedding/browser/webBrowser/nsIWebBrowserSetup.idl or
mozilla/embedding/browser/webBrowser/nsWebBrowser.cpp

Basically the same code snippet in nsWebBrowser.cpp is now under this flag. Can
you confirm?

2) WRT patch id=31830,
/cvsroot/mozilla/embedding/browser/powerplant/source/CBrowserShell.cpp,
CBrowserShell.cpp is:
a) missing #include "nsIWebBrowserSetup.h", 
b) Code snippets for CBrowserShell constructors are different. For example,
there isn't mIsChromeShell() function called. And didn't find nsIWebBrowserSetup
use or anything like it in this file (specifically, this snippet is missing):

if (mIsChromeShell)
{
    nsCOMPtr<nsIWebBrowserSetup> wbSetup(do_QueryInterface(mWebBrowser));
    if (wbSetup)
        wbSetup->SetProperty(nsIWebBrowserSetup::SETUP_CONTENT_IS_CHROME, PR_TRUE);

What happened to this?

also, cBrowserWindow.cp didn't include some patch code.

3) WRT patch id=32034, /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp
is missing some source, but looks like code from Brian's #15 comment
(http://bugzilla.mozilla.org/show_bug.cgi?id=75538#c15) plus Adam's patch
accounts for the nsGlobalWindow changes.

But it looks like in nsGlobalWindow.cpp, GlobalWindowImpl::GetInterface(),
you're now directly returning the QI of nsIWebNav rather than the docShell (in
the patch). Could you confirm? Thanks.

I noticed that in nsGlobalWindow.h, "public nsIDomWindowEventOwner" is missing.
What happened to it?

Comment 22

[Conrad Carlen (not reading bugmail)]

> 1) WRT patch id=31829, Looks like SETUP_CONTENT_IS_CHROME flag is superceded by
 in SETUP_IS_CHROME_WRAPPER in the source: 

Yeah - I changed the name in the patch for bug 129288.

> 2) WRT patch id=31830,
/cvsroot/mozilla/embedding/browser/powerplant/source/CBrowserShell.cpp,
CBrowserShell.cpp is:

The code for PPEmbed changed so much since this patch, there's not much use in
comparing the PPEmbed portions of this patch to what went in with bug 129288.

Comment 23

[David Epstein]

Thanks Conrad. verifying against 3/15/02 0.9.9 Mozilla debug build