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

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Robert Kaiser]

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

Comment 1

[Robert Kaiser]

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)

Comment 2

[James Ross]

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.

Comment 3

[James Ross]

Attachment: Static nsIXULAppInfo implkementation

Comment 4

[Robert Kaiser]

(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)

Comment 5

[James Ross]

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.

Comment 6

[James Ross]

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.

Comment 7

[James Ross]

Attachment: nsIXULAppInfo for suite

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 8

[[e-mail]]

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)
$^ > $@

Comment 9

[Robert Kaiser]

Attachment: patch v2: move over to suite/

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 ;-)

Comment 10

[Robert Kaiser]

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)

Comment 11

[Robert Kaiser]

Attachment: patch v3: should actually work correctly

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.

Comment 12

[Robert Kaiser]

Attachment: patch v4: reuse toolkit's IDL file

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.

Comment 13

[Benjamin Smedberg]

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...

Comment 14

[Benjamin Smedberg]

Oh, and the nsIXULAppInfo.idl rule should be at the end of the makefile (after
rules.mk).

Comment 15

[Robert Kaiser]

Attachment: patch v5: address bsmedberg's comments

OK, this addresses bsmedberg's comments - and also looks much nicer to myself.

Comment 16

[[e-mail]]

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?

Comment 17

[[e-mail]]

Comment on attachment 186121 [details] [diff] [review]
patch v5: address bsmedberg's comments

>+DIRS	= \
>+			components \
>+			$(NULL)
Nit: These indents are somewhat enthusiastic...

Comment 18

[Robert Kaiser]

(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 19

[Robert Kaiser]

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.

Comment 20

[chris hofmann]

Comment on attachment 186121 [details] [diff] [review]
patch v5: address bsmedberg's comments

a=chofmann

Comment 21

[Robert Kaiser]

Checked in. Thanks again to James Ross for the actual implementation!