The default bug view has changed. See this FAQ.

NS_ERROR_NOT_IMPLEMENTED when opening venkman

VERIFIED FIXED

Status

Other Applications
Venkman JS Debugger
--
blocker
VERIFIED FIXED
6 years ago
6 years ago

People

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

Tracking

({regression})

Trunk
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
Since bug 595243 landed, venkman is not usable anymore. While I understand that the focus is on firefox and therefore firebug/chromebug, this does not work for other Mozilla Apps.

Please at least fix up venkman so it works again.
(Reporter)

Comment 1

6 years ago
More info, sorry:

When opening venkman, a dialog shows up that it could not start the debugger service, since jsdIDebuggerSerive::On() now throws NS_ERROR_NOT_IMPLEMENTED. It seems now that instead a debugMode needs to be enabled, see bug 595243.

After clicking ok, the debugger window opens, but everything is empty except the toolbar and menu.

Comment 2

6 years ago
you need to use .asyncOn(...)

http://mxr.mozilla.org/mozilla-central/ident?i=asyncOn
(Reporter)

Comment 3

6 years ago
Created attachment 493000 [details] [diff] [review]
Fix - v1

This fixes the initial startup, but still some files are not shown. I guess thats a different issues though.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #493000 - Flags: review?
(Reporter)

Updated

6 years ago
Attachment #493000 - Flags: review? → review?(gijskruitbosch+bugs)

Comment 4

6 years ago
this isn't backwards compat.

What you should do is something like:

if ("asyncOn" in jsd) {
  jsd.asynOn(this);
} else {
  jsd.on();
  this.onDebuggerActivated();
}

and stick the rest of your code into this.onDebuggerActivated.
(Reporter)

Comment 5

6 years ago
Created attachment 493111 [details] [diff] [review]
Fix - v2

You're right, I guess I should keep it backwards compatible. This patch takes care and also aligns the style to whats in the files already. Note there is no explicit this object there, but I put the code into an extra function.
Attachment #493000 - Attachment is obsolete: true
Attachment #493111 - Flags: review?(gijskruitbosch+bugs)
Attachment #493000 - Flags: review?(gijskruitbosch+bugs)

Updated

6 years ago
Duplicate of this bug: 608685

Comment 7

6 years ago
+++ b/resources/content/tests/eval.xul	Wed Nov 24 22:47:02 2010 +0100

arguably this should also support the old way :)
Blocks: 595243
No longer depends on: 595243
Duplicate of this bug: 615106
Duplicate of this bug: 622867
(In reply to comment #7)
> +++ b/resources/content/tests/eval.xul    Wed Nov 24 22:47:02 2010 +0100
> 
> arguably this should also support the old way :)

Philipp, may I suggest fixing the test and requesting review from timeless? Gijs seems to be busy.
(Reporter)

Comment 11

6 years ago
Created attachment 506340 [details] [diff] [review]
Fix - v3

Ah yes, sorry. Totally forgot about this bug. Thanks for the note!
Attachment #493111 - Attachment is obsolete: true
Attachment #506340 - Flags: review?
Attachment #493111 - Flags: review?(gijskruitbosch+bugs)
(Reporter)

Updated

6 years ago
Attachment #506340 - Flags: review? → review?(timeless)

Comment 12

6 years ago
Comment on attachment 506340 [details] [diff] [review]
Fix - v3

(In reply to comment #11)
> Created attachment 506340 [details] [diff] [review]
> Fix - v3
> 
> Ah yes, sorry. Totally forgot about this bug. Thanks for the note!

Two points. First, AFAICT there are two instances where you changed to an async pattern (not counting the test) and one has actual code in the callback listener (the dump statement is less important...).

What if the first bit of code turns on JSD (on startup), will the callback still fire once you call asyncOn a second time (from venkman-debugger.js) ? The code in that callback does need to be executed. Please test/verify this, and if not, make sure the code runs if jsds.isOn (it may be better to just run the code directly if jsds.isOn(), and only run .on()/.asyncOn() otherwise).


Second, initDebugger is expected to be synchronous. If you check the caller, there is some (but not all!) code there (in venkman-static.js) that expects the debugger is initialized (from the looks of it, at least the (delayed) initPost, the venkman-started hook, etc.). It would be best to use a callback structure (pass a function to initDebugger, have onDebuggerActivated call it) to make sure this code doesn't fire until the debugger is actually active.


With the callback and ensuring the hooks get added even if JSDS is turned on at startup, r=me.
Attachment #506340 - Flags: review?(timeless) → review+
(Assignee)

Comment 13

6 years ago
Created attachment 512915 [details] [diff] [review]
v4: sync asyncOn

(Stealing this with Fallen's permission.)

(In reply to comment #12)
> What if the first bit of code turns on JSD (on startup), will the callback
> still fire once you call asyncOn a second time (from venkman-debugger.js)?

No, it won't. 
Fixed.

> Second, initDebugger is expected to be synchronous. 
...
> It would be best to use a callback structure

Fixed.

Venkman still dumps some JS errors on startup, but those don't seem to be related with this bug.
Assignee: philipp → mnyromyr
Attachment #506340 - Attachment is obsolete: true
Attachment #512915 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 512915 [details] [diff] [review]
v4: sync asyncOn

Drive-by review:

>+        // (initAtStartup was removed by changeset 46998/bug 568691/2010-06-10)
Changeset numbers are local and unportable.
Could try linking to a rev on hg.mozilla.org though.

>+                jsds.asyncOn({ onDebuggerActivated: onDebuggerActivated });
jsdIActivationCallback has recently been made a [function], so you can just pass onDebuggerActivated directly.

>+          jsds.asyncOn({ onDebuggerActivated: function() {} });
You can also pass null for no callback. (Maybe it should be optional?)

>-    console.executionHook = { onExecute: jsdExecutionHook };
>+        console.executionHook = { onExecute: jsdExecutionHook };
[Is a -w diff possible at all?]

Comment 15

6 years ago
Comment on attachment 512915 [details] [diff] [review]
v4: sync asyncOn

With the nits noted by Neil, r=me.
Attachment #512915 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 16

6 years ago
Created attachment 513265 [details] [diff] [review]
v5: final version

This got pushed as <http://hg.mozilla.org/venkman/rev/65c7be10defd>, with Neil's nits fixed (I dropped the changeset part completely).
Attachment #512915 - Attachment is obsolete: true
Attachment #513265 - Flags: review+
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Comment 17

6 years ago
Karsten, thanks for taking care!
Checked out latest source, built XPI, and tested in Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110217 Firefox/4.0b12pre ID:20110217123839. Works like a charm. Thanks Karsten for fixing that! Marking as verified fixed.
Status: RESOLVED → VERIFIED

Comment 19

6 years ago
could someone please update and release the extension on AMO..
(Assignee)

Comment 20

6 years ago
Note that this patch does only work with Firefox trunk builds, but *not* with FF4 beta builds up to beta 11! 
(The function parameter change was only recently 2011-02-14 in bug 633833, well after beta 11 was released.)
(Assignee)

Comment 21

6 years ago
s/this patch/Venkman with this patch/
Duplicate of this bug: 642182
Duplicate of this bug: 645037
So, the dupe count increases. Someone with appropriate rights should update the maxVersion on AMO as a minimum, or better still release a fixed version with maxVersion bumps to 4.0.* (Firefox) and 2.1.* (SeaMonkey).

Comment 25

6 years ago
I've set the max version for JavaScript Debugger 0.9.88.1 to Firefox 4.0b6 (i.e. beta 6) which I think is the last one that worked; if not, just say and I can adjust it, but it at least isn't claiming 4.0 final compatibility now.

Updated

6 years ago
Duplicate of this bug: 645053

Updated

6 years ago
Duplicate of this bug: 645374

Updated

6 years ago
Duplicate of this bug: 658492
You need to log in before you can comment on or make changes to this bug.