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)
SeaMonkey
UI Design
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: kairo, Assigned: kairo)
Details
Attachments
(1 file, 5 obsolete files)
10.32 KB,
patch
|
benjamin
:
review+
neil
:
superreview+
chofmann
:
approval1.8b3+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•20 years ago
|
||
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•20 years ago
|
||
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•20 years ago
|
||
Updated•20 years ago
|
Attachment #184126 -
Attachment mime type: application/octet-stream → application/x-javascript
Assignee | ||
Comment 4•20 years ago
|
||
(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•20 years ago
|
||
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•20 years ago
|
||
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•20 years ago
|
||
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•20 years ago
|
||
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)
$^ > $@
Assignee | ||
Updated•19 years ago
|
Flags: blocking-seamonkey1.0a?
Assignee | ||
Comment 9•19 years ago
|
||
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)
Assignee | ||
Comment 10•19 years ago
|
||
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)
Updated•19 years ago
|
Flags: blocking-seamonkey1.0a? → blocking-seamonkey1.0a+
Assignee | ||
Updated•19 years ago
|
Attachment #185964 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #185964 -
Flags: review?(neil.parkwaycc.co.uk)
Assignee | ||
Comment 11•19 years ago
|
||
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 | ||
Updated•19 years ago
|
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)
Assignee | ||
Comment 12•19 years ago
|
||
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)
Assignee | ||
Updated•19 years ago
|
Attachment #185990 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #185990 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 13•19 years ago
|
||
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-
Comment 14•19 years ago
|
||
Oh, and the nsIXULAppInfo.idl rule should be at the end of the makefile (after
rules.mk).
Assignee | ||
Comment 15•19 years ago
|
||
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)
Updated•19 years ago
|
Attachment #186121 -
Flags: review?(benjamin) → review+
Comment 16•19 years ago
|
||
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 17•19 years ago
|
||
Comment on attachment 186121 [details] [diff] [review]
patch v5: address bsmedberg's comments
>+DIRS = \
>+ components \
>+ $(NULL)
Nit: These indents are somewhat enthusiastic...
Assignee | ||
Comment 18•19 years ago
|
||
(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.
Assignee | ||
Comment 19•19 years ago
|
||
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 20•19 years ago
|
||
Comment on attachment 186121 [details] [diff] [review]
patch v5: address bsmedberg's comments
a=chofmann
Attachment #186121 -
Flags: approval1.8b3? → approval1.8b3+
Assignee | ||
Comment 21•19 years ago
|
||
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.
Description
•