431715 - Create and package a correct chrome.manifest in make*xpi.sh

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[Robert Kaiser]

From bug 417319 comment #17:
"The only slight issue that comes up with that (and which we should file a
followup for) is that we should care that make*xpi.sh create a correct
chrome.manifest and package it, but we should do that independently of this bug
here. (Note that packages created through the Mozilla build system
automatically end up with that file in the right location already.)"

AFAIK, currently the scripts generate a chrome.manifest that has slightly incorrect paths (wrong root) and do not package that file. The files and packages created by the makefile process are correct, though.

Comment 1

[:Gijs (he/him)]

Pretty sure this is just a simple change alike the one I did in bug 428848. I don't really like it, though. Why do we need it?

Comment 2

[Robert Kaiser]

I haven't yet looked at the problem in detail again, but from what I remember, chrome.manifest is generated but has a slight path incorrection and is not packaged.

Why we should package chrome.manifest is mainly because the contents.rdf has been deprecated in toolkit-based apps for a while, which is one reason why we already use chrome.manifest in SeaMonkey's builtin chatzilla. Additionally, the new manifest file style isn't just more readable, it also allows for a number of features we can't do that easily with contents.rdf, like using certain overlays only with certain applications. In the long term, once chatzilla can kill support for pre-toolkit (xpfe-based) host apps, we also can kill contents.rdf and only use the simpler _and_ more feature-rich manifest.

The biggest problem with make*xpi.sh not packaging a correct chrome.manifest is that the integration for the SeaMonkey 2 pref window only works correctly with the rules we write into chrome.manifest, but not with those in contents.rdf.

Comment 3

[:Gijs (he/him)]

(In reply to comment #2)
> I haven't yet looked at the problem in detail again, but from what I remember,
> chrome.manifest is generated but has a slight path incorrection and is not
> packaged.

As I do in the patch in the bug I mentioned, the only change required is passing -e to the make-jars.pl script. Then the paths are also correct.

> Why we should package chrome.manifest is mainly because the contents.rdf has
> been deprecated in toolkit-based apps for a while, which is one reason why we
> already use chrome.manifest in SeaMonkey's builtin chatzilla. Additionally, the
> new manifest file style isn't just more readable, it also allows for a number
> of features we can't do that easily with contents.rdf, like using certain
> overlays only with certain applications. In the long term, once chatzilla can
> kill support for pre-toolkit (xpfe-based) host apps, we also can kill
> contents.rdf and only use the simpler _and_ more feature-rich manifest.

I know about this bright and wonderful future, but right now it sucks if we have to package both.


> The biggest problem with make*xpi.sh not packaging a correct chrome.manifest is
> that the integration for the SeaMonkey 2 pref window only works correctly with
> the rules we write into chrome.manifest, but not with those in contents.rdf.
Why, though? The contents.rdf info gets put into a chrome.manifest file anyway - why does this not work properly?

Comment 4

[James Ross]

contents.rdf deing deprecated is *not* a valid reason to use chrome.manifest, FWIW.

On the plus side, there's no hosts (that I know of) which support chrome.manifest but not the application= argument (Firefox 1.5 being the first to support either). Thus, I think we can ship it without breaking things if we're careful. (And that means a very carefuly review of the file and lots of testing.)

Comment 5

[Robert Kaiser]

(In reply to comment #3)
> (In reply to comment #2)
> > The biggest problem with make*xpi.sh not packaging a correct chrome.manifest is
> > that the integration for the SeaMonkey 2 pref window only works correctly with
> > the rules we write into chrome.manifest, but not with those in contents.rdf.
> Why, though? The contents.rdf info gets put into a chrome.manifest file anyway
> - why does this not work properly?

We currently have different rules for SeaMonkey's pref tree overlay in chrome.manifest vs. contents.rdf, as SeaMonkey 1.x (ignoring the manifest) has it defined in preftree.xul and SeaMonkey 2 (preferring the manifest) in preferences.xul instead.

(In reply to comment #4)
> contents.rdf deing deprecated is *not* a valid reason to use chrome.manifest,
> FWIW.

Right - but IMHO a good reason for trying to get ready for it - and SeaMonkey trunk ("2.0a1pre") already uses the Makefile-generated chrome.manifest in the builtin ChatZilla.

> On the plus side, there's no hosts (that I know of) which support
> chrome.manifest but not the application= argument (Firefox 1.5 being the first
> to support either). Thus, I think we can ship it without breaking things if
> we're careful. (And that means a very carefuly review of the file and lots of
> testing.)

Right. Some testing is already there due to SeaMonkey trunk using this approach, but of course we'd need to test packages with the manifest in the other supported host apps - and as you say, I don't think we have a compat problem with the features we have in jar.mn so far that get written to the manifest.

Comment 6

[:Gijs (he/him)]

Attachment: Patch

Well, if we have to, might as well fix it.

Comment 7

[James Ross]

Comment on attachment 323362 [details] [diff] [review]
Patch

r=silver; I'm trusting you that -e does fix the paths. :)

Comment 8

[Robert Kaiser]

Comment on attachment 323362 [details] [diff] [review]
Patch

>Index: xpi/makexpi.sh
>===================================================================
>-safeCommand $PERL make-jars.pl -v -z zip -p preprocessor.pl -s "$FEDIR" -d "$JARROOT" '<' "$FEDIR/jar.mn"
>+safeCommand $PERL make-jars.pl -e -v -z zip -p preprocessor.pl -s "$FEDIR" -d "$JARROOT" '<' "$FEDIR/jar.mn"
> echo -n .
> safeCommand $PERL make-jars.pl -v -z zip -p preprocessor.pl -s "$FEDIR/sm" -d "$JARROOT" '<' "$FEDIR/sm/jar.mn"
> echo -n .
> safeCommand $PERL make-jars.pl -v -z zip -p preprocessor.pl -s "$FEDIR/ff" -d "$JARROOT" '<' "$FEDIR/ff/jar.mn"
> echo -n .

The other two lines below also need the -e, apart from that, it looks good. With those two lines changed, the chrome.manifest has the same lines as the one generated by the mozilla build system, so everything looks good :)

r=me with adding the -e to the other two make.jars lines as well.

Comment 9

[:Gijs (he/him)]

Checking in mozilla/extensions/irc/xpi/makelocalexpi.sh;
/cvsroot/mozilla/extensions/irc/xpi/makelocalexpi.sh,v  <--  makelocalexpi.sh
new revision: 1.5; previous revision: 1.4
done
Checking in mozilla/extensions/irc/xpi/makexpi.sh;
/cvsroot/mozilla/extensions/irc/xpi/makexpi.sh,v  <--  makexpi.sh
new revision: 1.16; previous revision: 1.15
done

Committed with nits --> FIXED.