1046389 - Breakpoint slides to wrong line

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Brian Grinstead [:bgrins]]

1) Open: http://www.landmarker.io/?server=demo
2) Open debugger, viewport.js
3) Set a breakpoint on line 199
4) It moves up to line 147

Reported on 31, I can confirm in an fx-team build on OSX

See also:
https://twitter.com/jamesabooth/status/494479899194048512
https://twitter.com/jamesabooth/status/494481648508235776

Comment 1

[Girish Sharma [:Optimizer]]

Able to confirm on latest nightly on windows as well.

Most probably related to source mapping

Comment 2

[Brian Grinstead [:bgrins]]

Also, when switching away from and back to the file I saw an error (though I can't reproduce it now):

TypeError: info is null debugger.js:184

http://dxr.mozilla.org/mozilla-central/source/browser/devtools/sourceeditor/debugger.js#184

Comment 3

[James Booth]

Haven't been able to reproduce myself, but a colleague confirmed the problem exists on Linux (Firefox 31, Ubuntu 14.04). After refreshing the page after reproducing the bug, he saw the following error:

A script on the page has stopped responding chrome://browser/content/devtools/codemirror/codemirror.js:3978

Don't know if that is any help. 

landmarker.io is my webapp, let me know if I can help in any way.

Comment 4

[Tom Tromey :tromey]

A bit more data on this.

Setting the breakpoint on 199 makes it slide to line 147.
However, if you look at the list of sources, you'll see that the breakpoint
is actually set in some other source.
And, if you switch to this source and then switch back to viewport.js, the
breakpoint marker will no longer be visible.

Supporting bits from the RDP trace:

DBG-SERVER: Packet 68 sent to "conn0.child7/source54"
DBG-SERVER: Received packet 68: {
  "to": "conn0.child7/source54",
  "type": "setBreakpoint",
  "location": {
    "line": 147
  }
}
DBG-SERVER: Packet 69 sent from "conn0.child7/source54"
DBG-SERVER: Received packet 69: {
  "error": "noCodeAtLineColumn",
  "actor": "conn0.child7/breakpoint59",
  "actualLocation": {
    "source": {
      "actor": "conn0.child7/source55",
      "url": "http://www.landmarker.io/Users/jab08/gits/landmarker.io/src/js/index.js",
      "addonID": null,
      "addonPath": null,
      "isBlackBoxed": false,
      "isPrettyPrinted": false,
      "introductionUrl": null,
      "introductionType": "scriptElement"
    },
    "line": 147
  },
  "from": "conn0.child7/source54"
}

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

Ouch, this is really bad. The main problem is that the breakpoint sliding code doesn't abort when it detects that the actualLocation contains a different original source URL. In such a situation we can not offer a meaningful magic behavior to the user, so we should just store the breakpoint and move on.

The frontend could also have the same assertion in place, but it's really the backend's job to handle it properly and always return actualLocations with the same URL.

Comment 6

[Eddy Bruel [:ejpbruel]]

(In reply to Panos Astithas [:past] from comment #5)
> Ouch, this is really bad. The main problem is that the breakpoint sliding
> code doesn't abort when it detects that the actualLocation contains a
> different original source URL. In such a situation we can not offer a
> meaningful magic behavior to the user, so we should just store the
> breakpoint and move on.
> 
> The frontend could also have the same assertion in place, but it's really
> the backend's job to handle it properly and always return actualLocations
> with the same URL.

How would we ever end up with an actual location with a different source url? The current breakpoint sliding algorithm uses findScripts with a given source url to find the scripts on which we attempt to slide.

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

findScripts() works on the generated sources, not on the original ones. Breakpoint sliding however affects the original sources (in this case at least, and in general when the source-mapping pref has not been disabled). 

The simple solution I described above to abort sliding, is just the easiest one. The best would be to have the sliding code consider original sources, but I think that would be harder to get right and more error-prone.

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

I am currently working on implementing shorlander's line gutter mockup in bug 762979. I am wondering if we want to keep the sliding effect since it is a bit more difficult to get the sliding effect going through the line numbers.

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

You mean the animation effect? That is separate from the actual breakpoint sliding, which was present long before James implemented the nice animation. I'm not the right person to gauge how difficult the work to keep the effect is, but "a bit more difficult" doesn't sound deterring enough ;-)

Comment 10

[Eddy Bruel [:ejpbruel]]

Putting looking into this on my todo list for tomorrow.

Comment 11

[Jason Laster [:jlast]]

This now works.