Summary: | Web Inspector: Source links in Event Listeners panel are missing for bound functions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||
Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | graouts, inspector-bugzilla-changes, jonowells, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=143862 | ||||||||
Attachments: |
|
Created attachment 256743 [details]
Partial Patch
These code changes are part of a fix for this issue, but this will drill down to the original function and ignore all steps in between (like the source code location where the function is bound) and that may not actually be what we want.
After discussion with Devin, I think what we really want is for getFunctionDetails to provide the complete chain of bound functions. For instance a bound function's target function may itself be a bound function. You could imagine a chain like this: 1 function addArguments() { 2 var sum = 0; 3 for (var x of arguments) { sum += x; } 4 return sum; 5 } 6 var foo = addArguments.bind(null, 1); 7 var bar = foo.bind(null, 2); Where logging `bar` has a chain like this: JSBoundFunction (name=bound bound addArguments, location=script.js:7, boundThis=null, boundArguments=[1], scope=...) JSBoundFunction (name=bound addArguments, location=script.js:6, boundThis=null, boundArguments=[2], scope=...) JSFunction (name=addArguments, location=script.js:1, scope=...) The frontend should probably get a list of function details about each of these functions in the chain and decide how to display that information. You could think of this as a call stack for the creation of this function and each of its pieces. Note this chain is always one directional, it cannot be cyclic. ----- The same data can be useful for backtraces which include bound functions. For example this test: <script> function foo() { bar.bind(null)(); } function bar() { ({}).x.x; } foo(); </script> Currently Produces: [Error] TypeError: undefined is not an object (evaluating '({}).x.x') bar (test.html:2) bar foo (test.html:1) (anonymous function) (test.html:3) There are a couple things wrong with this. It should produce: [Error] TypeError: undefined is not an object (evaluating '({}).x.x') bar (test.html:2) bound bar (test.html:1) foo (test.html:1) (global code) (test.html:3) Note the "bound bar" being the location of the .bind() invocation. It seems JavaScriptCore is not stashing the location this function was created. ----- So I think in fixing this we should consider consistent displaying of Functions given that any function may be bound and have a chain of origin. Having bound functions in backtraces would be great! As for the UI for the Event Listener panel, we should incorporate stacktrace popovers (https://bugs.webkit.org/show_bug.cgi?id=143862) once we have them. I think, having a link to the original function is a desired behavior. Hovering over that link should bring a stacktrace with bound functions along with regular call frames. *** Bug 139380 has been marked as a duplicate of this bug. *** (In reply to comment #2) > Created attachment 256743 [details] > Partial Patch > > These code changes are part of a fix for this issue, but this will drill > down to the original function and ignore all steps in between (like the > source code location where the function is bound) and that may not actually > be what we want. Can we land Devin's patch for now and implement everything you described a bit later? Seeing "(anonymous function)" for every bound function is really not helpful. (In reply to comment #6) > (In reply to comment #2) > > Created attachment 256743 [details] > > Partial Patch > > > > These code changes are part of a fix for this issue, but this will drill > > down to the original function and ignore all steps in between (like the > > source code location where the function is bound) and that may not actually > > be what we want. > > Can we land Devin's patch for now and implement everything you described a > bit later? Seeing "(anonymous function)" for every bound function is really > not helpful. I think fixing this correctly is not that hard, someone just has to do it. The existing patch is an excellent starting point. I think the source location where the function is bound is equally as important as knowing which function it was bound to. |
Created attachment 256677 [details] [Image] No source links We use Function.prototype.bind for event listener handlers universally in Web Inspector. Event Listeners panel without the source links is pretty useless.