Closed
Bug 893491
Opened 12 years ago
Closed 12 years ago
Statically name functions defined as this.func = function() { ... }
Categories
(Core :: JavaScript Engine, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: simon.lindholm10, Assigned: jimb)
Details
(Whiteboard: firebug-p1)
Attachments
(1 file)
2.35 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
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
Whiteboard: firebug-p1
Assignee | ||
Comment 2•12 years ago
|
||
Does this patch help?
Assignee: general → jimb
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 3•12 years ago
|
||
Any chance to get a try build? (Win 32 bit)
Honza
Comment 4•12 years ago
|
||
I would add a test for
function F() {
this.g = function () {}
}
let f = new F()
displayName(f.g)
Assignee | ||
Comment 5•12 years ago
|
||
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Tom Schuster [:evilpie] from comment #4)
Okay; I added that, and it's in the try push.
Comment 7•12 years ago
|
||
(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
Assignee | ||
Comment 8•12 years ago
|
||
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!
Assignee | ||
Comment 9•12 years ago
|
||
New try push:
https://tbpl.mozilla.org/?tree=Try&rev=6dd7435c379f
Assignee | ||
Updated•12 years ago
|
Attachment #806073 -
Flags: review?(terrence)
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 10•12 years ago
|
||
The try push looks good, so this just needs review.
Comment 11•12 years ago
|
||
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.
Attachment #806073 -
Flags: review?(terrence) → review+
Comment 12•12 years ago
|
||
I also looked at this and it seemed fine to me.
Assignee | ||
Comment 13•12 years ago
|
||
(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!
Assignee | ||
Comment 14•12 years ago
|
||
Target Milestone: --- → mozilla27
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•