Open Bug 743341 Opened 12 years ago Updated 2 years ago

[jsdbg2] Debugger.prototype.findScript's 'innermost' query property is not what's needed

Categories

(Core :: JavaScript Engine, defect)

defect

Tracking

()

People

(Reporter: jimb, Unassigned)

References

(Blocks 1 open bug)

Details

The 'innermost' query property for Debugger.prototype.findScripts should be dropped.

'Innermost' was meant to help find the right script in situations like this:

function f() {             // line 1
  x = function g() {       // line 2
    // break here          // line 3
  }
}

When setting a breakpoint at "break here", one wants to set a breakpoint in g's script, but not in f's. Passing 'innermost:true' in findScripts' query argument directs findScripts to return only the innermost script covering the given url and line, in this case g's. That's fine.

But what if one wants to set a breakpoint at the line *before* "break here"? If I say findScripts({url:..., line:2, innermost:true}), then findScripts, as specified will return only g's script --- even though f contains executable code on that line! Imagine if that line read: "Array.map(a, function g() {"; it's crazy not to be able to set a breakpoint before the call to Array.map.

Of course, you can; you just need to not use 'innermost' to do the query. The JavaScript needs to get all the scripts covering line 2, use getLineOffsets to decide which scripts have code *on* that line, and then set breakpoints on all of them.

Part of what makes 'innermost' useless is that we're querying by line, not by line and column. (We don't have column numbers yet.) If we were querying by line and column, the ambiguities mentioned above wouldn't arise.

Because the number of scripts covering a given url and line is not going to be particularly big, it should be completely fine to drop 'innermost' altogether (this will simplify findScripts), and let the caller do that kind of further processing. If there were an accessor for scripts' lexical depth, that would be helpful, too.
Depends on: 743351
Assignee: general → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.