Make SetInitAtStartup actually work

RESOLVED FIXED

Status

Other Applications
Venkman JS Debugger
--
critical
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Gijs, Assigned: Karsten Düsterloh)

Tracking

Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

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?
Flags: blocking1.9.1?

Comment 1

8 years ago
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.
Doesn't block, would take fix with tests.
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
(Assignee)

Comment 3

8 years ago
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...
(Assignee)

Comment 4

8 years ago
Created attachment 368110 [details] [diff] [review]
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.
Assignee: nobody → mnyromyr
Attachment #368110 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 5

8 years ago
As a sidenote: we still need to fix bug 364864.
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?
(Assignee)

Comment 7

8 years ago
> 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 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...)
Attachment #368110 - Flags: review?(gijskruitbosch+bugs) → review+
Duplicate of this bug: 123958
(Assignee)

Comment 10

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

Landed as <http://hg.mozilla.org/venkman/rev/9fea2b31c556>.
Attachment #368110 - Attachment description: always on with reevaluation → always on with reevaluation [checked in]
Blocks: 483282
Attachment #368110 - Attachment description: always on with reevaluation [checked in] → always on with reevaluation [Checkin: Comment 10]
What's left to be done here?
Status: NEW → ASSIGNED
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
One nit: all command line handlers get reinvoked for remoting (always on Windows; not sure how Mac and Linux work here.)
You need to log in before you can comment on or make changes to this bug.