Currently when nsBookmarksService wishes to open a window, it asks the command line service for the chrome url for the browser task and then opens a window with the chrome url. We don't have this facility in toolkit and there are not many places we need to use it. However, I'm stuck as to the best way to go, so suggestions are welcome.

I believe this bug is the only thing blocking us from getting a compiled build when we get part 5 of bug 330053 completed. Therefore I'm submitting this as a temporary patch so we can get something building so that its easier for >1 developers to do work on SeaMonkey as an XUL app. We can then work on a proper patch before our first release.

Comment on attachment 217609 [details] [diff] [review] Temporary patch This looks like a bad workaround to me, but it may be acceptable to get at least a somewhat working build of "suiterunner" now. Actually, I even wonder if a "proper fix" in the current world might be to "hardcode" how to get a SeaMonkey browser window there, as I wildly guess noone outside of suite might use that code, right?

Comment on attachment 217609 [details] [diff] [review] Temporary patch I think we should just inline the code from http://lxr.mozilla.org/seamonkey/source/xpfe/browser/src/nsBrowserInstance.cpp#647 (and do the xremote service to if we're still using it)

Revised patch addressing Neil's comments.

You almost certainly want nsIURILoader.loadURI

(In reply to comment #5) >You almost certainly want nsIURILoader.loadURI You'd think so, but it can't open a content window without an opener.

Comment on attachment 217668 [details] [diff] [review] Patch v2 Your new code should work both with and without MOZ_XUL_APP, no need to ifdef it. >+ if (NS_SUCCEEDED(rv) && chromeUrl.IsEmpty()) >+ { >+ rv = NS_ERROR_FAILURE; >+ } >+ } >+ if (NS_FAILED(rv)) Rather than all these tests I think you might as well test for chromeUrl.IsEmpty() instead. > wwatch->OpenWindow(0, chromeUrl, "_blank", "chrome,dialog=no,all", > suppArray, getter_AddRefs(newWindow)); > } The indentation in this whole section of code is screwy. Would you mind reindenting it to the 4 spaces that the rest of the file seems to use?

Revised patch as per Neil's comments.

Normal version of the -w.

Comment on attachment 217914 [details] [diff] [review] Patch v3 (diff -w) >+ nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv)); >+ if (NS_SUCCEEDED(rv)) >+ { >+ prefs->GetCharPref("browser.chromeURL", getter_Copies(chromeUrl)); >+ if (chromeUrl.IsEmpty()) >+ { >+ chromeUrl.AssignLiteral("chrome://navigator/content/navigator.xul"); >+ } >+ wwatch->OpenWindow(0, chromeUrl, "_blank", >+ "chrome,dialog=no,all", >+ suppArray, >+ getter_AddRefs(newWindow)); >+ } I think you've got your nesting wrong here; only the GetCharPref needs to be inside the first if. Also when outdenting the wwatch lines you should find you can tack suppArray onto the end of the previous line. r+sr=me with those fixed.

This is now checked in with Neil's nits addressed.