Closed Bug 394567 Opened 13 years ago Closed 13 years ago

Move SeaMonkey parts of mozilla/themes to suite/

Categories

(SeaMonkey :: Themes, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kairo, Assigned: kairo)

References

Details

Attachments

(5 files, 1 obsolete file)

Most of the mozilla/themes directory, containing Classic and Modern, is actually SeaMonkey-specific and should move to suite/ - some parts are even XPFE-only and may be completely or party unused now (Camino-like-Firefox is the only XPFE consumers that is left right now on trunk).

Warning - this bug will be a cvs copy orgy ;-)
Attached file the cvs copies for themes (obsolete) —
Here's the CVS copies file - 784 copies in one go is probably really a lot. And I even left out all files that SeaMonkey trunk doesn't use any more (contents.rdf, xpfe-style previews, all of classic/global)
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #279251 - Flags: superreview?(neil)
Attachment #279251 - Flags: review?(neil)
Here's the patch to move from using mozilla/themes to mozilla/suite/themes for SeaMonkey.
Note that you need to do the copies before applying the patch, as some copied files are patched (the Makefiles and jar.mn files of the themes).

It might be a good idea to e.g. improve the order of entries in the jar.mn as well, but if that's wanted, it can be done as a followup.
Attachment #279256 - Flags: superreview?(neil)
Attachment #279256 - Flags: review?(neil)
Robert, how much overlap is there between this bug and bug 383909?
Simon:
There should be no overlap at all, actually, as bug 383909 is only about removing files that aren't used any more, but this is about copying the files that are still used to a different place.
I don't intend to remove all those files in the bug here, as I don't know what parts of Classic still are used by Camino-like-Firefox, which still might use parts of xpfe.

I think we could go for removing the old copy of modern here and maybe suite-specific parts of Classic (as long as they are not [re]packaged in embedding), but I'll leave the cleanup of classic/global/ or maybe the whole themes/ dir up to bug 383909, esp. as there may be still fixes blocking it from happening (as well as the switch of our mac-only cousin to toolkit).
oops, I had only removed one of the two cookie icons that the P3P UI removal now killed.
Attachment #279251 - Attachment is obsolete: true
Attachment #279458 - Flags: superreview?(neil)
Attachment #279458 - Flags: review?(neil)
Attachment #279251 - Flags: superreview?(neil)
Attachment #279251 - Flags: review?(neil)
Comment on attachment 279256 [details] [diff] [review]
patch for moving to suite/themes

>Index: mozilla/suite/themes/Makefile.in
...
>+include $(DEPTH)/config/autoconf.mk
Not sure we actually need this...

>Index: mozilla/suite/themes/modern/Makefile.in
...
>-include $(DEPTH)/config/autoconf.mk
On the other hand I think we do need this one, for stuff such as $(SEAMONKEY_VERSION)
(In reply to comment #6)
> (From update of attachment 279256 [details] [diff] [review])
> >Index: mozilla/suite/themes/Makefile.in
> ...
> >+include $(DEPTH)/config/autoconf.mk
> Not sure we actually need this...

I'm also not sure, I think I just copied it, I can kill it easily.


> >Index: mozilla/suite/themes/modern/Makefile.in
> ...
> >-include $(DEPTH)/config/autoconf.mk
> On the other hand I think we do need this one, for stuff such as
> $(SEAMONKEY_VERSION)

It's included automatically by config.mk anyways AFAIK, and we have this right below this (see end of the context in this hunk).
Comment on attachment 279256 [details] [diff] [review]
patch for moving to suite/themes

OK, r+sr=me without the extra autoconf.mk include.
Attachment #279256 - Flags: superreview?(neil)
Attachment #279256 - Flags: superreview+
Attachment #279256 - Flags: review?(neil)
Attachment #279256 - Flags: review+
Attachment #279458 - Flags: superreview?(neil)
Attachment #279458 - Flags: superreview+
Attachment #279458 - Flags: review?(neil)
Attachment #279458 - Flags: review+
Depends on: 395181
The main change has been checked in now.

Here's a small followup: Standard8 reminded me that we can now stop pulling mozilla/themes for suite from client.mk, and I'd like to cvs remove modern from the old location, which also needs removal from toolkit-makefiles.sh, which in turn made me realize I had forgotten to add the new makefiles in suite's makefiles.sh
Attachment #280068 - Flags: superreview?(neil)
Attachment #280068 - Flags: review?(neil)
Comment on attachment 280068 [details] [diff] [review]
Followup 1: don't pull themes/ and remove old modern

>Index: mozilla/suite/makefiles.sh
>===================================================================
>+  suite/suite/themes/Makefile

ignore the double suite/ here, corrected locally.
Comment on attachment 280068 [details] [diff] [review]
Followup 1: don't pull themes/ and remove old modern

r=me on the suite bits but themes/Makefile.in still says DIRS=classic modern
Attachment #280068 - Flags: superreview?(neil)
Attachment #280068 - Flags: superreview+
Attachment #280068 - Flags: review?(neil)
Attachment #280068 - Flags: review+
I've checked in the addition of the new Makefiles to suite/makefiles.sh - here's the rest of this again, but along with the removal of modern from themes/Makefile.in so that it really can be cvs removed.

mozilla/themes might actually be completely unused now, so we might be able to remove even the other Makefiles and most of classic except global/ which has a few blockers for removal noted in bug 383909
Attachment #280183 - Flags: superreview?(neil)
Attachment #280183 - Flags: review?(neil)
Attachment #280183 - Flags: superreview?(neil)
Attachment #280183 - Flags: superreview+
Attachment #280183 - Flags: review?(neil)
Attachment #280183 - Flags: review+
Attachment #280183 - Flags: approval1.9?
Comment on attachment 280183 [details] [diff] [review]
Followup 1.1: don't pull themes/ and remove old modern

mconnor gave approval for this over IRC, checked in the patch itself, will do the modern removal later, maybe tomorrow.
Attachment #280183 - Flags: approval1.9?
Removal of modern theme from its old location is checked in.
Here's one last followup:
I looked into what is being repackaged for embed and discovered that only global/ and communicator/contents.rdf is actually needed from Classic there, which I could confirm with logs from Camino tinderboxen.
I'd like to cvs remove everything except global/ and communicator/contents.rdf from themes/classic along with this patch to the jar.mn, after which I'll mark this one fixed, the rest is up to bug 383909 then.
Attachment #281831 - Flags: superreview?(neil)
Attachment #281831 - Flags: review?(neil)
Attachment #281831 - Flags: superreview?(neil)
Attachment #281831 - Flags: superreview+
Attachment #281831 - Flags: review?(neil)
Attachment #281831 - Flags: review+
Comment on attachment 281831 [details] [diff] [review]
Followup 2: remove parts of classic that are not needed by embed

checked in the jar.mn change, will do the cvs removes once I know that Camino stays green.
cvs removes done, let's mark this fixed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.