Closed Bug 406205 Opened 12 years ago Closed 12 years ago

Add Watch Expression issue

Categories

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

x86
Windows XP
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: neek, Assigned: Gijs)

References

Details

(Keywords: regression)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.10) Gecko/20071115 Firefox/2.0.0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.10) Gecko/20071115 Firefox/2.0.0.1 with Javascript Debugger 0.9.87.1

Add Watch Expression is not working properly. If you already have a previous watch and try to add a new one, it doesn't prompt to enter the value, it repeats the previous watch instead. 

Reproducible: Always

Steps to Reproduce:
1. Right click Add Watch Expression and add an expression/value
2. Right click again and Add Watch Expression and add an expression/value



Expected Results:  
It always should prompt to enter the expression.
Confirming, assigning to me, pretty sure this used to work, major problem -> sev: cri.
Assignee: rginda → gijskruitbosch+bugs
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Brilliant. Got this figured out. Now we need a fix... :\
Depends on: 385004
So the trick is that the 'expression' argument conflicts with the context menu expression property which is set to whatever the expression/variable is that's selected in the list you're using the context menu on (watches, locals). Oops. Fixing this by changing the watch expression var, not the other one because it's used all over the darn codebase and it might regress more stuff.
Attachment #290917 - Flags: review?(ajvincent)
Also, lowering to major because you can work around this pretty easily by using the interactive session rather than the context menu.
Severity: critical → major
Just to let you know, if I understood your last comment correctly, it is actually NOT necessary for the am item in the watch list to be selected in order for this bug to occur.  I mention this because of your statement "whatever the expression/variable is that's selected in the list".  Case in case this helps you can also get around the problem by erasing all Watches and then adding a new one.
Comment on attachment 290917 [details] [diff] [review]
Use different name for watch expression argument

Looks harmless enough.
Attachment #290917 - Flags: review?(ajvincent) → review+
(In reply to comment #5)
> Just to let you know, if I understood your last comment correctly, it is
> actually NOT necessary for the am item in the watch list to be selected in
> order for this bug to occur.  I mention this because of your statement
> "whatever the expression/variable is that's selected in the list".  Case in
> case this helps you can also get around the problem by erasing all Watches and
> then adding a new one.
> 

Right clicking anywhere in the list will automatically select *something*, as far as I know. And this patch fixed the issue for me, so:

Checking in mozilla/extensions/venkman/resources/content/venkman-views.js;
/cvsroot/mozilla/extensions/venkman/resources/content/venkman-views.js,v  <--  venkman-views.js
new revision: 1.40; previous revision: 1.39
done
Checking in mozilla/extensions/venkman/resources/locale/en-US/venkman.properties;
/cvsroot/mozilla/extensions/venkman/resources/locale/en-US/venkman.properties,v  <--  venkman.properties
new revision: 1.70; previous revision: 1.69
done


I'd like to fix the other issue you reported as well before doing another release, but if you'd like a new version that incorporates this fix, either check out the code yourself or send me mail (and I'll mail you an updated xpi file). Thanks for reporting the issue! :-)
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Duplicate of this bug: 407510
Thanks for looking into this.

Sorry if I don't understand these fixes/workarounds.  I can choose *any* watch expression and the problem happens.  Erasing all watch expressions and adding a new one *does* work, but then you can only have one watch expression at a time which does not fix the issue that the information for the watched expression is wrong.  The interactive session is still usable if you know what the structure of the watch expression should be, but not helpful if you are looking at someone else's code and it *might* not be obvious what the structure of the variable is.
The watch is expression is *much* handier if you are looking at a complex, deeply nested structure.

I will wait for a complete fix release before using any new code.  Thanks again for looking into this as Venkman is invaluable and still does things that Firebug does not.
(In reply to comment #9)
> Thanks for looking into this.
> 
> Sorry if I don't understand these fixes/workarounds.  I can choose *any* watch
> expression and the problem happens.  Erasing all watch expressions and adding a
> new one *does* work, but then you can only have one watch expression at a time
> which does not fix the issue that the information for the watched expression is
> wrong.  The interactive session is still usable if you know what the structure
> of the watch expression should be, but not helpful if you are looking at
> someone else's code and it *might* not be obvious what the structure of the
> variable is.
> The watch is expression is *much* handier if you are looking at a complex,
> deeply nested structure.
> 
> I will wait for a complete fix release before using any new code.  Thanks again
> for looking into this as Venkman is invaluable and still does things that
> Firebug does not.

Venkman 0.9.87.2 was pushed public on addons.mozilla.org a few hours ago, and in that build this bug should be fixed.

Product: Other Applications → Other Applications Graveyard
You need to log in before you can comment on or make changes to this bug.