257162 - Enable standalone XUL applications via XULRunner

Status: VERIFIED FIXED

(Toolkit Graveyard :: XULRunner, enhancement, P1)

Description

[Darin Fisher]

Enable standalone XUL applications via XULRunner.

I will be using this bug to track landing the XULRunner development branch on
the trunk.  I will be attaching patches to this bug for the various affected
modules.

Comment 1

[Darin Fisher]

Attachment: v1 - chrome registry changes

This patch enables support for multiple "app"-level chrome directories.  It
adds the new directory key: NS_APP_CHROME_DIR_LIST, which corresponds to a list
of alternate chrome directories.  It supplements the default NS_APP_CHROME_DIR.


installed-chrome.txt is read from each extra chrome directory, and added to the
per-profile RDF chrome data source.  This seemed better than trying to make the
chrome registry aggregate a new data source for each chrome directory.

This patch also includes some 'carried-away' cleanup to the chrome protocol
handler.

Comment 2

[Darin Fisher]

Attachment: v1 - widget changes for XUL window icons

This patch changes nsIWidget::SetIcon to take a string identifier instead of a
resource:// URL.  This allows the widget code to search multiple locations for
the icon(s) matching the given string identifier.  This patch causes the widget
code to search all of the application chrome directories for
"icons/default/$id.$ext"

We could extend this API in the future to support stock GNOME icons as well.  I
discussed this change with blizzard and the design is based on our discussions.


nsXULWindow.cpp is the only caller of SetIcon.

The widget implementations were anyways stripping the resource://chrome/
prefix, and manually talking to the directory service to find
NS_APP_CHROME_DIR, so this patch just avoids the unnecessary overhead of
passing a URL that is mostly ignored.

NS_APP_CHROME_DIR_LIST is searched ahead of NS_APP_CHROME_DIR so that XUL apps
can override builtin window icons if desired.

Comment 3

[Darin Fisher]

Attachment: v1 - toolkit and resource: changes

This patch includes changes to nsAppRunner.cpp to support a new command line
argument, -app that can be used to specify a .moz file that is an INI file
containing fields corresponding to the members of nsXREAppData.

With this change, any toolkit based application will respond to a -app command
line switch.

Additional directories will be added to the following directory service keys:

  NS_APP_CHROME_DIR_LIST	  - $app/chrome
  NS_APP_PREFS_DEFAULTS_DIR_LIST  - $app/defaults/pref
  NS_XPCOM_COMPONENT_DIR_LIST	  - $app/components

Default preferences in $app/defaults/pref can be used to override
browser.chromeURL to the appropriate default chrome URL for the app.

$app is derived from the location of the .moz file.  The .moz file is assumed
to live in the $app directory, so the chrome, components, and defaults
directories will be assumed to be found in there.

This patch also introduces resource://app/, which corresponds to the directory
containing the .moz file, or $app as I have been calling it here.  To support
this new substitution, nsResProtocolHandler::GetSubstitution is modified such
that if a substitution is not known, the directory service is queried for the
key "resource:$root", where $root is "app" in this case.  nsXREDirProvider.cpp,
which implements nsIDirectoryServiceProvider, returns $app when queried for
"resource:app".

While resource:// URLs are not limited to corresponding to file:// URLs, I felt
like it was a natural thing to bind unknown resource: mapping to the directory
service instead of introducing some additional callback mechanism.  Oh, I
should have said... it also turned out to be quite inconvenient to try to call
nsIResProtocolHandler::SetSubstitution at the right times during startup (more
than one place would have been required, had to deal with chrome registration
happening during xpcom startup, etc.).

Comment 4

[Darin Fisher]

NOTE: none of these patches are specific to a XULRunner executable.  In fact,
with these patches alone, it is possible to use Firefox as XULRunner.  XULRunner
itself is really a matter of better factoring, packaging, and installation foo.

To make XULRunner a reality, we need to eliminate #ifdef MOZ_PHOENIX and friends
that appear in core gecko and toolkit code.  We also need to decide on what is
"in" the core XULRunner (i.e., how do we define the XUL runtime?).

Comment 5

[Benjamin Smedberg]

> To make XULRunner a reality, we need to eliminate #ifdef MOZ_PHOENIX and friends
> that appear in core gecko and toolkit code.  We also need to decide on what is
> "in" the core XULRunner (i.e., how do we define the XUL runtime?).

Heh, this is the hard part ;) Actually at this point, MOZ_THUNDERBIRD ifdefs
cause more grief than MOZ_PHOENIX/MOZ_XUL_APP.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 157193 [details] [diff] [review]
v1 - widget changes for XUL window icons

+++ widget/src/xpwidgets/nsBaseWidget.cpp	18 Aug 2004 00:06:32 -0000     
1.126.4.1
+ * Modifies aDir to point at icon file with given name and extension.
+ * Returns true if the icon file exists and can be read.
+ */
+static PRBool
+ResolveIconNameHelper(nsILocalFile *aFile,

there is no aDir :)

ideally this comment would mention that aIconExt must include the dot

Comment 7

[Darin Fisher]

Thanks biesi!

Comment 8

[Benjamin Smedberg]

Comment on attachment 157192 [details] [diff] [review]
v1 - chrome registry changes

> nsChromeProtocolHandler::GetScheme(nsACString &result)
> {
>-    result = "chrome";
>+    result.AssignLiteral("chrome");

If you really are thinking about the aviary branch, you can't use the new
*Literal methods.

>+static nsresult
>+GetFileAndLastModifiedTime(nsIProperties *aDirSvc,
>+                           const char *aDirKey,
>+                           const nsCSubstring &aLeafName,
>+                           nsILocalFile **aFile,
>+                           PRTime *aLastModified)
>+{
>+  *aLastModified = LL_ZERO;
>+
>+  nsresult rv;
>+  nsCOMPtr<nsILocalFile> file;
>+  rv = aDirSvc->Get(aDirKey, NS_GET_IID(nsILocalFile), getter_AddRefs(file));
>+  if (NS_FAILED(rv))
>+    return rv;
>+  rv = file->AppendNative(aLeafName);
>+  if (NS_SUCCEEDED(rv)) {
>+    // if the file does not exist, this returns zero
>+    file->GetLastModifiedTime(aLastModified);
>+    file.swap(*aFile);
>+  }
>+  return rv;

Why do you need the extra COMPtr here? (just use aFile throughout). Or, what I
have done in some places, make aFile a nsCOMPtr<nsILocalFile> &aFile.

> NS_IMETHODIMP
> nsChromeRegistry::CheckForNewChrome()
>+    // we assume that any other list files will only reference chrome that
>+    // should be installed in the profile directory.  we might want to add
>+    // some actual checks to this effect.

Why do we assume that? Maybe we're making a bad assumption about where the
chrome.rdf and overlayinfo cache are going to live for xulrunner apps? IMO they
should live in the "resource://app/" directory, not the resource:/// directory.
When CheckForNewChrome is called, we probably don't have a current profile (or
at least you can't assume that we have a profile). Answer me this, otherwise
this looks good.

Comment 9

[Benjamin Smedberg]

Comment on attachment 157195 [details] [diff] [review]
v1 - toolkit and resource: changes

>+static nsXREAppData *LoadAppData(const char *appDataFile)

I don't like the style, can it be nsXREAppData* LoadAppData  ?

>+{
>+  static nsXREAppData data;
>+  static char vendor[256], name[256], version[32], buildID[32], copyright[256];

copyright[512] perhaps?

>+  parser.GetString("Main", "Vendor", vendor, sizeof(vendor));
>+  parser.GetString("Main", "Name", name, sizeof(name));
>+  parser.GetString("Main", "Version", version, sizeof(version));
>+  parser.GetString("Main", "BuildID", buildID, sizeof(buildID));
>+  parser.GetString("Main", "Copyright", copyright, sizeof(copyright));

Need error-checking here. If the name doesn't exist, INIParser doesn't null-out
the buffer, and I presume you would want to bail anyway in some cases:

Name/Version are required
Vendor/BuildID/Copyright are optional, I think

I'm a bit worried about forwards-compatibility, especially if this lands for
aviary... maybe we can include a magic INI key of some sort, to prevent "old"
versions of the xulrunner from running new/incompatible XUL apps, e.g.:
MinVersion=2 would cause us to bail. (Assuming ffox1.0 is "1", and
libxul/xulrunner is "2", and allowing for decimals.

>+  {
>+    nsCOMPtr<nsILocalFile> lf;
>+
>+    if (appDataFile) {
>+      NS_GetFileFromPath(appDataFile, getter_AddRefs(lf));
>+      lf->SetNativeLeafName(NS_LITERAL_CSTRING(""));

Whoa, that can't be right. Maybe you mean lf->GetParent ?

Comment 10

[Benjamin Smedberg]

Comment on attachment 157195 [details] [diff] [review]
v1 - toolkit and resource: changes

>+static nsXREAppData *LoadAppData(const char *appDataFile)
>+{
>+  static nsXREAppData data;
>+  static char vendor[256], name[256], version[32], buildID[32], copyright[256];

Oh, and you can save some code here:

>+  data.appVendor = vendor;
>+  data.appName = name;
>+  data.appVersion = version;
>+  data.appBuildID = buildID;
>+  data.copyright = copyright;
>+  data.useStartupPrefs = PR_FALSE;

static char vendor[256], name[256], version[32], buildID[32], copyright[256];
static nsXREAppData data = { vendor, name, version, buildID, copyright };

Comment 11

[Benjamin Smedberg]

and you can actually make "data" const once you do that ;)

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 157193 [details] [diff] [review]
v1 - widget changes for XUL window icons

Index: widget/src/gtk/nsWindow.cpp
+  NS_ConvertUCS2toUTF8 iconKey(aIconSpec);

can you make this UTF16 instead of UCS2 while you're here?

+    ResolveIconName(aIconSpec, NS_LITERAL_STRING("16.xpm"),
+		     getter_AddRefs(iconFile));

hm, passing "16.xpm" as extension is slightly abusing the API I guess, but this
is in the same module, so ok.


+      printf("Looking for small icon file: %s\n", path.get());
+      printf("Loaded small icon file: %s\n", path.get());
they seem to be now in the same if, so either none of them or both will be
printed. is it useful to have them both, then?

Index: widget/src/windows/nsWindow.cpp

-    nsCAutoString cPath; cPath.AssignWithConversion(iconPath);

thanks! :)

   HICON bigIcon = (HICON)::LoadImageW( NULL,

hm... wonder why  this works on win9x... didn't think it had this function

Index: widget/src/xpwidgets/nsBaseWidget.cpp

+ * Resolve the given icon name into a local file object.  This method is

hm... can you mention that it also checks whether the file exists? that wasn't
obvious to me at first.

is there a reason to put this comment into the .cpp file instead of the .h
file?

Comment 13

[Darin Fisher]

>    HICON bigIcon = (HICON)::LoadImageW( NULL,
> 
> hm... wonder why  this works on win9x... didn't think it had this function

The code tries LoadImageW first, and then falls back to LoadImageA if the
GetLastError() returns ERROR_CALL_NOT_IMPLEMENTED.

Comment 14

[Darin Fisher]

> is there a reason to put this comment into the .cpp file instead of the .h
> file?

convention.  the header file doesn't document any of the other methods.  any
documentation appears to be in the .cpp file.

Comment 15

[Darin Fisher]

Thanks bsmedberg and biesi for the reviews!


> Why do we assume that? Maybe we're making a bad assumption about where the
> chrome.rdf and overlayinfo cache are going to live for xulrunner apps? IMO 
> they should live in the "resource://app/" directory, not the resource:/// 
> directory. When CheckForNewChrome is called, we probably don't have a current 
> profile (or at least you can't assume that we have a profile). Answer me this, 
> otherwise this looks good.

I discussed this issue with bsmedberg over IRC.  Here's the deal:

By putting chrome.rdf (and overlayinfo) for xulapps in the profile directory, we
allow the user to create them.  This is good because it means thatapp developers
neither have to include chrome.rdf in the installation package nor provide some
sort of installation process that generates chrome.rdf when the xulapp is
installed.  As for profile selection details, toolkit/xre/nsAppRunner.cpp
ensures that a profile exists before invoking application logic.  It either
shows the profile selection dialog, uses the default profile, or expects a
profile to be selected on the command line.  A xulapp should not need to show
chrome before this process occurs.  Moreover, chrome.rdf is just a cache, so we
could move it in the future if appropriate.


> Name/Version are required

Well, the code seems to require all of the fields if you count DumpVersion. 
But, otherwise only Name and BuildID (not Version) are required.  I'll add code
to require those fields and use an empty string for the others when they don't
appear in the INI file.

Comment 16

[Darin Fisher]

Attachment: v1.1 patch - combined

OK, here's the revised patch for the trunk.

Comment 17

[Darin Fisher]

Comment on attachment 157837 [details] [diff] [review]
v1.1 patch - combined

widget/ and xpfe/ changes since they have already been reviewed by biesi and
blizzard.

Comment 18

[Darin Fisher]

> widget/ and xpfe/ changes since they have already been reviewed by biesi and
> blizzard.

er... prefix that with "You can ignore"

Comment 19

[Benjamin Smedberg]

Comment on attachment 157837 [details] [diff] [review]
v1.1 patch - combined

Please file a followup bug about the windows DDE mutex string and window class
name.

Comment 20

[Darin Fisher]

> Please file a followup bug about the windows DDE mutex string and window class
> name.

see bug 258217.

Comment 21

[Darin Fisher]

v1.1 patch checked in on trunk

Comment 22

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 157837 [details] [diff] [review]
v1.1 patch - combined

a=[e-mail] valid for today only, skip the CPH changes since they're not
necessary - if it causes hell we can yank it quickly. The areas being touched
are CR, nsAppRunner, ResHandler and Window

Comment 23

[Ben Goodger (use ben at mozilla dot org for email)]

Oops. Darin will land this after 1.0PR - sometime next week. Setting blocking1.0
flag. 

Comment 24

[Jan Varga [:janv]]

I copied the sample app and tried this:
./firefox -app /home/varga/myapp/myapp.moz
it didn't work first time, I had to add [XRE] to myapp.moz
[XRE]
Version=0.1

[Main] has been renamed to [App]

ok, now it works but it uses /home/varga/.firefox to store myapp's profiles
it would be better to use /home/varga/.myvendor/myapp IMHO

Comment 25

[Jan Varga [:janv]]

This happens only when Firefox uses old profile scheme e.g. ~/.firefox
I fixed my profiles manually and now it works (almost).
Anyway, there are other problems like bug 258470 and bug 258476.

Comment 26

[Darin Fisher]

(In reply to comment #25)
> This happens only when Firefox uses old profile scheme e.g. ~/.firefox
> I fixed my profiles manually and now it works (almost).
> Anyway, there are other problems like bug 258470 and bug 258476.
> 

Yikes!  What sample are you using?  The format of the INI file changed since I
blogged about this stuff.  Moreover, the sample app under mozilla/xulrunner has
not been updated!

Comment 27

[Jan Varga [:janv]]

(In reply to comment #26)
> Yikes!  What sample are you using?  The format of the INI file changed since I
> blogged about this stuff.  Moreover, the sample app under mozilla/xulrunner has
> not been updated!

I updated it according to the code in nsAppRunner.cpp :)
Anyway, those problems with old profile locations and bug 258470 seem to be valid.

Comment 28

[Stefan Borggraefe]

This caused Bug 258473.

Comment 29

[Darin Fisher]

Attachment: build fu to enable xulrunner executable

Comment 30

[Darin Fisher]

Comment on attachment 158285 [details] [diff] [review]
build fu to enable xulrunner executable

This is likely to evolve significantly over time.  Consider this an initial
stab at carving out XULRunner.

This accompanies the code that is already on the trunk under mozilla/xulrunner.

Comment 31

[Benjamin Smedberg]

Comment on attachment 158285 [details] [diff] [review]
build fu to enable xulrunner executable

>--- client.mk	5 Sep 2004 19:39:02 -0000	1.212
>+++ client.mk	9 Sep 2004 07:04:42 -0000

I think it would make much more sense to just pull mozilla/xulrunner with
mozilla/toolkit (have everyone pull this little bit of code), instead of adding
a separate option. If you really want to go with the patch you have, you need
to add XULRUNNER_CO_TAG to the TAG list near the top of the file.

>Index: Makefile.in

>+ifdef MOZ_XULRUNNER
>+tier_99_dirs	+= xulrunner xpfe/bootstrap/init.d
>+endif

The init.d stuff isn't going to work with xulrunner, and I don't want to
pretend to support it. I told gisburn as much when he added it (that it was
only good as a temporary hack).

>Index: xpfe/Makefile.in

> ifdef MOZ_PHOENIX
> DIRS += \
>         browser/public \
>         browser/src \
>         $(NULL)
> else
>+ifdef MOZ_XULRUNNER
>+DIRS += \
>+        browser/public \
>+        browser/src \
>+        $(NULL)

Makefile trick: ifneq (,$(MOZ_PHOENIX)$(MOZ_XULRUNNER))


>Index: xpfe/browser/src/Makefile.in


> ifdef MOZ_PHOENIX
> REQUIRES += toolkitcomps history
>+else
>+ifdef MOZ_XULRUNNER
>+REQUIRES += toolkitcomps history
>+endif
> endif

Same

>Index: xpfe/components/Makefile.in

> ifdef MOZ_PHOENIX
> # Firefox uses this flag to stop the automatic processing of the jar.mn file
> # that lives parallel to this Makefile. The jar.mn is responsible for the 
> # packaging of a large number of chrome files Firefox does not need. 
> NO_DIST_INSTALL=1
> NO_INSTALL=1
> endif
>+ifdef MOZ_XULRUNNER
>+NO_DIST_INSTALL=1
>+NO_INSTALL=1
>+endif

Same

>+ifdef MOZ_XULRUNNER
>+DIRS += bookmarks/public
>+endif

Ick, this hurts, can you make sure there is a followup bug on the toolkit
dependencies on nsIBookmarksService or whatever needs this?

Comment 32

[Darin Fisher]

bsmedberg: thanks for all the tips!  i applied your suggested fixes, and checked
this patch in on the trunk.  as for xpfe/components/bookmarks/public, that is
required for xpfe/components/search, which is currently built by firefox (and
hence, i'm currently building it for xulrunner).  in the future, we might end up
not building the search component -- hard to say right now.

Comment 33

[Benjamin Smedberg]

*** Bug 206358 has been marked as a duplicate of this bug. ***

Comment 34

[Nigel McFarlane]

This is great work, thank you. Now that its cooking, here's my 2c.
Some feedback and simple observations from the land of technology uptake.

My interest is seeing this technology become user-friendly so that
mere humans can play with it. As easy as XML/Scripting for Beginners. Hope
these notes help. I'm focussing here on xulrunner, not XRE.

It's easy to be disgusted by newbies. The problem is that everyone's
a newbie sometime, and newbies will always outweigh experts in number.
Newbie coders are the real case; expert coders are the rare case.
Even experts appreciate simple corner-cutting features.

*Initial* use of this feature set should be smooth and easy, or else
people will reject the idea of a xulrunner AND we'll be back in
the complexity we are currently in with -chrome AND they'll never
graduate to the more complete features offered.

If this is to be widely successful, then new programmers
need to be able to use it "out of the box" with limited
config. That requires fallback cases that apply if the
newbie programmer does nothing.

I ask you to cast your minds back to your first C program.
You didn't start with Makefiles or CVS. You just typed:

cc test.c
./a.out

Perl was even easier.

Turning this logic on app/xulrunner:

- If the newbie types moz -app [url|path|filename] and nothing
  else, what then?

  Something sane should happen. A window should appear.

- Could NS_APP_CHROME_DIR_LIST be NS_APP_CHROME_PATH
  or something equivalent? People know what a path is.

  I have in mind that when the newbie sets up his environment,
  he's going to want to set maybe 2 environment variables at
  most. (zero ideally). So could there be a $MOZILLA_CHROME_PATH
  envvar that overrides (or defaults to) NS_APP_CHROME_PATH?

  Already the newbie must set maybe LD_LIBRARY_PATH (or equiv),
  maybe MOZILLA_FIVE_HOME and maybe PATH. That is, if they just
  want to experiment in their home directory "normally and safely".
  A MOZILLA_CHROME_PATH should only have to be touched in
  intermediate cases. "." should also be in the default chrome path.
  I think for bundled Fedora installs, et al, MOZILLA_CHROME_PATH
  would be the first non-default envvar a newbie needs. But if the
  newbie pulls down a "recent" version, and installs it in /foo/bar
  then it gets harder straight away and MOZILLA_CHROME_PATH really
  needs to default sensibly.

  The idea of a .ini file for XRE is a good intermediate solution,
  but not for beginners. Please no .ini for xulrunner.

  The idea of requiring installed-chrome.txt edits is a post-beginner
  solution. If that file is missing, xulrunner should scan the
  chrome top directories for package names and deduce the (probably only one)
  package the user's trying to make. There should also be a default
  package name (called default?) for $PWD.

  Similarly a chrome.rdf or contents.rdf is way too hard for beginners.
  Beginners are going to be stressed just identifying that they need
  a content/skin/locale split. At least they can drop the skin & locale
  features til later, leaving an (alas meaningless) "content" directory.
  Please don't make them build a contents.rdf file.

  For xulrunner, if there's no .rdf file in "." or in "./<package>/content",
  xulrunner should try to auto-generate the needed RDF facts. They can
  be deduced from the above scan. Since overlays aren't beginner stuff,
  that much at least isn't required, only ordinary top-level package
  detection. Maybe a regxpcom-style solution?

Can I also observe (for brendan) that a chrome-interpretation phase,
whether scanning or merely reading existing constructed information, is
a "baking" kind of process that is preparatory to any actual app code
interpretation. It moves moz app development workflow a bit towards a
compile-and-run model, and a bit away from "pure" JIT-interpret-on-the-spot
model. That's an architectural question that needs a viewpoint, since
all the chrome RDF baking stuff is a performance and complexity cost.

From the point of view of beginners, either that baking process needs
to stay completely hidden, or else it should be ditched (no chrome).
I think in Perl, the site_perl stuff is only engaged once the coder
starts using the 'use' or 'require' keywords. That doen't happen at
step 1. And in perl, hacking site_perl is only for highly organised
intermediate perl programmers. Not that moz is anything like perl ;-).
If you look also at the example of Java, the baking process is still out
of the way of beginners, but for any sizeable programming task, the
baking process (making EARs, etc) is still entirely formal and necessary.
Not that mozilla is anything like Java either ;-).

I'll log bugs for any of the points above that survive feedback &
discussion here. Or we can go to the NG (if only we had one).

- N.

Comment 35

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #34)
> - If the newbie types moz -app [url|path|filename] and nothing
>   else, what then?
> 
>   Something sane should happen. A window should appear.

well, of course? why do you think anything else would be needed?

> - Could NS_APP_CHROME_DIR_LIST be NS_APP_CHROME_PATH
>   or something equivalent? People know what a path is.

Why, this is purely a mozilla-internal.

>   I have in mind that when the newbie sets up his environment,

It's not an environment variable.

>   Already the newbie must set maybe LD_LIBRARY_PATH (or equiv),

this should be done by a ./xulrunner script, like with ./mozilla on unix.

>   Similarly a chrome.rdf or contents.rdf is way too hard for beginners.

No chrome.rdf is needed, see xulrunner/examples/simple

Comment 36

[Nigel McFarlane]

comment 35 doesn't really help, alas.

- I'm aware of the difference between internal directory
  defines and envvars. I'm pointing out there's value in
  exposing this config via an envvar.

- Currently (or else I'm behind) a window won't appear if
  the -chrome / -app argument is a filename. It has to be a URL.

- A startup script won't help a user that's got a custom install.
  My point is that envvars are a config hurdle that needs
  consideration for newbies.

- I take your point on chrome.rdf. My point, however, is that
  xulrunner/examples/simple is simpler but still not at all simple.

Can we have a more considered review of comment 34 from the bug owner
or other lurkers? There's no other forum that springs to mind
without a newsgroup being created.

- N.

Comment 37

[Benjamin Smedberg]

Nigel, we can discuss this in npm.seamonkey or npd.xul, but not here. If you
want something much simpler than xulrunner/samples/simple then it's going to
take a long time to rearchitect the various pieces. In particular something that
has *no* configuration files at all is at this point infeasible, because of the
way we handle profiles and chrome registration. I have a plan, in 1.9a, to
reduce the complexity of the installed-chrome.txt/contents.rdf so that no RDF is
needed.

Note that the xulrunner is a very different beast than the -chrome flag. You are
not passing a URI, you are passing the filename of your app.xulapp file.

What is this "user that has a custom install" mumbo-jumbo? The idea is that you
install the xulrunner (through an RPM on most linux systems), and xulrunner is
in your path. You don't have to think about the fact that at this point in time
it's a shell script.

Comment 38

[Nigel McFarlane]

Mumbo jumbo is my job - sigh.

According to comment 3, -app works for Fx, Tb, MAS. In my view
newbies will try out XUL from that angle. Fx will always be more
popular than xulrunner. Think MS Excel. RPM is way too obscure
for most newbies.

To summarize my position, I think the chrome init code
could *deduce* config files for simple beginner cases,
now that we have multi-tier chrome support.

For this bug, my comments amount to a request that $PWD be
auto-added to the list of alternate chrome directories, and that
$PWD be treated to special-case chrome registration.

For more text -> n.p.d.xul as Benjamin recommended. See

<news://news.mozilla.org:119/ciap4i$[e-mail]>

- N.

Comment 39

[James Ross]

I don't know if this is the best place, or if I should file a new bug, but
there's a bunch of binary files in mozilla/xulrunner/app that don't have -kb
set, meaning that they are completely corrupted on Windows. The ones I've had a
problem with are xulrunner.ico (which breaks the build!), document.ico and
document.png. I would guess the os2 versions of the icos are similarily affected.

Comment 40

[Benjamin Smedberg]

We've decided that this is too much to take on aviary, pushing out.

Comment 41

[Darin Fisher]

Attachment: Implement gecko version checking (part 1)

Implement gecko version checking (part 1):

 - move LoadAppData into nsXULRunnerApp.cpp
 - remove [XRE]/Version
 - add [Gecko]/{MinVersion,MaxVersion} where MinVersion is required
 - add comments to simple.xulapp

Some info about XULRunner version checking here:
http://wiki.mozilla.org/index.php/XUL:Xul_Runner_Versioning

Comment 42

[Darin Fisher]

Comment on attachment 169147 [details] [diff] [review]
Implement gecko version checking (part 1)

bsmedberg: note, i decided to not bother support the '+' form of maxVersion. 
instead, 1.8 will match 1.8.2 as well as 1.8b and the like.

Comment 43

[Benjamin Smedberg]

Comment on attachment 169147 [details] [diff] [review]
Implement gecko version checking (part 1)

>Index: xulrunner/app/nsXULRunnerApp.cpp

>+static PRUint32 geckoVersion = 0;

It seems a little wasteful that we have to do this at runtime. Is there any
chance we can do it at build-time instead?

>+static PRUint32 ParseVersion(const char *versionStr)
>+{
>+  PRUint16 major, minor;
>+  if (PR_sscanf(versionStr, "%hu.%hu", &major, &minor) != 2) {

If we feed this a string like 1.8a or 1.8.3, this will still work correctly and
get "1.8", correct?

>+  if (!hasAppOption)
>+  {
>+    // fixup argv to start with -app.
>+    char **argv2 = NULL;
>+    if (argv[1][0] != '-')

Do we need to check "/" for win32?

>Index: xulrunner/examples/simple/Makefile.in

>+# generate various fields for simple.xulapp
> BUILD_ID=`date +%""Y%""m%d%H`
>+GECKO_VERSION=`echo $(MOZILLA_VERSION) | sed 's|\(^[0-9]*\.[0-9]*\).*|\1|'`

If we accept "1.8a5" etc, why not use the version string directly, instead of
munging int?

>Index: xulrunner/examples/simple/simple.xulapp

>+; This field specifies your organization's name.  This field is optional.

recommended, but optional.

Comment 44

[Darin Fisher]

> It seems a little wasteful that we have to do this at runtime. Is there any
> chance we can do it at build-time instead?

Yeah, I wasn't too worried about optimizing that but I suppose we could do
something at build time.


> If we feed this a string like 1.8a or 1.8.3, this will still work correctly 
> and get "1.8", correct?

Yes, I verified that it works.


> >+  if (!hasAppOption)
> >+  {
> >+    // fixup argv to start with -app.
> >+    char **argv2 = NULL;
> >+    if (argv[1][0] != '-')
> 
> Do we need to check "/" for win32?

Well... in that case, hasAppOption would be true.  Hmm... which means that
checking argv[1][0] here is just redundant.  That was leftover from the old
code.  I'll remove the |if| check.


> If we accept "1.8a5" etc, why not use the version string directly, instead of
> munging int?

We don't accept "[Gecko]/MinVersion=1.8a5"

The idea is for XUL apps to specify compatability granularity of major.minor and
no more.

It's an implementation detail that "1.8a5" would be treated as "1.8" :-/


> recommended, but optional.

Yeah, I'll make that change.  Thanks for the quick review!

Comment 45

[Darin Fisher]

Attachment 169147 [details] [diff] has been committed to the trunk.  I also renamed xulrun.exe to
xulrunner.exe under Windows (and OS/2) since there's no reason for the
abbreviated executable name.

Comment 46

[Darin Fisher]

-> Toolkit : XULRunner

Comment 47

[Darin Fisher]

I think we can close this bug as fixed.  There is still work to be done to make
XULRunner more usable, but in its current state it is definitely usable.

Comment 48

[Benjamin Smedberg]

See also bug 302101 - Xulrunner 1.8 tracking bug and bug 299986 - xulrunner 1.9
tracking bug