243362 - Copy Qualified name needs to do some escaping

Status: RESOLVED FIXED

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

Description

[timeless]

consider:

a={"test-case":0}

copy qualified name results in:

this.a.test-case

Caught exception: “SyntaxError: syntax error”

Comment 1

[James Ross]

Since the Copy qualified Name function litterally just sticks the expression
into the clipboard, I think we need the escaping around line 799/808 in here:

http://lxr.mozilla.org/mozilla/source/extensions/venkman/resources/content/venkman-views.js#787

I would suggest anything that contains non-alphanumerics, or doesn't start with
a letter, should be escaped into a ["..."] format (with \" escaping). Would this
be suitable?

Comment 2

[timeless]

spidermonkey actually has a way to do this stuff. iirc brendan poked things that
messed up near the decompiler recently.

Comment 3

[Brendan Eich [:brendan]]

Not escaping, quoting and indexing (a["test-case"], not a.test-case).  But since
Venkman is doing its own "decompiling", it will have to handle the
non-identifier case -- there's nothing SpiderMonkey can do to help here, short
of exposing native helper functions, which doesn't seem like a good trade to me.

/be

Comment 4

[Robert Ginda]

I've been away from this for a while, but what about just changing:
  cx.expression = cur.displayName + "." + cx.expression;
to
  cx.expression = cur.displayName + "[" + cx.expression.quote() + "]";
?

Comment 5

[James Ross]

Attachment: Escape expressions used for Copy Qual Name

This changes the way cx.expression is generated so that items are escaped as
needed. It matches numbers (for foo[42]), 'words' (for foo.bar) and everything
else is fully escaped (for foo[".!%"]).

I also added support for the two ECMA properties, __proto__ and __parent__.

Comment 6

[Robert Ginda]

Comment on attachment 150573 [details] [diff] [review]
Escape expressions used for Copy Qual Name

r=rginda

Comment 7

[Brendan Eich [:brendan]]

__proto__ and __parent__ are SpiderMonkey extensions, not part of ECMA-262.

/be

Comment 8

[James Ross]

Checked in.