221603 - package the profile UI in toolkit.jar

Status: RESOLVED WORKSFORME

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Pierre Chanial]

Atm, the profile UI is packaged in comm.jar. And since libprofile.so is part of
the GRE, any XUL app have to ship with comm.jar.
We should cut this dependency.

After applying the patch I will attach shortly, some skin files need to be moved:

o from themes/classic/communicator/profile to themes/classic/global/profile
  - migrate.gif
  - newProfile1_2.css
  - profile.css
  - profileicon-large.gif
  - profileManager.css

o from themes/modern/communicator/profile to themes/modern/global/profile
  - migrate.gif
  - newProfile1_2.css
  - profile.css
  - profile.gif

Comment 1

[Pierre Chanial]

Attachment: patch v1.0

Comment 2

[timeless]

what a mess.

can't you just move profile manager to chrome://profile/.../ and then let
the chrome service map that to whatever jar it chooses.

Comment 3

[Pierre Chanial]

Attachment: patch v2.0

following timeless comment, by putting the profile manager UI in a top level
directory.

Comment 4

[Pierre Chanial]

Attachment: Patch v2.0 continued

New contents.rdf files.

Comment 5

[Pierre Chanial]

targeting m1.6a :-)

Comment 6

[Boris Zbarsky [:bzbarsky]]

I may well not be able to review this before the freeze...

Comment 7

[Pierre Chanial]

bz: np, I'll go for sr hunting.
Forgot to mention that 
- themes/classic/communicator/profile should be moved to themes/classic/profile
- themes/modern/communicator/profile should be moved to themes/modern/profile

Comment 8

[Conrad Carlen (not reading bugmail)]

If profile chrome is in a new top level directory, doesn't that require changes
to xpinstall?

Comment 9

[Pierre Chanial]

ccarlen: I didn't know it was necessary since I didn't create a new jar file.
Since bz is busy, requesting sr and advice from bryner.

Comment 10

[Daniel Veditz [:dveditz]]

Yes, if you create a new contents.rdf (or equivalently, change the build scripts
to add lines to installed-chrome.txt) you need to add registerChrome() calls to
various install scripts. You'll have to register the locale chrome in the
lang????.jst script templates and the content and skin wherever other
toolkit.jar chrome is registered (probably various xpcom.jst, possibly browser.jst).

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 133664 [details] [diff] [review]
patch v2.0

r- primarily for missing install and build fu

>Index: allmakefiles.sh
>===================================================================
>+profile/resources/content/contents.rdf
>+profile/resources/locale/en-US/contents.rdf

This can't be right, other than this addition everything else is a makefile (as
you'd expect from the file's name, which is not allmakefilesandcontentsrdf.sh).

see
http://lxr.mozilla.org/mozilla/source/xpfe/global/resources/content/Makefile.in
#42 for example.

Has Seamonkey and Firebird combined to use a single "Mozilla" toolkit now? If
not putting one of them into the GRE seems a bit wrong. Putting app-specific
dialogs into toolkit.jar also seems wrong. I think you're going to have to run
this by whoever owns toolkit these days (ben goodger? bryner might know) but
I'm guessing you need to end up with a separate profile.jar -- lumping this in
toolkit makes less work for *you*, but I don't think you can otherwise argue
it's better that way. But I'll let others argue that point, my r- is for the
build breakage and missing install piece. Of course the decisions made on the
points I raise here will impact exactly what you register in the makefile and
install scripts.

Comment 12

[timeless]

dveditz, see
http://bonsai.mozilla.org/cvsblame.cgi?
file=mozilla/allmakefiles.sh&rev=1.446&mark=60-63,314,441-442,666-
668,685,687,689,703,728-730,732,734,736-737,739,741-742,747-749,754-755,#60

I gave up marking, it's a royal pain. i blame kario :)

basically someone decided that since this file does magic conversions it would 
be a good way to version rdf files.

the explanation for why we version them this way isn't rational so i'm not 
going to try to explain it here (people curious about why i don't like it are 
free to ask me outside bugzilla).

Comment 13

[Brian Ryner (not reading)]

The contents.rdf thing is totally valid; we generate those files with configure
now so that MOZILLA_VERSION can be substituted in (which makes it so we don't
have content and locale versions getting out of sync when we bump the version).
 I'm not sure what other "missing install piece" dveditz is referring to.

Whether it should be in toolkit.jar is debatable.  I would personally say that
since you've gone ahead and moved its chrome package (chrome://profile/), it
might make sense to go ahead and split it into a new jar file as dveditz suggests.

Then again, more jar files slows startup time, etc.  dveditz, are you saying the
GRE includes toolkit.jar?  I thought it just included embed.jar.

Comment 14

[Brian Ryner (not reading)]

pch, what are you planning to do with this patch?

Comment 15

[Brian Ryner (not reading)]

Comment on attachment 133664 [details] [diff] [review]
patch v2.0

clearing review request due to unaddressed questions

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 133666 [details] [diff] [review]
Patch v2.0 continued

clearing review request due to unaddressed questions

Comment 17

[Benjamin Smedberg]

This happened with the profile rewrite.

Comment 18

[Simon Paquet [:sipaq]]

Comment on attachment 133666 [details] [diff] [review]
Patch v2.0 continued

Since this bug has been resolved as WFM, I'm removing the
review request.