Use chrome.manifest in ChatZilla when building as in-tree extension

RESOLVED FIXED

Status

Other Applications
ChatZilla
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Caio Tiago Oliveira (asrail), Assigned: Caio Tiago Oliveira (asrail))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Assignee)

Description

11 years ago
Created attachment 256407 [details] [diff] [review]
Proposed patch

SeaMonkey builds Chatzilla in its own build and it's moving to toolkits.
Regardless Chatzilla being built as an extension, it should use manifests instead of contents.rdf for Suiterunner.

It's inside a "#ifdef MOZ_XUL_APP", so it won't break backward compatibility when building it as an extension.
Attachment #256407 - Flags: review?(silver)
(Assignee)

Comment 1

11 years ago
Created attachment 256411 [details] [diff] [review]
Adding locale-jar.mn...
Attachment #256407 - Attachment is obsolete: true
Attachment #256411 - Flags: review?(silver)
Attachment #256407 - Flags: review?(silver)
(In reply to comment #0)
> Created an attachment (id=256407) [details]
> It's inside a "#ifdef MOZ_XUL_APP", so it won't break backward compatibility
> when building it as an extension.

I think you are wrong here. I believe it will break Chatzilla's compatibility to build an extension with the latest version of code and then install that version of chatzilla into both older and new versions of applications which is what extension compatibility is all about.

We should really just be building Chatzilla as an extension in-tree (similar to what we are now doing with DOMI). Building it as an extension in-tree would mean that it wouldn't use the main chrome installed-chrome.txt (which is the main issue here) but it would sort it all out in the extension building code itself.

James (Silver) is the decision maker as he's the Chatzilla owner, but I think I would WONTFIX this bug but file a new one for building chatzilla in-tree for SeaMonkey (and updating installers etc).
(In reply to comment #2)
> (In reply to comment #0)
> James (Silver) is the decision maker as he's the Chatzilla owner, but I think I
> would WONTFIX this bug but file a new one for building chatzilla in-tree for
> SeaMonkey (and updating installers etc).

The new one for building chatzilla in-tree for SeaMonkey is actually bug 351715 I've just found out.

Comment 4

11 years ago
I'm currently lost as to the the point of this bug (or it's Venkman counterpart, bug 371671). Why is this change wanted? What bugs does it fix? What advantages does it have?

Comment 5

11 years ago
AIUI, there is no consensus on what ChatZilla should even become in suiterunner anyway. That is, whether it should retain the appmanaged status it has in current SeaMonkey builds, etc. As for advantages, I don't know exactly what the point of the use of manifests would be, though I haven't looked into it myself much, I'm fairly sure that as things stand right now, ChatZilla won't work on Suiterunner because it'll get confused about what host it's running in (Toolkit suite is a concept it hasn't heard of). Then again, a lot of other things also don't work in Suiterunner.
(Assignee)

Comment 6

11 years ago
It's a way of avoiding the use of installed-chrome.txt on Suiterunner, which requires writing access to the app dir to create the manifests files later.

Ar Mark already pointed out, building it as extension (bug 351715) will remove the need for this.

You can decide and mark this one as wontfix.

For Venkman, either bug 371671 or building it as extension will be required.
(Assignee)

Comment 7

11 years ago
(In reply to comment #5)
> AIUI, there is no consensus on what ChatZilla should even become in suiterunner
> anyway. That is, whether it should retain the appmanaged status it has in
> current SeaMonkey builds, etc. As for advantages, I don't know exactly what the
> point of the use of manifests would be, though I haven't looked into it myself
> much, I'm fairly sure that as things stand right now, ChatZilla won't work on
> Suiterunner because it'll get confused about what host it's running in (Toolkit
> suite is a concept it hasn't heard of). Then again, a lot of other things also
> don't work in Suiterunner.
> 

CZ works right now on Suiterunner, it only won't run if you install it as root and run as user or run from a Mac disk image (or any other scenario where you don't have write permissions to the app dir), see bug 371298.


Comment 8

11 years ago
I think if both are changed to be real extensions, chatzilla and venkman might still work on a contents.rdf-basis - but if they're installed in the app's chrome directory, they need to adopt .manifests

BTW, from what I know, you can even have .manifest and contents.rdf in parallel to be able to easily support old and new chrome registries.

We need to move away from installed-chrome.txt completely though due to the problem mentioned in comment #7

The best solution is probably to move both components to real extensions, which also eases dealing with other stuff (optional items in installer, viewing them as separate things for source L10n, etc.)
(In reply to comment #4)
> I'm currently lost as to the the point of this bug (or it's Venkman
> counterpart, bug 371671).

I'm going to answer this on bug 351715 as that's the one we really want.

(In reply to comment #5)
> Then again, a lot of other things also don't work in Suiterunner.

Yes, but we're working on multiple fronts to resolve that.

I'm going to resolve this as WONTFIX as I think CZ doesn't need this to work with suiterunner and it'd probably break backwards compatibility. I'll comment on bug 351715 as to what/why we need changes.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → WONTFIX
(Assignee)

Updated

11 years ago
Attachment #256411 - Attachment is obsolete: true
Attachment #256411 - Flags: review?(silver)

Comment 10

11 years ago
We'll probably need to reopen this as we just realized that not having a manifest in the extension makes chromereg try to create a chrome.manifest from the contents.rdf there - which is nice and works well basically.
It just runs into one problem: If the user running the build has no write privileges on the extension directory (as it is in the app dir), then writing this chrome.manifest fails and chromereg can't load the extension because it wants to parse that manifest to do so.
Let's see about how we deal with the venkman counterpart first though, if we find a solution we can all agree on there, we can make the same thing work for chatzilla as well.

Comment 11

11 years ago
(In reply to comment #10)
> We'll probably need to reopen this as we just realized that not having a
> manifest in the extension makes chromereg try to create a chrome.manifest from
> the contents.rdf there - which is nice and works well basically.
> It just runs into one problem: If the user running the build has no write
> privileges on the extension directory (as it is in the app dir), then writing
> this chrome.manifest fails and chromereg can't load the extension because it
> wants to parse that manifest to do so.
> Let's see about how we deal with the venkman counterpart first though, if we
> find a solution we can all agree on there, we can make the same thing work for
> chatzilla as well.
> 

Pretty sure this is in direct contradiction with the bug 351715 comment 6, which says:
> - We'd like to build CZ as a toolkit extension in-tree for developer use and so
> we can package CZ as a true add-on extension for installation (which would also
> allow the user to upgrade it)

which means there should not be any problems with write rights at all.

Comment 12

11 years ago
It does not contradict bug 351715 comment 6 - we need to install it as an extension in the app dir, we can't install it in the profile because we don't ship with a profile, we only ship an app dir. But the user only has write permission in the profile, he's not guaranteed to have it in the app dir.

If it's installed in the app dir (even under extensions/), it either needs a pre-made manifest or write permissions to create one on startup.

The "(which would also allow the user to upgrade it)" is still true in some sense. Even though the user cannot replace the version in the app dir on-disk (and EM will never try that anyways), he can "update" chatzilla, which will result in a newer version being placed in his profile, and any extension in the profile overrides (at startup/loading time) an extension with the same ID in the app dir, so from his POV he'll have upgraded, even if the old chatzilla version is still in the app dir. When he auto-updates SeaMonkey, that chatzilla version might be updated as well, but the user won't notice as he is overriding it already with his profile. A second, different profile might still use the version in the app dir though.

Comment 13

11 years ago
The need for a chrome.manifest is a bug in chromreg, as it has all the information it needs available without it.

There is, at this point, no need or reason to ship ChatZilla's XPIs with a chrome.manifest, so I would like to keep any solution for generating one to use it only in the built-in-tree extension case.

Naturally, there is no way to generate a chrome.manifest correctly except for running the host app. I look forward to the file getting out of sync. :(
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Use of manifests in Chatzilla when building with Suiterunner → Use chrome.manifest in ChatZilla when building as in-tree extension

Comment 14

11 years ago
(In reply to comment #13)
>The need for a chrome.manifest is a bug in chromreg, as it has all the
>information it needs available without it.
We actually hit a similar problem with chrome.rdf on branch. IIRC SeaMonkey 1.x only ships with installed-chrome.txt but that is suffices to rebuild chrome.rdf if it is older or non-existent.

Newer versions of the RDF data source actually verifiy that they were succesfully written to disk which stopped us from starting up when the app chrome.rdf was missing and could not be created (typically when running from a Mac disc image file). Fortunately we were in a position to tweak our chrome registry to continue without having to create the app chrome.rdf file.

As for the file getting out of sync, who keeps install.js/.rdf in sync?

Comment 15

11 years ago
install.js/rdf have almost nothing that needs to be kept in sync, and the only bit which does and changes (version) is updated by makexpi.sh automatically.

chrome.manifest is duplicating a reasonable amount more data, and is more likely to need changes in the future IMHO, which is where the small concern comes from. Thanks to the aforementioned bug in chromereg, I don't think we have a choice anyway.
I'm going to resolve this as fixed by bug 351715. It isn't quite as the remaining locales part will be fixed by bug 397246. However, this is a bug that we don't need as the work for it has been/is being done elsewhere and so there is no point in keeping this open.
Status: REOPENED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.