194315 - Patches to synch minotaur with the trunk

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[Scott MacGregor]

I'm going to put various sets of patches here for minotaur in case someone trys
to sink minotaur up to whatever the current trunk happens to be.

Comment 1

[Scott MacGregor]

Attachment: build and configure changes

Build and configure changes for building an xre_app and building moz_minotaur.
see Bug #173084 for more information about how this particular patch should be
used.

Comment 2

[Scott MacGregor]

Attachment: force links to work right in minotaur

The changes required to allow link handling & the throbber to work properly in
minotaur.

Comment 3

[Scott MacGregor]

Attachment: Remove component bar + editor/browser from window list

Remove component bar + editor/browser from window list. Remove help from menu
items since we don't have a help viewer.

Comment 4

[Scott MacGregor]

Attachment: remove navigator/editor items

This patch removes navigator and editor menu items from the File / New menu.
This code should be replaced with xul pre-processor code.

Comment 5

[Scott MacGregor]

Attachment: force all attachments to kick out to the system default

Comment 6

[Don Crandall]

Attachment: Allows completion of Minotaur build on Mac OS X

This directory is needed for a Mac OS X version.

Currently, to get a build that shows the main window, a second build_all pass
is required.

Recommended location for the resulting directory: /mozilla/mailnews/app

No patch provided as this directory does not currently exist.

Comment 7

[Scott MacGregor]

Attachment: updated patch

Here are the changes (minus the build config changes) I need in order to build
minotaur on the trunk. All of these changes are ifdef MOZ_MINOTAUR so they have

no effect on the seamonkey build. 

Explanation:
1) nsURILoader.cpp, nsDocShell.cpp --> minotaur code to force attachments and
link clicks to work correctly.

2) xpfe\components
   Don't build certain components minotaur doesn't need. 
   Instead of polluting xpfe\components\build\nsModule.cpp and Makefile.in with

   lots of minotaur ifdefs, I just forked the file leaving the original
contents 
   in a ifndef MOZ_MINOTAUR and then adding an else clause that had the
minotaur
   specifics for xpfe\components\build. 
 Note: These ifdefs will eventually go away as minotaur will not build with 
   xpfe\components. 

3) nsAppShellService --> In Ensure1Window, don't try to make that 1 window
   a browser window. Instead make sure it is a mail window. 

4) nsBrowserInstance --> this will go away soon when we no longer build
   xpfe\browser for minotaur. For not, make sure we don't register any content
   types for browser.

Comment 8

[Brian Ryner (not reading)]

Comment on attachment 119537 [details] [diff] [review]
updated patch

>Index: xpfe/components/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/components/Makefile.in,v
>retrieving revision 1.42
>diff -u -w -r1.42 Makefile.in
>--- xpfe/components/Makefile.in	24 Feb 2003 17:44:17 -0000	1.42
>+++ xpfe/components/Makefile.in	5 Apr 2003 05:42:30 -0000
>@@ -26,6 +26,7 @@
> 
> include $(DEPTH)/config/autoconf.mk
> 
>+ifndef MOZ_MINOTAUR
> DIRS	= \
> 			bookmarks \
> 			directory \
>@@ -49,12 +50,35 @@
> DIRS += autocomplete history urlbarhistory download-manager
> endif
> 
>-ifdef MOZ_ENABLE_XREMOTE
>-DIRS		+= xremote
>+ifeq ($(OS_ARCH),WINNT)
>+DIRS		+= winhooks urlwidget alerts
> endif
> 
>+else
>+
>+DIRS    = \
>+          intl \
>+	  find \
>+          download-manager \
>+          autocomplete \
>+          windowds \
>+          prefwindow \
>+          console \
>+          sidebar/public \
>+          history/public \
>+          shistory/public \
>+          bookmarks/public \
>+          timebomb \
>+          $(NULL)
>+
> ifeq ($(OS_ARCH),WINNT)
>-DIRS		+= winhooks urlwidget alerts
>+DIRS    += alerts winhooks
>+endif
>+
>+endif
>+
>+ifdef MOZ_ENABLE_XREMOTE
>+DIRS		+= xremote
> endif
> 

Let's not duplicate the alerts and winhooks lines.  Put it outside of the
minotaur/phoenix ifdefs, before build.

>Index: uriloader/base/nsURILoader.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/uriloader/base/nsURILoader.cpp,v
>retrieving revision 1.108
>diff -u -w -r1.108 nsURILoader.cpp
>--- uriloader/base/nsURILoader.cpp	19 Mar 2003 23:55:12 -0000	1.108
>+++ uriloader/base/nsURILoader.cpp	5 Apr 2003 05:42:30 -0000
>@@ -389,9 +389,29 @@
>     // Until that day, if we couldn't find a handler for the content type, then go back to the listener who
>     // originated the url request and force them to handle the content....this forces us through the old code
>     // path for unknown content types which brings up the file save as dialog...
>+
>+#ifdef MOZ_MINOTAUR
>+    // MINOTAUR HACK: force anything we can't handle through the helper app dialog
>+    if (!contentListener)
>+    {
>+        nsCOMPtr<nsIURI> uri;
>+        PRBool abortProcess = PR_FALSE;
>+        aChannel->GetURI(getter_AddRefs(uri));
>+        nsCOMPtr<nsIExternalHelperAppService> helperAppService (do_GetService(NS_EXTERNALHELPERAPPSERVICE_CONTRACTID));
>+        if (helperAppService)
>+        {
>+            rv = helperAppService->DoContent(contentType.get(), uri, m_originalContext, &abortProcess, getter_AddRefs(contentStreamListener));
>+            if (NS_SUCCEEDED(rv) && contentStreamListener)
>+                return RetargetOutput(request, contentType.get(), contentType.get(), contentStreamListener);
>+        }
>+        
>+        return rv = NS_ERROR_FAILURE; // this will cause us to bring up the unknown content handler dialog.
>+    }                                                                           
>+#else
>     if (!contentListener) {
>       contentListener = m_contentListener;
>     }
>+#endif
> 

I don't really know much about this code.  Maybe bzbarsky can suggest a way to
do this that doesn't involve an #ifdef.

>Index: docshell/base/nsWebShell.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/docshell/base/nsWebShell.cpp,v
>retrieving revision 1.602
>diff -u -w -r1.602 nsWebShell.cpp
>--- docshell/base/nsWebShell.cpp	15 Mar 2003 01:02:19 -0000	1.602
>+++ docshell/base/nsWebShell.cpp	5 Apr 2003 05:42:30 -0000
>@@ -128,6 +128,11 @@
> #include "nsIDocument.h"
> #include "nsITextToSubURI.h"
> 
>+#ifdef MOZ_MINOTAUR
>+#include "nsIExternalProtocolService.h"
>+#include "nsCExternalHandlerService.h"
>+#endif
>+

Same for this file.

>Index: xpfe/appshell/src/nsAppShellService.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/appshell/src/nsAppShellService.cpp,v
>retrieving revision 1.195
>diff -u -w -r1.195 nsAppShellService.cpp
>--- xpfe/appshell/src/nsAppShellService.cpp	6 Mar 2003 23:07:00 -0000	1.195
>+++ xpfe/appshell/src/nsAppShellService.cpp	5 Apr 2003 05:42:30 -0000
>@@ -1232,7 +1232,13 @@
>       if (NS_SUCCEEDED(rv) && !tempString.IsEmpty())
>         PR_sscanf(tempString.get(), "%d", &height);
> 
>+#ifdef MOZ_MINOTAUR
>+      PRBool windowOpened;
>+      rv = LaunchTask("mail", height, width, &windowOpened); 
>+
>+#else
>       rv = OpenBrowserWindow(height, width);
>+#endif
>     }
>   }
>   return rv;

Could you get rid of the need to do this by using the browser.chromeURL default
pref?

>Index: xpfe/browser/src/nsBrowserInstance.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/browser/src/nsBrowserInstance.cpp,v
>retrieving revision 1.255
>diff -u -w -r1.255 nsBrowserInstance.cpp
>--- xpfe/browser/src/nsBrowserInstance.cpp	25 Mar 2003 18:34:51 -0000	1.255
>+++ xpfe/browser/src/nsBrowserInstance.cpp	5 Apr 2003 05:42:30 -0000
>@@ -621,6 +621,7 @@
> // nsBrowserInstance: Helpers
> //*****************************************************************************
> 
>+#ifndef MOZ_MINOTAUR
> 
> ////////////////////////////////////////////////////////////////////////
> // browserCntHandler is a content handler component that registers
>@@ -858,3 +859,4 @@
> 
>   return NS_OK;
> }
>+#endif

It looks like you're not even linking in the library built in xpfe/browser, so
why do you need this #ifdef?

>Index: xpfe/components/build/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/components/build/Makefile.in,v
>retrieving revision 1.48
>diff -u -w -r1.48 Makefile.in
>--- xpfe/components/build/Makefile.in	15 Mar 2003 01:04:20 -0000	1.48
>+++ xpfe/components/build/Makefile.in	5 Apr 2003 05:42:30 -0000
>@@ -32,6 +32,8 @@
> IS_COMPONENT	= 1
> MODULE_NAME	= application
> 
>+ifndef MOZ_MINOTAUR
>+
> REQUIRES	= xpcom \
> 		  xpcom_obsolete \
> 		  string \
>@@ -136,5 +138,77 @@
> 	$(EXTRA_DSO_LIBS) \
> 	$(MOZ_JS_LIBS) \
> 	$(NULL)
>+
>+else
>+
>+REQUIRES	= xpcom \
>+		  string \
>+		  content \
>+		  rdf \
>+		  necko \
>+		  necko2 \
>+		  nkcache \
>+		  intl \
>+		  locale \
>+		  mork \
>+		  widget \
>+		  dom \
>+          alerts\
>+          uriloader \
>+          mimetype \
>+          webbrowserpersist \
>+          progressDlg \
>+		  pref \
>+		  docshell \
>+		  webshell \
>+		  appshell \
>+          downloadmanager \
>+		  $(NULL)
>+
>+CPPSRCS		= nsModule.cpp
>+
>+ifdef MOZ_LDAP_XPCOM
>+REQUIRES	+= mozldap
>+DEFINES		+= -DMOZ_LDAP_XPCOM
>+endif
>+
>+SHARED_LIBRARY_LIBS = \
>+	$(DIST)/lib/$(LIB_PREFIX)windowds_s.$(LIB_SUFFIX) \
>+	$(DIST)/lib/$(LIB_PREFIX)downloadmanager_s.$(LIB_SUFFIX) \
>+	$(DIST)/lib/$(LIB_PREFIX)autocomplete_s.$(LIB_SUFFIX) \
>+	$(DIST)/lib/$(LIB_PREFIX)appcompintl_s.$(LIB_SUFFIX) \
>+	$(NULL)
>+
>+LOCAL_INCLUDES	= \
>+	-I$(srcdir)/../windowds          \
>+	-I$(srcdir)/../../browser/src      \
>+    -I$(srcdir)/../download-manager/src \
>+    -I$(srcdir)/../autocomplete/src  \
>+	$(NULL)
>+
>+
>+ifeq ($(OS_ARCH),WINNT)
>+
>+SHARED_LIBRARY_LIBS += \
>+    $(DIST)/lib/$(LIB_PREFIX)alerts_s.$(LIB_SUFFIX) \
>+	$(DIST)/lib/$(LIB_PREFIX)winhooks_s.$(LIB_SUFFIX) \
>+	$(NULL)
>+
>+OS_LIBS	+= $(call EXPAND_LIBNAME,ole32 shell32)
>+
>+LOCAL_INCLUDES	+= \
>+    -I$(srcdir)/../alerts/src \
>+	-I$(srcdir)/../winhooks \
>+	$(NULL)
>+endif
>+
>+EXTRA_DSO_LDOPTS = \
>+	$(MOZ_UNICHARUTIL_LIBS) \
>+	$(MOZ_COMPONENT_LIBS) \
>+	$(EXTRA_DSO_LIBS) \
>+	$(MOZ_JS_LIBS) \
>+	$(NULL)
>+
>+endif
> 
> include $(topsrcdir)/config/rules.mk

Can you try to minimize the amount of stuff that's ifdef'd for minotaur?

>Index: xpfe/components/build/nsModule.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/components/build/nsModule.cpp,v
>retrieving revision 1.31
>diff -u -w -r1.31 nsModule.cpp
>--- xpfe/components/build/nsModule.cpp	25 Mar 2003 18:34:54 -0000	1.31
>+++ xpfe/components/build/nsModule.cpp	5 Apr 2003 05:42:30 -0000
>@@ -34,6 +34,9 @@
>  * the terms of any one of the NPL, the GPL or the LGPL.
>  *
>  * ***** END LICENSE BLOCK ***** */
>+
>+#ifndef MOZ_MINOTAUR  
>+
> #include "nsIGenericFactory.h"
> #include "nsICategoryManager.h"
> #include "nsBookmarksService.h"
>@@ -319,3 +322,84 @@
> };
> 
> NS_IMPL_NSGETMODULE(application, components)
>+
>+#else   // minotaur specific stuff that will get smaller and smaller until it shrinks to zero. 
>+
>+#include "nsIGenericFactory.h"
>+#include "nsICategoryManager.h"
>+
>+#include "rdf.h"
>+#include "nsXPIDLString.h"
>+#include "nsCharsetMenu.h"
>+#include "nsFontPackageHandler.h"
>+#include "nsWindowDataSource.h"
>+#include "nsRDFCID.h"
>+#include "nsAutoComplete.h"
>+#include "nsDownloadManager.h"
>+#include "nsDownloadProxy.h"
>+
>+#if defined(MOZ_LDAP_XPCOM)
>+#include "nsLDAPAutoCompleteSession.h"
>+#endif
>+
>+#if defined(XP_WIN)
>+#include "nsAlertsService.h" 
>+#include "nsWindowsHooks.h"
>+#endif // Windows
>+
>+#include "nsCURILoader.h"
>+
>+// Factory constructors
>+NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsWindowDataSource, Init)
>+
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsAutoCompleteItem)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsAutoCompleteResults)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsFontPackageHandler)
>+NS_GENERIC_FACTORY_CONSTRUCTOR_INIT(nsDownloadManager, Init)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsDownloadProxy)
>+
>+#if defined(MOZ_LDAP_XPCOM)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsLDAPAutoCompleteSession)
>+#endif
>+
>+#if defined(XP_WIN)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsAlertsService)
>+NS_GENERIC_FACTORY_CONSTRUCTOR(nsWindowsHooks)
>+#endif // Windows
>+
>+static const nsModuleComponentInfo components[] = {
>+
>+    { "Download Manager", NS_DOWNLOADMANAGER_CID, NS_DOWNLOADMANAGER_CONTRACTID,
>+      nsDownloadManagerConstructor },
>+    { "Download", NS_DOWNLOAD_CID, NS_DOWNLOAD_CONTRACTID,
>+      nsDownloadProxyConstructor },
>+    { "AutoComplete Search Results", NS_AUTOCOMPLETERESULTS_CID, NS_AUTOCOMPLETERESULTS_CONTRACTID,
>+      nsAutoCompleteResultsConstructor},
>+    { "AutoComplete Search Item", NS_AUTOCOMPLETEITEM_CID, NS_AUTOCOMPLETEITEM_CONTRACTID,
>+      nsAutoCompleteItemConstructor},  
>+
>+#if defined(MOZ_LDAP_XPCOM)
>+    { "LDAP Autocomplete Session", NS_LDAPAUTOCOMPLETESESSION_CID,
>+	  "@mozilla.org/autocompleteSession;1?type=ldap",
>+	  nsLDAPAutoCompleteSessionConstructor },
>+#endif 
>+
>+#if defined(XP_WIN)
>+    { "nsAlertsService", NS_ALERTSSERVICE_CID, NS_ALERTSERVICE_CONTRACTID, nsAlertsServiceConstructor},
>+    { NS_IWINDOWSHOOKS_CLASSNAME, NS_IWINDOWSHOOKS_CID, NS_IWINDOWSHOOKS_CONTRACTID, nsWindowsHooksConstructor },
>+#endif // Windows
>+
>+    { "nsCharsetMenu", NS_CHARSETMENU_CID,
>+      NS_RDF_DATASOURCE_CONTRACTID_PREFIX NS_CHARSETMENU_PID,
>+      NS_NewCharsetMenu },
>+    { "nsFontPackageHandler", NS_FONTPACKAGEHANDLER_CID,
>+      "@mozilla.org/locale/default-font-package-handler;1",
>+      nsFontPackageHandlerConstructor },
>+    { "nsWindowDataSource",
>+      NS_WINDOWDATASOURCE_CID,
>+      NS_RDF_DATASOURCE_CONTRACTID_PREFIX "window-mediator",
>+      nsWindowDataSourceConstructor },
>+};
>+
>+NS_IMPL_NSGETMODULE(application, components)
>+#endif

Same here. I'd prefer not to duplicate code inside and outside of minotaur
ifdef's.

Comment 9

[Scott MacGregor]

re-assigning

Comment 10

[Scott MacGregor]

Attachment: patch

I talked to cls and he agrees that we should go down the path of adding a
build2 directory under xpfe\components to better isolate the thunderbird
version of xpfe components. 

This new patch incorporates the following:
1) Adds the build2 directory if building moz_thunderbird (cls, myself and
bryner believe this is the best approach)
2) Removed the uriloader patch which is no longer necessary
3) Removed the nsBrowserInstance patch which is no longer necessary (as pointed
out by bryner during the previous review)
4) I didn't bother attaching the thunderbird specific files: nsModule.cpp and
Makefile.in that reside in xpfe\components\build2. 

things that did not change:
1) the ifdef in webshell. Bryner, I'll file a separate bug to find a way to fix
this without an ifdef to nsWebShell.cpp, but would like to check in the ifdef
for now. I haven't thought of a way to solve it in WebShell yet that won't
involve some amount of risk. 

2) the LoadTask ifdef in nsAppShellSerivce.cpp:
overriding brower.chromeURL would be a neat way to avoid this ifdef.
Unfortunately, the implementation of OpenBrowserWindow which would take this
argument expects the browser command line service to be a part of the build and
thunderbird does not build with that. So it would require implementation
changes for how OpenBrowserWindow works. 

3) I suspect I clean up the change in xpfe\components\Makefile.in if we wanted
to add some more ifdefs to avoid code dupliation for items like the REQUIRES
list.

Comment 11

[Scott MacGregor]

Attachment: updated patch (fixes some bitrot)

Comment 12

[Scott MacGregor]

Comment on attachment 121362 [details] [diff] [review]
updated patch (fixes some bitrot)

carrying over bryner's sr.

Comment 13

[Scott MacGregor]

patch is in.