483681 - Make SetInitAtStartup actually work

Status: RESOLVED FIXED

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[:Gijs (he/him)]

Spun off from bug 483282. It turns out SetInitAtStartup actually doesn't work because of bug 364864. We should fix that. What I'm thinking of is the following:

1. Always register for autoreg and app-startup.
2. Keep a pref for whether it should actually start work as soon as possible.
3. Check that pref when the autoreg and app-startup events actually come in.

In this case, we can map the Get/SetInitAtStartup code to the pref, I believe. Timeless, does that sound sane to you?

Comment 1

[timeless]

i really really want bug 364864 to be fixed.

otherwise, changing the code to always have the category (since we don't have a choice) and use a pref is fine with me.

however, i'd appreciate it if we note somewhere to people that they must use the api and not the pref. This probably involves a comment before the pref in all.js and a pref name intentionally picked so that people do not use it directly.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Doesn't block, would take fix with tests.

Comment 3

[Karsten Düsterloh]

So I toyed around a bit in jsdASObserver::Observe (which is said app-startup observer), but I can't read user-set prefs there - it seems to be called too early in the startup process (maybe not even a profile yet?).
I can read app default pref values (including those from /bin/extensions/*), but that isn't particularly useful...

Comment 4

[Karsten Düsterloh]

Attachment: always on with reevaluation [Checkin: Comment 10]

We can't read profile prefs on component registration.
We can't read profile prefs on app startup.
Thus we can't fix the original problem in JSD.

We can read profile prefs on commandline processing, though.
Thus we can avoid the perf hit for the host by investing a few cycles on startup:
- on component registration, always register the app-startup observer
- if the the app-startup observer is registered, let it always turn on JSD
- in the commandline handler (where we already may open Venkman's main window!), check extensions.venkman.jsd.initAtStartup and posiibly correct catman registration and/or JSD state.

Comment 5

[Karsten Düsterloh]

As a sidenote: we still need to fix bug 364864.

Comment 6

[:Gijs (he/him)]

I don't understand what this patch fixes. It doesn't change the actual /startup-init command, so it doesn't fix that. It doesn't fix the underlying issue, which you says is still required for this patch to do something. It doesn't fix the method mentioned in the bug summary... so what *does* it do?

Help?

Comment 7

[Karsten Düsterloh]

> It doesn't fix the underlying issue, which you says is still required for
> this patch to do something.

No, it's not required for this, but it still needs fixing.

This patch enables host applications to override/control Venkman's startup registration by using a pref.

Comment 8

[:Gijs (he/him)]

Comment on attachment 368110 [details] [diff] [review]
always on with reevaluation
[Checkin: Comment 10]

(In reply to comment #7)
> > It doesn't fix the underlying issue, which you says is still required for
> > this patch to do something.
> 
> No, it's not required for this, but it still needs fixing.
> 
> This patch enables host applications to override/control Venkman's startup
> registration by using a pref.


OK, this makes sense. r=me. Please remember to check in on HG, not on CVS (the patch is from CVS, so...)

Comment 10

[Karsten Düsterloh]

Comment on attachment 368110 [details] [diff] [review]
always on with reevaluation
[Checkin: Comment 10]

Landed as <http://hg.mozilla.org/venkman/rev/9fea2b31c556>.

Comment 11

[Serge Gautherie (:sgautherie)]

What's left to be done here?

Comment 12

[:Gijs (he/him)]

I'm going to mark this fixed. JSD can't be fixed for the reasons outlined in comment #3 and comment #4, actually making the catmanager work is bug 364864, making Venkman work without JSD starting up on startup at all is bug 483685, making SeaMonkey use this pref to not have JSD startup if Venk is installed at least until it's started at least once is bug 483282, and so I don't see anything to be done in *this* bug. We've got enough other ones open to deal with all the other issues.

Comment 13

[[e-mail]]

One nit: all command line handlers get reinvoked for remoting (always on Windows; not sure how Mac and Linux work here.)