392475 - Build Venkman as an extension for XUL (toolkit) based apps

Status: RESOLVED FIXED

Description

[Mark Banner (:standard8)]

Created attachment 276987 [details] [diff] [review]
Build Venkman as an extension v1

This patch will let SeaMonkey (and other toolkit based apps) build Venkman into the extensions/ directory, as well as maintaining the existing makexpi.sh build process for other delivery methods.

SeaMonkey needs this to be able to make Venkman an optional install in the 2.0.* builds just like it was before. It'll also be nice to have a proper entry in the add-on manager.

Some of this was based on the ideas in bug 351715 (the chatzilla version). Just let me know if you don't like anything.

I'll deal with packaging/installer updates in another bug.

Comment 1

[Mark Banner (:standard8)]

James, any particular reason why you cancelled the review on this? It'd be useful to know what I need to do to make this happen (or if there's another bug around).

Thanks.

Comment 2

[James Ross]

I am no longer working on Venkman within the confines of mozilla.org and associated.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 276987 [details] [diff] [review]
Build Venkman as an extension v1

timeless, would you be able to review this (see above comments)?

Comment 4

[timeless]

Comment on attachment 276987 [details] [diff] [review]
Build Venkman as an extension v1

INSTALL_EXTENSION_ID
that this isn't a thing you can easily change (ala version.txt) bothers me

+# Make Mozilla Suite / SeaMonkey 1.0/1,1 updates. 

1,1 => 1.1 ?

Comment 5

[Mark Banner (:standard8)]

Patch checked in, I made the 1.0/1,1 into 1.x as suggested by KaiRo on irc.

Comment 6

[Robert Kaiser]

timeless:
INSTALL_EXTENSION_ID should be a constant for the whole lifetime of venkman, actually.

Comment 7

[[e-mail]]

I'm getting an alert on startup, do I need to clean up my chrome or something?
JS stack follows:
0 [native frame]
1 showMessage(titleKey = "malformedRegistrationTitle", titleParams = , messageKey = "malformedRegistrationMessage", messageParams = SeaMonkey) ["file:///C:/mozilla/dist/bin/components/nsExtensionManager.js":825]
    extensionStrings = [xpconnect wrapped nsIStringBundle @ 0x4255680 (native @ 0x4253db0)]
    title = "Chrome Registration Failed"
    message = "SeaMonkey could not install this item because of a failure in Chrome Registration. Please contact the author about this problem."
    ps = [xpconnect wrapped nsIPromptService @ 0x425caa8 (native @ 0x425d458)]
    this = [object BackstagePass @ 0xe41a88 (native @ 0xcae95c)]
2 anonymous() ["file:///C:/mozilla/dist/bin/components/nsExtensionManager.js":1821]
    manifestFile = [xpconnect wrapped (nsISupports, nsILocalFile, nsIFile, nsILocalFileWin, nsIHashable) @ 0x424eb40 (native @ 0x424e938)]
    chromeDir = [xpconnect wrapped (nsISupports, nsILocalFile, nsIFile, nsILocalFileWin, nsIHashable) @ 0x424f8d0 (native @ 0x424f6c8)]
    manifestURI = [xpconnect wrapped nsIURI @ 0x4250c58 (native @ 0x42507e4)]
    files = [xpconnect wrapped nsISimpleEnumerator @ 0x4250e20 (native @ 0x1237938)]
    file = [xpconnect wrapped nsIRDFResource @ 0x42512b8 (native @ 0x1236d10)]
    chromeFile = [xpconnect wrapped (nsISupports, nsILocalFile, nsIFile, nsILocalFileWin, nsIHashable) @ 0x42517d8 (native @ 0x42515d0)]
    fileName = "venkman.jar"
    fileURLSpec = "file:///C:/mozilla/dist/bin/extensions/%7Bf13b157f-b174-47e7-a34d-4815ddfdfeb8%7D/chrome/venkman.jar"
    zipReader = undefined
    contentsFile = undefined
    providers = undefined
    i = undefined
    items = undefined
    item = undefined
    fileURI = undefined
    contentsFileURI = undefined
    cr = undefined
    stageFile = undefined
    this = [object Object]
3 anonymous(file = null) ["file:///C:/mozilla/dist/bin/components/nsExtensionManager.js":1537]
    this = [object Object]
4 anonymous(id = "{f13b157f-b174-47e7-a34d-4815ddfdfeb8}", file = null) ["file:///C:/mozilla/dist/bin/components/nsExtensionManager.js":4644]
    ds = [object Object]
    type = 2
    installLocation = [object Object]
    entries = undefined
    i = undefined
    location = undefined
    itemLocation = [xpconnect wrapped (nsISupports, nsILocalFile, nsIFile, nsILocalFileWin, nsIHashable) @ 0x424c508 (native @ 0x424c300)]
    installer = [object Object]
    this = [object Object]
5 anonymous() ["file:///C:/mozilla/dist/bin/components/nsExtensionManager.js":3363]
    ds = [object Object]
    updatedTargetAppInfos =
    needsRestart = true
    items = [object Object],[object Object],[object Object],[object Object]
    i = 0
    id = "{f13b157f-b174-47e7-a34d-4815ddfdfeb8}"
    installLocation = undefined
    oldLocation = [object Object]
    newLocation = [object Object]
    newTargetAppInfo = null
    ctr = undefined
    elements = undefined
    itemResource = undefined
    value = undefined
    cr = undefined
    this = [object Object]
6 anonymous(commandLine = [xpconnect wrapped nsICommandLine @ 0xcd3b50 (native @ 0x10d2010)]) ["file:///C:/mozilla/dist/bin/components/nsExtensionManager.js":2655]
    isDirty = true
    forceAutoReg = true
    componentList = [xpconnect wrapped (nsISupports, nsILocalFile, nsIFile, nsILocalFileWin, nsIHashable) @ 0xe7dab8 (native @ 0xdb99a0)]
    needsRestart = undefined
    this = [object Object]
7 [native frame]

Comment 8

[:Gijs]

Neil: how did this venkman end up in your suite? Did you install the xpi yourself, did you do a suite build with vnk enabled, or something else? Was the app dir / chrome clean before? What about your profile?

Comment 9

[Mark Banner (:standard8)]

Neil: you might want to try dumping the dist/bin/chrome, dist/bin/extensions, dist/xpi-stage and dist/chrome-stage directories and then rebuilding.

Comment 10

[Robert Kaiser]

It might even be enough to only dump anything venkman-related from chrome/ - that is the venmkan.jar (and/or an eventual flat chrome directory) as well as the entries in installed-chrome.txt, and clean up app-chrome.manifest as it will be rebuilt from installed-chrome.txt anyways.

Comment 11

[:Gijs]

Reopening, as people are reporting problems all over the place. Can someone please tell me why we can't use the same approach for Venkman outlined in bug 351715 comment 6 for ChatZilla? As far as I can tell, that's exactly what should be used here, so the extension is installed in the user's profile and no problems with write rights ensue.

Comment 12

[[e-mail]]

Comment on attachment 276987 [details] [diff] [review]
Build Venkman as an extension v1

>+INSTALL_EXTENSION_ID = {f13b157f-b174-47e7-a34d-4815ddfdfeb8}
>+XPI_PKGNAME = venkman-$(VENKMAN_VERSION)
I think you only need one pair of these per extension, you've currently got one in extensions/venkman and one in extensions/venkman/resources

The EM alert turns out to be one of the issues due to a missing chrome.manifest; it couldn't find the contents.rdf files where install.rdf said they were because I was building flat chrome, but there are other valid reasons too.

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #11)
> Reopening, as people are reporting problems all over the place. Can someone
> please tell me why we can't use the same approach for Venkman outlined in bug
> 351715 comment 6 for ChatZilla? As far as I can tell, that's exactly what
> should be used here, so the extension is installed in the user's profile and no
> problems with write rights ensue.

That comment/wiki wasn't suggesting that the extension be installed in the user's profile. That was suggesting have the installation into appdir/extensions/... and looking at the consequences of it, and deciding it wasn't too bad overall, but we'd try it out with the user base. However I believe that installing venkman into the app dir is *NOT* the problem that we need to fix.

What I think we need to do to fix the problems is to ship a chrome.manifest (which I think means setting USE_EXTENSION_MANIFEST to 1 in the makefiles) and ensuring this doesn't affect updates/user profiles.

I hope to do that testing tonight to verify this.

(In reply to comment #12)
> (From update of attachment 276987 [details] [diff] [review])
> >+INSTALL_EXTENSION_ID = {f13b157f-b174-47e7-a34d-4815ddfdfeb8}
> >+XPI_PKGNAME = venkman-$(VENKMAN_VERSION)
> I think you only need one pair of these per extension, you've currently got one
> in extensions/venkman and one in extensions/venkman/resources

I'll check but I believe you need both, otherwise the jar.mn items in resources directory get put in the wrong place (see debugQA for another example).

> The EM alert turns out to be one of the issues due to a missing
> chrome.manifest; it couldn't find the contents.rdf files where install.rdf said
> they were because I was building flat chrome, but there are other valid reasons
> too.

See above for my comments on fixing this.

Comment 14

[Robert Kaiser]

I haven't heard of problems "all over the place", I only heard of the problem that we may not have write permissions to auto-generate a manifest, which we have a patch for in bug 371671

Comment 15

[Mark Banner (:standard8)]

Since we fixed bug 371671, I've not heard of any more problems for this, so marking it as fixed again.

Comment 16

[Steffen Wilberg]

> > >+INSTALL_EXTENSION_ID = {f13b157f-b174-47e7-a34d-4815ddfdfeb8}
> > >+XPI_PKGNAME = venkman-$(VENKMAN_VERSION)
> > I think you only need one pair of these per extension, you've currently got one
> > in extensions/venkman and one in extensions/venkman/resources
> 
> I'll check but I believe you need both, otherwise the jar.mn items in resources
> directory get put in the wrong place (see debugQA for another example).

"MODULE and XPI_NAME are both set to the name of your extension; they should be repeated in all project makefiles so that all of the files land in the same place in the XPI staging area (see below)."
http://developer.mozilla.org/en/docs/Creating_Custom_Firefox_Extensions_with_the_Mozilla_Build_System#Anatomy_of_a_Simple_C.2B.2B_Extension
It doesn't suggest repeating INSTALL_EXTENSION_ID and XPI_PKGNAME as well in sub-makefiles.