Closed Bug 294943 Opened 20 years ago Closed 19 years ago

Implement a good way to get brand + version from JS in suite trunk

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kairo, Assigned: kairo)

Details

Attachments

(1 file, 5 obsolete files)

Applications like ChatZilla and reporter have a big Problem nowadays: They want to reliably get product name and version, but suite trunk has no good interface for it. toolkit/aviary apps implement nsIXULAppInfo which allows easily querying that information, but and xpfe app like suite doesn't have such stuff. They can get the brand name from brand.properties but they still can't query the verion number, and that sucks. We should either implement nsIXULAppInfo (which would be probably the best way to do it) or give them any other reliable source for that info. For nsIXULAppInfo, see http://lxr.mozilla.org/mozilla/source/toolkit/xre/nsIXULAppInfo.idl
BTW, if we might implement nsIXULAppInfo, we might need the GUID of the new suite for it, which is 92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a (see bug 288371)
I've got a xulappinfo.js service that implements nsIXULAppInfo statically. It needs nsIXULAppInfo.xpt from my Firefox build, and it needs some preprocessing magic to fill in the blanks.
Attached file Static nsIXULAppInfo implkementation (obsolete) —
Attachment #184126 - Attachment mime type: application/octet-stream → application/x-javascript
(In reply to comment #2) > I've got a xulappinfo.js service that implements nsIXULAppInfo statically. It > needs nsIXULAppInfo.xpt from my Firefox build, and it needs some preprocessing > magic to fill in the blanks. Wow, this was fast and looks nice! Is this working in suite if you just drop in the xpt and this js into components/? It would be really nice if we could get a patch out of this that we can apply to trunk source - and please add comments to those prototype vars what they actually carry (I didn't find the comments in the .idl very enlightening either), so we can figure out what to stuff in with the preprocessor magic (and what to hardcode eventually, e.g. GUID)
Yes, I dropped this and the nsIXULAppInfo.xpt into components and it works. Of course, the values are bogus for testing (ChatZilla thought it was running on Blah 1.0, which was funny). I'll explain the values here, but they should really be documented in the IDL: vendor This is pretty clear, it is "who made this", e.g. "mozilla.org" I don't know whether it is appropriate for this to be mozilla.org for the new suite releases. ID This is the GUID for the application (I forgot the "{"/"}" in the static service). name This name of the application. aka MOZ_APP_DISPLAYNAME version The version, i.e. the proper release version number. aka MOZ_APP_VERSION appBuildID The date of the build, usually, e.g. 2005052023. Can be the same as geckoBuildID when the app is built w/ Gecko (as in this case). geckoBuildID The Gecko build ID, which must come from the build system when Gecko is built.
Ah dear, this took a few goes (and a copy of nsIXULAppInfo.idl!). But it works. CCing bsmedberg to ask the following question: It is possible to build an IDL from a different (known to exist) src dir from a particular build dir? In this case, we're in xpfe/components/xulappinfo (for lack of thinking of a better place) and would like to build toolkit/xre/nsIXULAppInfo.idl only. I tried a relative path in XPIDLSRCS, but that totally screwed up. :) Patch for the whole thing in a sec.
Attached patch nsIXULAppInfo for suite (obsolete) — Splinter Review
Note that this isn't all CVS diff because I couldn't be bothered setting up CVSDO, but all the added stff is there, including a complete copy of nsIXULAppInfo.idl (!) which hopefully can be eliminated when someone finds out the correct makefile magic. :)
Comment on attachment 184152 [details] [diff] [review] nsIXULAppInfo for suite >+export:: xulappinfo.js.in >+ $(PERL) $(MOZILLA_DIR)/config/preprocessor.pl $(DEFINES) $(ACDEFINES) $(srcdir)/xulappinfo.js.in > xulappinfo.js I don't think export:: is right; perhaps like this: xulappinfo.js: xulappinfo.js.in $(PERL) $(MOZILLA_DIR)/config/preprocessor.pl $(DEFINES) $(ACDEFINES) $^ > $@
Flags: blocking-seamonkey1.0a?
Attached patch patch v2: move over to suite/ (obsolete) — Splinter Review
OK, I've taken Silver's implementation, moved it over to suite/ and addressed Neil's comment. I've some notes though: 1) I haven't tried to build it (I'm doing so now) 2) I'm not completely sure how to test it easily 3) It's no real CVS diff (except the first hunk), as I didn't want to create the directories in CVS right now. I faked cvs diff output for the new files ;-)
Attachment #184126 - Attachment is obsolete: true
Attachment #184152 - Attachment is obsolete: true
Attachment #185964 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #185964 - Flags: review?(neil.parkwaycc.co.uk)
something else I noticed: allmakefiles.sh needs the addition of the two new Makefiles: suite/components/Makefile suite/components/xulappinfo/Makefile I'll include that change in a future patch when I know if the location in the tree is correct (so that I can do a real cvs diff after cvs adding the directories)
Flags: blocking-seamonkey1.0a? → blocking-seamonkey1.0a+
Attachment #185964 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #185964 - Flags: review?(neil.parkwaycc.co.uk)
OK, this version should really build and work now. As you might see, I included hunks for configure.in and autoconf.mk.in now, which are needed to even descend into suite/ in the build process, and the allmakefiles.sh hunk makes us create the needed makefiles. A build with that patch can be tested easily by entering Components.classes["@mozilla.org/xre/app-info;1"].getService(Components.interfaces.nsIXULAppInfo).name Components.classes["@mozilla.org/xre/app-info;1"].getService(Components.interfaces.nsIXULAppInfo).version etc. in the JS console, you should get whatever info configure has set. Note that we already return the GUID of the new suite project (and not of Mozilla suite) in .ID as we'll be rebranding SeaMonkey nightlies soon.
Assignee: jag → kairo
Attachment #185964 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #185990 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #185990 - Flags: review?(neil.parkwaycc.co.uk)
I hope this is the final patch. With help from bsmedberg, I could remove the extra copy of nsIXULAppInfo.idl from the patch and build the toolkit copy of the file with some Makefile magic.
Attachment #185990 - Attachment is obsolete: true
Attachment #186105 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #186105 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #185990 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #185990 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 186105 [details] [diff] [review] patch v4: reuse toolkit's IDL file I'm sorry to have to do this, but the makefile-fu is not quite correct. In particular, putting custom makefile rules above "rules.mk" is a recipe for problems because the default:: rule is not first. And you can't put it afterwards. What you *can* do is write the rule such: nsIXULAppInfo.idl: $(topsrcdir)/toolkit/xre/nsIXULAppInfo.idl $(NSINSTALL) etc...
Attachment #186105 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #186105 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #186105 - Flags: review-
Oh, and the nsIXULAppInfo.idl rule should be at the end of the makefile (after rules.mk).
OK, this addresses bsmedberg's comments - and also looks much nicer to myself.
Attachment #186105 - Attachment is obsolete: true
Attachment #186121 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #186121 - Flags: review?(benjamin)
Attachment #186121 - Flags: review?(benjamin) → review+
Comment on attachment 186121 [details] [diff] [review] patch v5: address bsmedberg's comments I assume the .idl will get copied/symlinked before the .xpt is generated?
Attachment #186121 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Comment on attachment 186121 [details] [diff] [review] patch v5: address bsmedberg's comments >+DIRS = \ >+ components \ >+ $(NULL) Nit: These indents are somewhat enthusiastic...
(In reply to comment #16) > (From update of attachment 186121 [details] [diff] [review] [edit]) > I assume the .idl will get copied/symlinked before the .xpt is generated? In the Makefile, there's an nsIXULAppInfo.idl: target which does nsinstall the .idl into our directory, so that we can reuse the file from toolkit/ - this is what bsmedberg told me to do. (In reply to comment #17) > (From update of attachment 186121 [details] [diff] [review] [edit]) > >+DIRS = \ > >+ components \ > >+ $(NULL) > Nit: These indents are somewhat enthusiastic... OK, it seems common to use two tabs, not three there, changed locally.
Comment on attachment 186121 [details] [diff] [review] patch v5: address bsmedberg's comments requesting approval: SeaMonkey-only change to support nsIXULAppInfo to make chatzilla, reporter and other tools/extensions happy that want to know our app name and version, or our GUID.
Attachment #186121 - Flags: approval1.8b3?
Comment on attachment 186121 [details] [diff] [review] patch v5: address bsmedberg's comments a=chofmann
Attachment #186121 - Flags: approval1.8b3? → approval1.8b3+
Checked in. Thanks again to James Ross for the actual implementation!
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: