Closed Bug 1149115 Opened 8 years ago Closed 8 years ago

Breakpoint not work for sourcemapped typescript when source file and output file are not in a same directory


(DevTools :: Debugger, defect)

36 Branch
Not set


(firefox40 fixed)

Firefox 40
Tracking Status
firefox40 --- fixed


(Reporter: duanyao.ustc, Assigned: ejpbruel)


(Depends on 1 open bug, Blocks 2 open bugs)



(2 files)

Attached file
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150306140254

Steps to reproduce:

1. Compile a typescript file in current dir, say "test.ts" into "./jsgen/test.js" and "./jsgen/"
    tsc --sourcemap --out jsgen/test.js test.ts
2. Let "test_jsgen.htm" reference "jsgen/test.js"
3. Load "test_jsgen.htm", set a breakpoint in "test.ts"
4. Reload "test_jsgen.htm".

The dir structure is:

You can use the attachment to reproduce. Unpack the zip file, and open "test_jsgen.htm". 
"test_flat.htm" is a reference and works fine, because it reference "test.js" in the same dir of "test.ts".
To re-compile js and sourcemap, run "".

The version of tsc I used is

Actual results:

The breakpoint indicator slip to the last line of "test.ts" no matter which line you try to set the breakpoint on, and the breakpoint doesn't work at all.

FF 39a1 still doesn't work properly. I can set breakpoint for "test_jsgen.htm", but the debugger stops at wrong line number; I can't even properly set breakpoint in "test_flat.htm", seems a regression.

Expected results:

The breakpoint is set sucessfully, and the debugger stops at the breakpoint when page is reloaded.
Component: Untriaged → Developer Tools: Debugger
I can reproduce this bug.
Ever confirmed: true
The problem here is twofold. First, the source map library has a bug where if the source in the needle is above the source root, we cannot find its relative path correctly.

Second, because creating source actors can cause existing source actors to be replaced, we need to make sure all source actors are created before trying to reset the breakpoints. If not, the location objects in the breakpoint actors might still point to the old source actors, which point to different scripts.

I have a patch in the works that resolves these two issues, after which the breakpoint hits properly. Will put up for review here soon.
Assignee: nobody → ejpbruel
Pull request for the fixes in the source-map library:
Patch for the script.js changes. Note that this does not depend on the earlier changes in source-map, but both patches are needed in order to fix the bug.
Attachment #8601392 - Flags: review?(jlong)
Comment on attachment 8601392 [details] [diff] [review]
Make sure source actors are created before breakpoints are reset

Review of attachment 8601392 [details] [diff] [review]:

::: toolkit/devtools/server/actors/script.js
@@ +2038,5 @@
> +    // with a new one. If the source actors have not been replaced by the time
> +    // we try to reset the breakpoints below, their location objects will still
> +    // point to the old set of source actors, which point to different scripts.
> +    this.synchronize(this.sources.createSourceActors(aSource));
> +

I don't love have synchronous this is getting, but for now there's no way around it.

This was one of the reasons we stored breakpoints on generated sources, btw. We could quickly still set the breakpoint and pause even before sourcemaps were loaded. But I suppose the correct solution is to wait for sourcemaps anyway, because we want to show the original contents.

Blocking here might just be the way things have to be, alas!
Attachment #8601392 - Flags: review?(jlong) → review+
Try push for merging the changes in the source-map library with fx-team:
Whiteboard: [leave-open]
Whiteboard: [leave-open]
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Depends on: 1212696
These patches introduced a nested event loop under script compilation, which violates assumptions Gecko's scriptloader makes about compilation/execution being atomic...  See bug 1212696 comment 18.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.