893491 - Statically name functions defined as this.func = function() { ... }

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Simon Lindholm]

From http://code.google.com/p/fbug/issues/detail?id=6574:

function F() {
  this.g = function() {
    return new Error().stack;
  };
}
new F().g()

should give a nice, static name of this.g that contains the string "g". Currently it's empty.

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Here is yet another test case (+instructions) that shows the problem.
https://getfirebug.com/tests/manual/issues/6574/test.html

Jim, any chance we could get this?

Should I CC yet somebody else?

Note that it's also a problem for built-in debugger.

Honza

Comment 2

[Jim Blandy :jimb]

Attachment: this-displayName.patch

Does this patch help?

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Any chance to get a try build? (Win 32 bit)

Honza

Comment 4

[Tom S. (please needinfo tschuster)]

I would add a test for 
function F() {
  this.g = function () {}
}
let f = new F()
displayName(f.g)

Comment 5

[Jim Blandy :jimb]

Try: https://tbpl.mozilla.org/?tree=Try&rev=b2a01df1b164

Comment 6

[Jim Blandy :jimb]

(In reply to Tom Schuster [:evilpie] from comment #4)

Okay; I added that, and it's in the try push.

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jim Blandy :jimb from comment #5)
> Try: https://tbpl.mozilla.org/?tree=Try&rev=b2a01df1b164
Yeah, works for me, thanks Jim!
Honza

Comment 8

[Jim Blandy :jimb]

Interesting; browser/base/content/test/general/browser_tabopen_reflows.js matches function names appearing in Error stacks, so that will need to be updated. But nothing else!

Comment 9

[Jim Blandy :jimb]

New try push:
https://tbpl.mozilla.org/?tree=Try&rev=6dd7435c379f

Comment 10

[Jim Blandy :jimb]

The try push looks good, so this just needs review.

Comment 11

[Terrence Cole [:terrence]]

Comment on attachment 806073 [details] [diff] [review]
this-displayName.patch

Review of attachment 806073 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not really the right reviewer for this, but as far as I can tell from looking at blame, there is no right reviewer for this. You have a working test and the code looks reasonable, however, so r=me.

Comment 12

[Tom S. (please needinfo tschuster)]

I also looked at this and it seemed fine to me.

Comment 13

[Jim Blandy :jimb]

(In reply to Terrence Cole [:terrence] from comment #11)
> I'm not really the right reviewer for this, but as far as I can tell from
> looking at blame, there is no right reviewer for this. You have a working
> test and the code looks reasonable, however, so r=me.

Thanks for the review!

Comment 14

[Jim Blandy :jimb]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b69adde6dd8e

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b69adde6dd8e