Mark Banner (:standard8) Assignee
	Description • 19 years ago

Ok, I couldn't think of what else to call this one. Anyway its for all the Makefile and build changes that we require doing just to get something reasonable built with MOZ_XUL_APP set.

	Mark Banner (:standard8) Assignee
	Comment 1 • 19 years ago

This patch creates a few files in /suite/app that we'll need for toolkit/xre startup. Some of it will become redundant when we switch to xulrunner, but one step at a time...

Most of the files are copied and adjusted from mail/app and browser/app. The suite/app/Makefile.in is still work in progress but I'd like to see if we can get an initial version in so that others can start helping out if they wish (especially with cross-platform work).

	Brian Ryner (not reading)
	Comment 2 • 19 years ago

Comment on attachment 214725 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v1

Sorry, I don't really have time to review suite patches.

	Mark Banner (:standard8) Assignee
	Comment 3 • 19 years ago

Comment on attachment 214725 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v1

As bryner and benjamin are busy just going for our main SeaMonkey reviewers.

	jag (Peter Annema)
	Comment 4 • 19 years ago

Comment on attachment 214725 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v1

General comments: Copyright dates? Not sure how that stuff works. I think the copyright date needs to at least say 2006 somewhere. Same kind of question for "Initial Developer", and for the contributors list.

>+++ suite/app/Makefile.in	2006-03-10 17:47:45.000000000 +0000

>+#DEFINES += -DTHUNDERBIRD_ICO=\"$(DIST)/branding/thunderbird.ico\" -DAB_CD=$(AB_CD)
>...
>+#    $(srcdir)/profile/all-thunderbird.js \
>+#    $(srcdir)/profile/channel-prefs.js \

Just remove these before you check in. There are others but I suspect you wanna keep those as examples of how to do those things, e.g. icons, when we get around to them. Out of curiosity, why'd you pick thunderbird instead of firefox to copy this all from?


>+# XXX should license be LICENSE.txt?
>+libs::
>+	$(INSTALL) $(topsrcdir)/LICENSE $(DIST)/bin

Nope, the file is mozilla/LICENSE in your source tree


>+++ suite/app/suiteApp.cpp	2006-03-10 17:45:48.000000000 +0000

Let's stay in style and go with nsSuiteApp.cpp


>+  "{92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}",
>+  "Copyright (c) 2005-2006 mozilla.org",

Is this correct? Firefox (nsBrowserApp) has 1998 - 2006, and I think they might be right.


>+++ suite/app/profile/Makefile.in	2006-03-10 21:10:38.000000000 +0000

>+#DIRS = extensions

Remove this unless you're planning on adding that in soon, then please add comments.

>+FILES := \
>+        $(topsrcdir)/profile/defaults/bookmarks.html \
>+	$(topsrcdir)/profile/defaults/mimeTypes.rdf \
>+        $(topsrcdir)/profile/defaults/localstore.rdf \
>+        $(topsrcdir)/profile/defaults/panels.rdf \
>+        $(topsrcdir)/profile/defaults/search.rdf \
>+	$(NULL)

I'm not 100% sure, but I think there might be build problems if you don't convert those leading spaces into tabs. And if not for than, then just to make it pretty :-)

>+ifneq (,$(filter gtk2 mac cocoa, $(MOZ_WIDGET_TOOLKIT)))
>+DEFINES += -DHAVE_SHELL_SERVICE=1
>+endif

I don't think that's needed here. I don't even know why mail/app/profile/Makefile.in has it. lxr for this to see how and where it's used.

	Mark Banner (:standard8) Assignee
	Comment 5 • 19 years ago

(In reply to comment #4)
> Out of curiosity, why'd you pick thunderbird instead of firefox
> to copy this all from?

Because at the time, I wasn't pulling /browser and I've only pulled it now for some other files - I never build it.

Thanks for the other comments.

	Robert Kaiser
	Comment 6 • 19 years ago

Comment on attachment 214725 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v1

>+# XXX When suite becomes a fully fledged MOZ_XUL_APP we need to remove
>+# this completely.
>+ifndef MOZ_XUL_APP
> DIRS		= \
> 		xulappinfo \
> 		$(NULL)
>+endif

This is correct, but I actually think I'd prefer to to that ifndef in suite/Makefile.in instead and exclude the whole components dir (as it actually should completely go away in the new dir structure).
Perhaps go with a comment that says "XXX This should completely go away in the future but still holds the xulappinfo component needed as long as suite is no fully fledged MOZ_XUL_APP"

	Mark Banner (:standard8) Assignee
	Comment 7 • 19 years ago

This patch addresses Jag's review comments and KaiRo's comments. Just to note, looks like HAVE_SHELL_SERVICE isn't needed for us at the moment. Carrying forward jag's r.

	Robert Kaiser
	Comment 8 • 19 years ago

Comment on attachment 214786 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v2

Hmm, I should look closer at such patches, found more nits:

>+++ suite/app/Makefile.in	2006-03-11 15:45:01.000000000 +0000
>+ifneq (,$(filter-out OS2 WINNT Darwin,$(OS_ARCH)))
>+seamonkey:: $(topsrcdir)/xpfe/bootstrap/mozilla.in Makefile.in Makefile $(DEPTH)/config/autoconf.mk
>+	cat $< | sed -e "s|%MOZAPPDIR%|$(mozappdir)|" \
>+                -e "s|%MOZ_USER_DIR%|.mozilla|" \
>+		-e "s|%MREDIR%|$(mredir)|" \
>+		-e "s|%MOZILLA-BIN%|$(PROGRAM)|g" > $@
>+	chmod +x $@
>+
>+libs:: seamonkey
>+	$(INSTALL) $< $(DIST)/bin
>+
>+install:: seamonkey
>+	$(SYSINSTALL) $< $(DESTDIR)$(bindir)
>+
>+GARBAGE += seamonkey

We've cared a lot to not hardcode the app name when we went for rebranding, could you please use MOZ_APP_NAME here instead of "seamonkey"? See xpfe/bootstrap/Makefile.in for a case where this is done properly already.


>+++ suite/app/nsSuiteApp.cpp	2006-03-11 15:45:19.000000000 +0000
>+static const nsXREAppData kAppData = {
>+  sizeof(nsXREAppData),
>+  nsnull,
>+  "Mozilla",
>+  "SeaMonkey",
>+  NS_STRINGIFY(APP_VERSION),
>+  NS_STRINGIFY(BUILD_ID),
>+  "{92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}",
>+  "Copyright (c) 1998-2006 mozilla.org",
>+  NS_XRE_ENABLE_PROFILE_MIGRATOR |
>+  NS_XRE_ENABLE_EXTENSION_MANAGER
>+};

And MOZ_APP_DISPLAYNAME instead of "SeaMonkey" here.

	Mark Banner (:standard8) Assignee
	Comment 9 • 19 years ago

Comment on attachment 214786 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v2

(In reply to comment #8)
> >+++ suite/app/nsSuiteApp.cpp	2006-03-11 15:45:19.000000000 +0000
> >+static const nsXREAppData kAppData = {
> >+  sizeof(nsXREAppData),
> >+  nsnull,
> >+  "Mozilla",
> >+  "SeaMonkey",
> >+  NS_STRINGIFY(APP_VERSION),
> >+  NS_STRINGIFY(BUILD_ID),
> >+  "{92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}",
> >+  "Copyright (c) 1998-2006 mozilla.org",
> >+  NS_XRE_ENABLE_PROFILE_MIGRATOR |
> >+  NS_XRE_ENABLE_EXTENSION_MANAGER
> >+};
> 
> And MOZ_APP_DISPLAYNAME instead of "SeaMonkey" here.
> 
I've just realised, unless we want to be "Mozilla SeaMonkey" then we don't want the Mozilla on the previous line either - I'll sort out that and both the nits in the next version of the patch.

	Benjamin Smedberg
	Comment 10 • 19 years ago

> >+++ suite/app/nsSuiteApp.cpp	2006-03-11 15:45:19.000000000 +0000
> >+static const nsXREAppData kAppData = {
> >+  sizeof(nsXREAppData),
> >+  nsnull,
> >+  "Mozilla",
> >+  "SeaMonkey",

> And MOZ_APP_DISPLAYNAME instead of "SeaMonkey" here.

I don't think you want to do this: this value determines the location of the profile, which should be determined separately froma "branding" setting. (e.g. Firefox w/o official branding still uses ~/.mozilla/firefox)

	Mark Banner (:standard8) Assignee
	Comment 11 • 19 years ago

(In reply to comment #10)
> > >+++ suite/app/nsSuiteApp.cpp	2006-03-11 15:45:19.000000000 +0000
> > >+static const nsXREAppData kAppData = {
> > >+  "Mozilla",
> > >+  "SeaMonkey",
> 
> > And MOZ_APP_DISPLAYNAME instead of "SeaMonkey" here.
> 
> I don't think you want to do this: this value determines the location of the
> profile, which should be determined separately froma "branding" setting. (e.g.
> Firefox w/o official branding still uses ~/.mozilla/firefox)
> 
Benjamin,

Your comment has highlighted an issue. If we use "Mozilla" "SeaMonkey", then --version will come out something like:

Mozilla SeaMonkey 1.5a, Copyright (c) 1998-2006 mozilla.org

This is wrong, it should just be:

SeaMonkey 1.5a, Copyright (c) 1998-2006 mozilla.org

However in the first place we'll get the profile in the most desirable location of ~/.mozilla/seamonkey, whereas in the second we'll get just ~/.seamonkey

Comments? We'd like to be able to get a fix for this so we can get the proper things in both cases.

	jag (Peter Annema)
	Comment 12 • 19 years ago

~/.seamonkey may not be a bad thing (assuming we get that and not ~/seamonkey), other than that we'd have to migrate peoples' profiles.

	Mark Banner (:standard8) Assignee
	Comment 13 • 19 years ago

Updated patch with comments from Neil on IRC to fix various things. We'll go with ~/.mozilla/seamonkey for the time being until we get a response from Benjamin. MOZ_XUL_APP=1 isn't officially supported, but we'll need to sort it out before we throw the switch.

	neil@parkwaycc.co.uk
	Comment 14 • 19 years ago

Comment on attachment 214933 [details] [diff] [review]
Part 1 - Add suite/app for starting up with toolkit v3 (checked in)

>+DESKTOP_ICONS = \
>+	$(topsrcdir)/xpfe/bootstrap/icons/gtk/abcardWindow \
>+	$(topsrcdir)/xpfe/bootstrap/icons/gtk/addressbookWindow \
>+	$(topsrcdir)/xpfe/bootstrap/icons/gtk/messengerWindow \
>+	$(topsrcdir)/xpfe/bootstrap/icons/gtk/msgcomposeWindow \
>+	$(NULL) 
Needs to be the whole xpfe list, but without the prefixes.

>+# XXX Uncomment this line and delete the one under it when icons move
>+#libs:: $(addprefix icons/$(ICON_DIR)/,$(DESKTOP_ICON_FILES))
Instead change the prefix as needed.

>+FILES := \
>+	$(topsrcdir)/profile/defaults/bookmarks.html \
>+	$(topsrcdir)/profile/defaults/mimeTypes.rdf \
>+	$(topsrcdir)/profile/defaults/localstore.rdf \
>+	$(topsrcdir)/profile/defaults/panels.rdf \
>+	prefs.js \
>+	$(topsrcdir)/profile/defaults/search.rdf \
>+	$(NULL)
Why do we have to supply a prefs.js?

>+libs:: $(FILES)
>+	$(INSTALL) $^ $(DIST)/bin/defaults/profile
>+	$(INSTALL) $^ $(DIST)/bin/defaults/profile/US
>+
>+install:: $(FILES)
>+	$(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/defaults/profile
>+	$(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/defaults/profile/US
US or en-US? (I guess you might need to land after KaiRo).

sr=me if you resolve the issues above.

	Robert Kaiser
	Comment 15 • 19 years ago

(In reply to comment #14)
> >+	$(SYSINSTALL) $(IFLAGS1) $^ $(DESTDIR)$(mozappdir)/defaults/profile/US
> US or en-US? (I guess you might need to land after KaiRo).

Should be en-US (after my hopefully-soon checkin).

	Mark Banner (:standard8) Assignee
	Comment 16 • 19 years ago

Yes there is lots more chrome (and jar.mns) than just mailnews to deal with, however, address book (especially) and mailnews are smaller beasts and easier to see things working with and getting the early builds running up. At some stage we should also change from the old contents.rdf to the new forms of registration, but again, we can deal with that later.

	Mark Banner (:standard8) Assignee
	Comment 17 • 19 years ago

This patch sorts out some defines that we need in Makefile.in for the basic SeaMonkey build when switching over to MOZ_XUL_APP.

There are more modifications required to files underneath the top level directory, but those will come later. For now, with this patch and the switch to MOZ_XUL_APP (later, not in this patch) will mean the following things change:

- Pick up the new chrome registry (/chrome rather than /rdf/chrome)
- Keep the themes that are stored under /themes
- Includes toolkit in the build
- Includes toolkit/library for non-static builds
- Includes the files that "tweak" things in xpfe/bootstrap/init.d that Firefox & Thunderbird also currently include.
- Stops building xpfe/bootstrap
- The patch also prevents building of toolkits/mozapps/installer. This is because SeaMonkey currently wishes to stay with the old installer, thought that may have to change later - I haven't looked at the implications on the installer of setting MOZ_XUL_APP=1 yet.

Requesting Benjamin's r for build config approval, and Neil's sr for SeaMonkey approval.

	Robert Kaiser
	Comment 18 • 19 years ago

Comment on attachment 216299 [details] [diff] [review]
Part 3 - adjust /Makefile.in

>+tier_50_dirs += themes

Hmm, I wonder if themes/ should go into tier 99 instead, along with suite/ (even more as we plan to build suite themes from suite/ in the future)

>+tier_99_dirs	+= xpfe/bootstrap/init.d

Would be good to know what the replacement for this is ion the toolkit world...

>+# XXX SUITE doesn't want to build the toolkit installer yet

Yes, would be nice to be able to work on that independently of the rest...

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 19 • 19 years ago

(In reply to comment #18)
> >+tier_99_dirs	+= xpfe/bootstrap/init.d
> 
> Would be good to know what the replacement for this is ion the toolkit world...

I don't think toolkit has a replacement for this.

	Benjamin Smedberg
	Comment 20 • 19 years ago

Comment on attachment 216299 [details] [diff] [review]
Part 3 - adjust /Makefile.in

>Index: Makefile.in


>+ifdef MOZ_SUITE
>+tier_50_dirs += themes
>+else
> ifndef MOZ_XUL_APP
> tier_50_dirs += themes
> endif
>+endif

Please make this one hunk by using:

ifneq (,$(MOZ_SUITE)$(MOZ_XUL_APP))

> ifdef MOZ_SUITE
> tier_99_dirs	+= suite
>+# When Suite becomes a full MOZ_XUL_APP we can remove this ifdef
>+ifdef MOZ_XUL_APP
>+tier_99_dirs	+= xpfe/bootstrap/init.d
>+endif
> endif

I don't think you ought to continue supporting init.d; I've raised objections to it on several occasions before. But I'm not going to deny the patch here for that reason.

	neil@parkwaycc.co.uk
	Comment 21 • 19 years ago

(In reply to comment #20)
>(From update of attachment 216299 [details] [diff] [review])
>>+ifdef MOZ_SUITE
>>+tier_50_dirs += themes
>>+else
>> ifndef MOZ_XUL_APP
>> tier_50_dirs += themes
>> endif
>>+endif
>ifneq (,$(MOZ_SUITE)$(MOZ_XUL_APP))
That won't work because one's an ifdef and one's an ifndef.
ifneq(_1,$(MOZ_SUITE)_$(MOZ_XUL_APP))
might work though.

	Mark Banner (:standard8) Assignee
	Comment 22 • 19 years ago

As agreed on IRC, following post review discussions between Neil, KaiRo and Benjamin: Part 3 will drop xpfe/bootstrap/init.d support for MOZ_XUL_APP=1 and move themes to tier_99 again for MOZ_XUL_APP=1.

	Mark Banner (:standard8) Assignee
	Comment 23 • 19 years ago

Revised patch for checkin, carrying forward r & sr.

	Mark Banner (:standard8) Assignee
	Comment 24 • 19 years ago

With the transfer to toolkit, SeaMonkey will need to pull /chrome as well as the other modules. This is achieved by using the _toolkit lists in client.mk as opposed to the _core ones. I don't see there being any problem with pulling /chrome in our current non-xul app builds of SeaMonkey, its just a little extra disk space for those doing dev builds.

	neil@parkwaycc.co.uk
	Comment 25 • 19 years ago

Comment on attachment 216976 [details] [diff] [review]
Part 4 - Get suite to checkout "toolkit" modules (checked in)

build config -> :bs

	Serge Gautherie (:sgautherie)
	Comment 26 • 19 years ago

Quote from recent post in mozilla.dev.apps.seamonkey:
[[
FYI: On the MozillaTest tinderbox page, there's a build named "MacOSX Darwin 8.5.0 phlox Dep experimental" now that's building directly from trunk with this configuration - or better it's trying to build with it, because it's burning at the moment.

This build is for testing if SeaMonkey trunk builds (and runs) OK with MOZ_XULAPP=1 and to have an experimental Mac build for testing later on when it really does build and run.

Robert Kaiser
]]

Adding bug's URL to this page.

PS: (May be that tinderbox needs to be 'Clobbered' too, after part 4 checkin, like the other 'Dep' builds.!?.)

	Robert Kaiser
	Comment 27 • 19 years ago

The part 4 checkin made every build fail that had the mozilla/chrome directory in place before it was first checked out from CVS. This applies to all depend srcdir builds, where at least mozilla/chrome needs to be clobbered. This does not affect clobber builds (such as creature) or objdir builds (such as phlox or the private ones of reasonable developers).

Removing the URL on this bug as it doesn't describe the bug and contains lots of stuff unrelated to the issue at stake. And please don't cross-post newsgroup entries here (nor do it the other way round, btw) - there's a reason why some stuff goes here and some goes there. Here is the place for work on the bugs, there is the place for status updates and discusssion about what's happening.

	Mark Banner (:standard8) Assignee
	Comment 28 • 19 years ago

This patch gives SeaMonkey the first bits of transition of components from xpfe to toolkit. I've attempted to keep the toolkit ifdefs to a minimum whilst also not wanting to take too much from toolkit straight away. We need to do this in small steps.

The patch transfers the following items from xpfe to toolkit:

alerts
console
commandlines
filepicker
startup

Additional Notes:
- I've attempted to clean up some of the defines around the Alerts Service inclusion in xpfe.
- From what I can tell Minimo and Camino still use items in xpfe and therefore I've had to go with the slightly horrible ifdefs to only include the code if MOZ_SUITE and MOZ_XUL_APP are not defined.

	Mark Banner (:standard8) Assignee
	Comment 29 • 19 years ago

Revised patch to correct a couple of issues, and to use the xpfe js console with the commandline handler from toolkit as the main body of code for the js console in toolkit isn't compatible enough with xpfe at the moment.

See also comment 28, and as an additional note that I forgot, this doesn't address ifdefs required for chrome/locales, that will need to be done in future patches.

	Mark Banner (:standard8) Assignee
	Comment 30 • 19 years ago

Comment on attachment 217601 [details] [diff] [review]
Part 5 v2 - First stab at sorting out inclusion of modules in xpfe and toolkit

This patch has some of the ifdefs wrong. :-( cancelling review.

	Mark Banner (:standard8) Assignee
	Comment 31 • 19 years ago

This one has fixed ifdefs and should work properly. Previous comments still apply.

	Mark Banner (:standard8) Assignee
	Comment 32 • 19 years ago

Comment on attachment 217605 [details] [diff] [review]
Part 5 v3 - First stab at sorting out inclusion of modules in xpfe and toolkit

KaiRo's just pointed out another ifdef error. :-(

	Mark Banner (:standard8) Assignee
	Comment 33 • 19 years ago

Ok, new version, third review request time lucky ;-) KaiRo's had a close look at this and we've resolved some more ifdef issues.

	Robert Kaiser
	Comment 34 • 19 years ago

Comment on attachment 217616 [details] [diff] [review]
Part 5 v4 - First stab at sorting out inclusion of modules in xpfe and toolkit

I'd be glad if we could add less/no (temporary) ifdefs to toolkit, but the main priority is to get a basic toolkit-enabled suite to build right now - we should work to get rid of all those XXXed ifdefs after we achieve that but still way before 1.9 gets into a release-like stage.

I don't think I'd be allowed to do any official review, but this looks good to me now (at least now the logic of all changes is clear to me and looks correct)...

	Mark Banner (:standard8) Assignee
	Comment 35 • 19 years ago

Comment on attachment 217616 [details] [diff] [review]
Part 5 v4 - First stab at sorting out inclusion of modules in xpfe and toolkit

This one is wrong as well :-( It causes the "standard" SeaMonkey to break.

	Mark Banner (:standard8) Assignee
	Comment 36 • 19 years ago

Sorry about all the new versions. This one should work correctly in all instances.

	Karsten Düsterloh
	Comment 37 • 19 years ago

Comment on attachment 216257 [details] [diff] [review]
Part 2 - Include mailnews chrome (checked in)

Sorry for the long delay. :(

	Mark Banner (:standard8) Assignee
	Comment 38 • 19 years ago

Comment on attachment 216257 [details] [diff] [review]
Part 2 - Include mailnews chrome (checked in)

Thanks Mnyromyr. Patch checked in.

	Benjamin Smedberg
	Comment 39 • 19 years ago

Comment on attachment 217688 [details] [diff] [review]
Part 5 v5 - First stab at sorting out inclusion of modules in xpfe and toolkit

Damn, I hope you get this stuff done quickly, this is ugly.

	Mark Banner (:standard8) Assignee
	Comment 40 • 19 years ago

Comment on attachment 217688 [details] [diff] [review]
Part 5 v5 - First stab at sorting out inclusion of modules in xpfe and toolkit

This has been bitrotted by another checkin. I'll update it in a while as it makes the patch clearer.

	Mark Banner (:standard8) Assignee
	Comment 41 • 19 years ago

Updated patch to account for bitrot from a checkin earlier today. Also moved nsUserInfo registering into the ifdefs in nsModule.cpp to resolve a undefined symbol problem that I hadn't been seeing before :/

Carrying forward benjamin's r, as I don't see that its worth bothering him for that minor change (the xpfe version of nsUserInfo is part of xpfe startup which the ifdefs are removing for moz_xul_app=1).

	neil@parkwaycc.co.uk
	Comment 42 • 19 years ago

Comment on attachment 218079 [details] [diff] [review]
Part 5 v6 - First stab at sorting out inclusion of modules in xpfe and toolkit

>+ifndef MOZ_SUITE
>+# XXX Suite doesn't want these just yet
> ifdef MOZ_XUL
> DIRS += \
> 	autocomplete \
Please ensure suiterunner filepicker autocomplete works before checking in.

	Frank Wein [:mcsmurf]
	Comment 43 • 19 years ago

I needed to make these changes to compile a build with MOZ_XUL_APP enabled on Windows. To apply it to your tree, you also have to copy the files mozilla.ico and splash.bmp from xpfe/bootstrap/ to suite/app/.

	Robert Kaiser
	Comment 44 • 19 years ago

Comment on attachment 218528 [details] [diff] [review]
Patch for the Windows build

>+// Splash screen dialog.

I think it would be best to leave the splash screen stuff out of this completely for the moment...

>Index: suite/app/mozilla.ico

This should go into suite/branding/icons/ in the new structure, and probably calling it "seamonkey.ico" might be a good idea... We just need to make sure that splash.rc can pick it up correctly.

What about mozilla.manifest - should it be changed to s "seamonkey" name as well in the new world?

(BTW, I think that name change should also happen for mozilla.in etc. when they are moved over...)

	Robert Kaiser
	Comment 45 • 19 years ago

This is the pathc to get suiterunner to build on Mac. Some of this is just correcting the Makefile to things we already do in xpfe/bootstrap (the Info.plist, .rsrc and PkgInfo lines) and I've removed one line that probably was accidentally copied from Thunderbird (mail-biff-badge.png). The remaining line (*.icns) is similar to what Firefox is doing, but with a saner location for the icons :)

In addition to patching the Makefile.in, the macbuild/ dir has to be copied from xpfe/bootstrap/ to suite/app/ - excluding the icons, which are copied into suite/branding/icons/mac/ so that all our icons are in the icons/ dir in the future.
I'm very much for doing this with normal cvs add this time, as we're only adding to small files in macbuild in addition to the icons that also need not have history preserved.

	Robert Kaiser
	Comment 46 • 19 years ago

Comment on attachment 218593 [details] [diff] [review]
Make mac build complete (checked in)

OK, checked in that set of changes as well, Mac should complete the build now.

Once the builds complete on all tier-1 platforms, I think it's time to close this bug and do further work on other dependencies of bug 328887

	Mark Banner (:standard8) Assignee
	Comment 47 • 19 years ago

(In reply to comment #46)
> (From update of attachment 218593 [details] [diff] [review] [edit])
> OK, checked in that set of changes as well, Mac should complete the build now.
> Once the builds complete on all tier-1 platforms, I think it's time to close
> this bug and do further work on other dependencies of bug 328887

I agree, but we also need Benjamin to answer comment 11 or to branch that out into a separate bug.

	Benjamin Smedberg
	Comment 48 • 19 years ago

I don't understand the *why* of comment 11... it's either Mozilla Seamonkey or it's not, and the profile location should reflect that IMO.

	Robert Kaiser
	Comment 49 • 19 years ago

Benjamin, we're clearly member of the Mozilla family, but we're not allowed to use the "Mozilla" trademark in our product name, so "Mozilla SeaMonkey" is clearly breaking our agreements with MoFo.
OTOH, I don't understand why we should clutter people's home directories (or "Application Data" for that matter) with a parallel directory to .mozilla (or "Mozilla" on Windows), when our profiles could just as well be inside that one and in parallel to firefox and thunderbird.

	Benjamin Smedberg
	Comment 50 • 19 years ago

"The mozilla family" doesn't mean much to me. Who *is* the vendor of SeaMonkey? If it's not mozilla (.org or .com), I don't think you should be using ~/.mozilla. If this is mozilla.org supported, why not have vendor == "mozilla.org"?

If you want to write a patch to add an nsXULAppInfo member to override the --version text, that's fine (although it should be in a different bug). But I don't want to wallpaper over the vendor issues, because the vendor flag will probably affect other things in the future, such as the default installation location.

	Robert Kaiser
	Comment 51 • 19 years ago

"mozilla.org" sounds good, as we are a mozilla.org project (not MoCo though).

Would that give us ".mozilla.org/seamonkey" as profile dir though or would it stay ".mozilla/seamonkey"?

	Benjamin Smedberg
	Comment 52 • 19 years ago

It would be ~/.mozilla.org, which is what I recommend.

	Frank Wein [:mcsmurf]
	Comment 53 • 19 years ago

For this patch to apply you need to copy mozilla.ico from xpfe/bootstrap/ to suite/branding/icons/windows/.

	Frank Wein [:mcsmurf]
	Comment 54 • 19 years ago

bsmedberg: Can you take a look at the ifdef change in Attachment 219055 [details] [diff] and say if this is ok (the change is in nsNativeAppSupportWin.h at the buttom of the patch)?

	Benjamin Smedberg
	Comment 55 • 19 years ago

Ugh... it would be better to remove it from Firefox entirely (since I'm almost positive that Firefox doesn't actually use that string anywhere), and make it a suite-specific thing until you can fix up or replace the windowshooks code. But as a stopgap measure it's probably not that objectionable.

	neil@parkwaycc.co.uk
	Comment 56 • 19 years ago

Comment on attachment 219055 [details] [diff] [review]
Patch for the Windows build v2

>-#ifdef MOZ_PHOENIX
>+#if defined(MOZ_PHOENIX) || defined(MOZ_SUITE)
> 
> // Splash screen dialog ID.
> #define IDD_SPLASH  100
You removed the splash screen so why do you still need this?

	Frank Wein [:mcsmurf]
	Comment 57 • 19 years ago

Maybe i should give more diff context next time :). AFAIK i needed this for the ID_DDE_APPLICATION_NAME define inside the ifdef (which is used in splash.rc). But now that i take a closer look in LXR i see that ID_DDE_APPLICATION_NAME is only used in xpfe/bootstrap/nsNativeAppSupport*. So yes, this change of the ifdef can probably be dropped and the ID_DDE_APPLICATION_NAME line be removed from splash.rc

	Frank Wein [:mcsmurf]
	Comment 58 • 19 years ago

Removed the ID_DDE_APPLICATION_NAME define since it's not needed in toolkit/ code and so also removed the ifdef change in nsNativeAppSupportWin.h.

	Robert Kaiser
	Comment 59 • 19 years ago

Comment on attachment 219154 [details] [diff] [review]
Patch for the Windows build v3 (checked in)

Checked this in on behalf of Frank.

	Robert Kaiser
	Comment 60 • 19 years ago

With all tier-1 platforms building, I consider this bug fixed. Any further work on improving suiterunner or getting it to run on other platforms should go into other bugs, IMO.