Last Comment Bug 686466 - Get rid of application.ini in non-xulrunner applications
: Get rid of application.ini in non-xulrunner applications
Status: RESOLVED FIXED
[mobilestartupshrink][inbound]
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Mike Hommey [:glandium]
:
Mentors:
: 704230 (view as bug list)
Depends on: 687805 698493 708453 application.ini 727122 822646
Blocks: 686480 695145 695699 726978
  Show dependency treegraph
 
Reported: 2011-09-13 08:51 PDT by Mike Hommey [:glandium]
Modified: 2014-03-16 13:00 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Get rid of application.ini on Android (4.90 KB, patch)
2011-09-13 10:07 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
part 1 - Centralize application.ini creation (10.61 KB, patch)
2011-09-20 08:02 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
part 2 - Use centralized application.ini creation for mobile (5.49 KB, patch)
2011-09-20 08:02 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini (20.59 KB, patch)
2011-09-20 08:04 PDT, Mike Hommey [:glandium]
ted: review-
Details | Diff | Splinter Review
part 2 - Use centralized application.ini creation for mobile (5.57 KB, patch)
2011-10-27 05:43 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini (21.33 KB, patch)
2011-10-27 05:43 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
part 1 - Centralize application.ini creation (10.60 KB, patch)
2011-10-27 05:44 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini (21.32 KB, patch)
2011-10-27 07:34 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini (21.43 KB, patch)
2011-11-10 02:11 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini (22.21 KB, patch)
2011-11-10 03:29 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini. (22.18 KB, patch)
2011-11-10 04:57 PST, Mike Hommey [:glandium]
ted: review+
Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini. (22.21 KB, patch)
2011-11-10 08:05 PST, Mike Hommey [:glandium]
mh+mozilla: review+
Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini. (23.46 KB, patch)
2011-11-11 00:57 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini. (23.49 KB, patch)
2011-11-16 06:52 PST, Mike Hommey [:glandium]
benjamin: review+
Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2011-09-13 08:51:37 PDT
This is especially important on android where we need to unpack the file for the XRE initialization code to read it. Instead of using an application.ini file, we can hardcode the application data in the binary itself.
Comment 1 Mike Hommey [:glandium] 2011-09-13 10:07:37 PDT
Created attachment 559982 [details] [diff] [review]
Get rid of application.ini on Android

This only covers the Android part, in an ugly way
Comment 2 Mike Hommey [:glandium] 2011-09-20 08:02:17 PDT
Created attachment 561189 [details] [diff] [review]
part 1 - Centralize application.ini creation
Comment 3 Mike Hommey [:glandium] 2011-09-20 08:02:59 PDT
Created attachment 561190 [details] [diff] [review]
part 2 - Use centralized application.ini creation for mobile
Comment 4 Mike Hommey [:glandium] 2011-09-20 08:04:22 PDT
Created attachment 561191 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-09-23 11:26:07 PDT
Comment on attachment 561189 [details] [diff] [review]
part 1 - Centralize application.ini creation

Review of attachment 561189 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/Makefile.in
@@ +79,5 @@
> +DEFINES += -DGRE_MILESTONE=$(GRE_MILESTONE) -DAPP_BUILDID=$(APP_BUILDID)
> +
> +DEFINES += -DMOZ_APP_VERSION="$(MOZ_APP_VERSION)"
> +
> +MOZ_SOURCE_STAMP := $(firstword $(shell hg -R $(topsrcdir) parent --template="{node|short}\n" 2>/dev/null))

You lost a ?= here, we run this at the top-level as well and export it.

@@ +91,5 @@
> +DEFINES += -DMOZ_SOURCE_REPO="$(SOURCE_REPO)"
> +endif
> +
> +DEFINES += -DMOZ_APP_BASENAME="$(MOZ_APP_BASENAME)" \
> +           -DMOZ_APP_VENDOR="$(MOZ_APP_VENDOR)"

Reformat this to be two-space indent and everything on its own line while you're here?

::: browser/app/application.ini
@@ +1,1 @@
> +#if 0

Hah!
Comment 6 Ted Mielczarek [:ted.mielczarek] 2011-09-23 11:51:44 PDT
Comment on attachment 561190 [details] [diff] [review]
part 2 - Use centralized application.ini creation for mobile

Review of attachment 561190 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/app/Makefile.in
@@ +84,3 @@
>  APP_ICON = mobile
>  
> +DEFINES += -DAPP_NAME=$(MOZ_APP_NAME) \

Change formatting to two-space indent + one entry per line while you're here?
Comment 7 Ted Mielczarek [:ted.mielczarek] 2011-09-23 12:19:02 PDT
Comment on attachment 561191 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini

Review of attachment 561191 [details] [diff] [review]:
-----------------------------------------------------------------

I ran out of time to give this a thorough review, but we can't stop shipping application.ini without breaking a bunch of automation at the moment.

::: browser/app/nsBrowserApp.cpp
@@ +176,5 @@
>    }
>  
>    nsXREAppData *appData;
> +  int result;
> +  if (appini) {

Do we preserve the mtime of application.ini when we package it? If so, could we bake the mtime into the header file and only read the file if it's newer?

::: build/Makefile.in
@@ +71,2 @@
>  DIST_FILES = application.ini
> +endif

I'm pretty sure not shipping application.ini will break some things. I think we need to continue to ship it even if we're not reading it.

It would probably be confusing to ship it and not honor changes to it, though, hence my comment above.
Comment 8 Joel Maher ( :jmaher ) 2011-09-23 12:35:28 PDT
talos depends on this for buildid and sourcestamp.  Should we not report these values?
Comment 9 Jeff Hammel 2011-09-23 14:02:53 PDT
QA mozmill efforts also depend on application.ini
Comment 10 Mike Hommey [:glandium] 2011-09-23 14:27:06 PDT
Do they all rely on application.ini even on mobile ?
Comment 11 Jeff Hammel 2011-09-23 14:31:18 PDT
(In reply to Mike Hommey [:glandium] from comment #10)
> Do they all rely on application.ini even on mobile ?

For QA automation, I don't think so, but I'm also not in QA so I can't really speak for them
Comment 12 Mike Hommey [:glandium] 2011-10-25 01:49:09 PDT
(In reply to Ted Mielczarek [:ted, :luser] from comment #7)
> Comment on attachment 561191 [details] [diff] [review] [diff] [details] [review]
> part 3 - Use a pre-generated nsXREAppData struct instead of application.ini
> 
> Review of attachment 561191 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> I ran out of time to give this a thorough review, but we can't stop shipping
> application.ini without breaking a bunch of automation at the moment.
> 
> ::: browser/app/nsBrowserApp.cpp
> @@ +176,5 @@
> >    }
> >  
> >    nsXREAppData *appData;
> > +  int result;
> > +  if (appini) {
> 
> Do we preserve the mtime of application.ini when we package it? If so, could
> we bake the mtime into the header file and only read the file if it's newer?

More importantly, what happens to mtime when we install and upgrade it?
On Linux, tar will preserve it. On Mac, what is in the dmg is what is seen. On Windows, it would depend on either zip extraction tools or the installer itself.
For upgrades, it depends on our updater, if it preserves mtime or not.
Comment 13 Mike Hommey [:glandium] 2011-10-25 01:55:20 PDT
(In reply to Ted Mielczarek [:ted, :luser] from comment #7)
> It would probably be confusing to ship it and not honor changes to it,
> though, hence my comment above.

How about a big warning at the beginning of the file, like:
# This file is not used. If you modify it and want the application to
# use your modifications, start with the "-app /path/to/application.ini"
# argument.
Comment 14 Mike Hommey [:glandium] 2011-10-25 02:16:29 PDT
The following commit on birch clashes with part 1 here.
http://hg.mozilla.org/projects/birch/rev/76e6b7b29f81

Now, my question is, as the serverURL for crash reporter does use an id=uuid argument, without {}, how are email-like appids supposed to send their crash reports, if we ever have some?
Related question: the mobile application.ini doesn't give any arguments to the crash reporter serverURL. Would that work on all platforms?
Comment 15 Ted Mielczarek [:ted.mielczarek] 2011-10-25 05:34:18 PDT
The ServerURL bits are purely for stats, they're ignored by the server. They're just there so we can do data mining on the logs.

Your suggestion for a warning in the file sounds fine.
Comment 16 Mike Hommey [:glandium] 2011-10-27 05:43:06 PDT
Created attachment 569932 [details] [diff] [review]
part 2 - Use centralized application.ini creation for mobile
Comment 17 Mike Hommey [:glandium] 2011-10-27 05:43:47 PDT
Created attachment 569933 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini
Comment 18 Mike Hommey [:glandium] 2011-10-27 05:44:35 PDT
Created attachment 569934 [details] [diff] [review]
part 1 - Centralize application.ini creation
Comment 19 Mike Hommey [:glandium] 2011-10-27 07:34:19 PDT
Created attachment 569968 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini
Comment 20 Ted Mielczarek [:ted.mielczarek] 2011-11-07 08:50:06 PST
Comment on attachment 569934 [details] [diff] [review]
part 1 - Centralize application.ini creation

Review of attachment 569934 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/Makefile.in
@@ +70,5 @@
> +
> +ifdef LIBXUL_SDK
> +GRE_MILESTONE = $(shell $(PYTHON) $(topsrcdir)/config/printconfigsetting.py $(LIBXUL_DIST)/bin/platform.ini Build Milestone)
> +else
> +GRE_MILESTONE = $(shell tail -n 1 $(topsrcdir)/config/milestone.txt 2>/dev/null || tail -1 $(topsrcdir)/config/milestone.txt)

Should these become := while you're here?

@@ +103,5 @@
> +ifdef MOZILLA_OFFICIAL
> +DEFINES += -DMOZILLA_OFFICIAL
> +endif
> +
> +DEFINES += -DMOZ_APP_ID="$(MOZ_APP_ID)"

Move this up to the other DEFINES block?
Comment 21 Mike Hommey [:glandium] 2011-11-07 09:04:25 PST
(In reply to Ted Mielczarek [:ted, :luser] from comment #20)
> Comment on attachment 569934 [details] [diff] [review] [diff] [details] [review]
> part 1 - Centralize application.ini creation
> 
> Review of attachment 569934 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: build/Makefile.in
> @@ +70,5 @@
> > +
> > +ifdef LIBXUL_SDK
> > +GRE_MILESTONE = $(shell $(PYTHON) $(topsrcdir)/config/printconfigsetting.py $(LIBXUL_DIST)/bin/platform.ini Build Milestone)
> > +else
> > +GRE_MILESTONE = $(shell tail -n 1 $(topsrcdir)/config/milestone.txt 2>/dev/null || tail -1 $(topsrcdir)/config/milestone.txt)
> 
> Should these become := while you're here?

I thought it was better to avoid running these when we don't need the value.
Comment 22 Ted Mielczarek [:ted.mielczarek] 2011-11-07 12:19:33 PST
Comment on attachment 569968 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini

Review of attachment 569968 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Just one thing that needs fixed, and a couple of suggestions.

::: build/appini_header.py
@@ +43,5 @@
> +
> +def main(file):
> +    config = ConfigParser.RawConfigParser()
> +    config.read(file)
> +    options = []

Feels like maybe you want to use a set here? Should work exactly the same, except you use options.add('FOO').

@@ +54,5 @@
> +            options += ['NS_XRE_ENABLE_PROFILE_MIGRATOR']
> +    except: pass
> +    try:
> +        if config.getint('Crash Reporter', 'Enabled') == 1:
> +            options += ['NS_XRE_ENABLE_PROFILE_MIGRATOR']

Copy/paste error here, you used NS_XRE_ENABLE_PROFILE_MIGRATOR again.

@@ +59,5 @@
> +    except: pass
> +    if not options:
> +        options = [ '0' ]
> +    print '#include "nsXULAppAPI.h"'
> +    print 'static inline nsXREAppData *getAppData(nsILocalFile *dir = NULL) {'

nsXULAppApi says the directory field can be NULL if the xre and app are in the same dir. Can you make use of that fact and just declare a static singleton struct in this header instead of having this getter method?

@@ +61,5 @@
> +        options = [ '0' ]
> +    print '#include "nsXULAppAPI.h"'
> +    print 'static inline nsXREAppData *getAppData(nsILocalFile *dir = NULL) {'
> +    print '    static nsXREAppData appData = {'
> +    print '        sizeof(nsXREAppData),'

This whole thing feels like it might look nicer in a triple-quoted multiline string.

@@ +77,5 @@
> +    print '        "%s",' % config.get('Crash Reporter', 'ServerURL')
> +    try:
> +        print '        "%s"' % config.get('App', 'Profile')
> +    except:
> +        print '        NULL'

You could write this as:
"%s" % config.get('App', 'Profile') if config.has_option('App', 'Profile') else 'NULL'

@@ +83,5 @@
> +    print '    return &appData;'
> +    print '}'
> +
> +if __name__ == '__main__':
> +    main(sys.argv[1:])

You might want to explicitly error here if len(sys.argv) < 2.
Comment 23 Mike Hommey [:glandium] 2011-11-08 07:37:33 PST
(In reply to Ted Mielczarek [:ted, :luser] from comment #22)
> nsXULAppApi says the directory field can be NULL if the xre and app are in
> the same dir. Can you make use of that fact and just declare a static
> singleton struct in this header instead of having this getter method?

In practice, that is not true. Maybe we could fix that, though.
Comment 24 Mike Hommey [:glandium] 2011-11-10 02:11:03 PST
Created attachment 573464 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini

How about this?

Note that for appdata construction, i was originally using:
"appdata = { "%s:%s" % (s, o) : config.get(s, o) for s in config.sections() for o in config.options(s) }"

but python 2.6 chokes on it with a syntax error (while it works with 2.7).
Comment 25 Mike Hommey [:glandium] 2011-11-10 03:29:48 PST
Created attachment 573473 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini

Forgot to hg qref the final iteration.
Comment 26 Ted Mielczarek [:ted.mielczarek] 2011-11-10 04:45:45 PST
Right, that's a dict comprehension, which got added in Python 3.0 and 2.7. You can achieve the same effect by writing:
appdata = dict(("%s:%s" % (s, o), config.get(s, o) for s in config.sections() for o in config.options(s))
in earlier Python.
Comment 27 Mike Hommey [:glandium] 2011-11-10 04:57:02 PST
Created attachment 573485 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.
Comment 28 Ted Mielczarek [:ted.mielczarek] 2011-11-10 07:36:58 PST
Comment on attachment 573485 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.

Review of attachment 573485 [details] [diff] [review]:
-----------------------------------------------------------------

I'm starting to wonder if we can't pull more of those ns*App source files into a common file somewhere. We've got a lot of repeated code there.

::: build/Makefile.in
@@ +153,5 @@
>  	chmod +x $@
>  GARBAGE += leaktest.py
>  
> +ifdef MOZ_APP_BASENAME
> +application.ini: application.ini.in $(APP_INI_DEPS)

We should probably define something in rules.mk that Makefiles can use to run files through the preprocessor, like PREPROCESSOR_FILES = foo.in, and it'd generate "foo" from that. (You don't have to implement that here, maybe we should just file a followup.)

@@ +160,5 @@
> +
> +ifdef MOZ_APP_STATIC_INI
> +application.ini.h: appini_header.py application.ini 
> +	$(PYTHON) $^ > $@
> +libs:: application.ini.h

Can we generate this in export? Feels more correct, since that's when we generate headers. (Probably doesn't matter much in practice here.)

::: build/appini_header.py
@@ +63,5 @@
> +
> +    print '''#include "nsXULAppAPI.h"
> +             static const nsXREAppData sAppData = {
> +                 sizeof(nsXREAppData),
> +                 NULL,

It might be worth adding C++ comments here for what the NULL fields are. (The ones with format strings are self-evident.)

@@ +82,5 @@
> +if __name__ == '__main__':
> +    if len(sys.argv) != 1:
> +        main(sys.argv[1])
> +    else:
> +        sys.stderr.write("Usage: %s /path/to/application.ini\n" % sys.argv[0]);

You can also use print >>sys.stderr, "foo", FYI. Doesn't matter that much to me.
Comment 29 Mike Hommey [:glandium] 2011-11-10 07:57:43 PST
(In reply to Ted Mielczarek [:ted, :luser] from comment #28)
> I'm starting to wonder if we can't pull more of those ns*App source files
> into a common file somewhere. We've got a lot of repeated code there.

That's bug 658847.
Comment 30 Mike Hommey [:glandium] 2011-11-10 08:05:49 PST
Created attachment 573516 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.

Refreshed accordingly. I also renamed "options" to "flags", so as to fit the nsXREAppData field name.
Comment 31 Mike Hommey [:glandium] 2011-11-10 08:06:58 PST
Comment on attachment 573516 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.

Carrying over r+
Comment 32 Mike Hommey [:glandium] 2011-11-11 00:57:23 PST
Created attachment 573762 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.

The change to make the appdata static made android fail because what XRE_main
guesses on Android was wrong. It worked in the previous iteration because we
were giving the right XRE path. This iteration makes it so that XRE_main itself
gets the right path on Android.

Sending this review to Benjamin instead of Ted because it touches more
toolkit/xre|xpcom stuff.
Comment 33 Mike Hommey [:glandium] 2011-11-11 03:25:19 PST
Comment on attachment 573762 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.

Still fails on try, though it works locally :(
Comment 34 Mike Hommey [:glandium] 2011-11-16 06:52:46 PST
Created attachment 574890 [details] [diff] [review]
part 3 - Use a pre-generated nsXREAppData struct instead of application.ini.

This time, it works properly

https://tbpl.mozilla.org/?tree=Try&rev=6c56c8d9fa19
Comment 36 Ed Morley [:emorley] 2011-11-22 03:47:57 PST
*** Bug 704230 has been marked as a duplicate of this bug. ***
Comment 38 Mike Kaply [:mkaply] 2012-02-14 09:02:23 PST
Apologize for jumping in here, but I don't see any mention.

Some of the values in application.ini need to be set by a client on Firefox, particularly in enterprise environments.

In particular, being able to turn off the profile migrator:

EnableProfileMigrator=1

And being able to turn off crash reporting (and maybe change the reporting URL).

[Crash Reporter]
Enabled=1
ServerURL=https://crash-reports.mozilla.com/submit?id=ec8030f7-c20a-464f-9b0e-13a3a9e97384&version=10.0.1&buildid=20120208060813

Are these now hardcoded into the executable?
Comment 39 Mike Hommey [:glandium] 2012-02-14 09:32:30 PST
(In reply to Michael Kaply (mkaply) from comment #38)
> Are these now hardcoded into the executable?

Yes, they are. Modifying application.ini was a broken way to disable these, anyways. Crash reporter can be disabled with an environment variable. I don't think there is something easy to disable the profile migrator (and i don't see why one would want that), but if you need one, you can file a new bug.
Comment 40 Mike Kaply [:mkaply] 2012-02-14 09:37:01 PST
> I don't think there is something easy to disable the profile migrator (and i don't see why one would want that)

See: https://bugzilla.mozilla.org/show_bug.cgi?id=286557

I'm the one that put that EnableProfileMigrator code there in the first place
Comment 41 Ben Bucksch (:BenB) 2012-02-14 09:44:45 PST
Mike Hommey, you didn't hard code the crash reporter URL in C++, did you? Please tell me that you didn't hardcode a URL in C++.

I concur that the settings Mike mentioned must be configurable by an end-user, preferably in prefs.
Comment 42 Ted Mielczarek [:ted.mielczarek] 2012-02-14 09:47:15 PST
Yes, we currently compile all the information from application.ini into the Firefox binary. I don't think we're going to change this decision.
Comment 43 Ben Bucksch (:BenB) 2012-02-14 09:51:20 PST
Sorry, but mkaply's right. This has to be configurable. It doesn't matter what your opinion is, this is important for many usecases. Both migration and crash stuff must be optional.

And NEVER EVER EVER hardcode URLs in C++.
Comment 44 Ben Bucksch (:BenB) 2012-02-14 09:52:41 PST
Followup: bug 727122
Comment 45 Ted Mielczarek [:ted.mielczarek] 2012-02-14 09:57:11 PST
If you want it to be optional, you can provide your own builds. These are Mozilla builds, and we determine the feature set that ships with them.

Hardcoding the server URL is the only thing that makes sense. Nobody else has the debug symbols for our builds, so it doesn't make any sense for them to receive crash reports from our builds.
Comment 46 Ben Bucksch (:BenB) 2012-02-14 10:07:26 PST
> These are Mozilla builds, and we determine the feature set that ships with them.

No, sir, your ruling does not stand for 400 million people, without any possibility to be challenged.

The question wasn't just about the URL, but the ability to disable the crash reporter and the profile migration.
Comment 47 Ted Mielczarek [:ted.mielczarek] 2012-02-14 10:08:46 PST
If you have a problem with the concept of module ownership, I suggest you take that up elsewhere.
Comment 48 Ben Bucksch (:BenB) 2012-02-14 10:49:52 PST
I have a problem with module owners who are not open to reasonable arguments.
Comment 49 Don Kleppinger 2012-03-27 13:29:10 PDT
Please look at bug 723493.  This broke our enterprise web app and is preventing us from upgrading to firefox 11.  I need to be able to configure a custom vendor & name in Application.ini without having to add a command line parameter to the firefox startup.

Note You need to log in before you can comment on or make changes to this bug.