1488654 - Debugger doesn't find the corresponding sources for event handlers

Status: NEW

(DevTools :: Debugger, defect, P3)

Description

[Brian Birtles (:birtles)]

I hope this bug report is going to be useful somehow. Yesterday I was debugging a Web compat issue and I thought I'd try using the debugger to set a breakpoint in onclick event handlers for a particular element. I ran into a lot of problems including the debugger just becoming unresponsive until I restarted Firefox.

Today I went to try and reproduce that issue but I couldn't even work out how I succeeded in setting a breakpoint in the function. Following is a video of my attempt:

  https://youtu.be/m80s_I-AwkE

I think there are a number of bugs here but I'm not sure which are already filed/worth filing.

Specifically:

A. When jumping to an event callback it doesn't seem to find the correct line.
B. The "highlight" of the current line is too short and subtle. Could we just
   always highlight the current line (e.g. like Notepad++)?
C. I seem to need to pretty print once, then go back to the inspect and re-jump
   to the appropriate line. Is there some way to avoid this?
D. There is no indication pretty printing is in progress so I'm never sure if
   I actually correctly triggered the action or not (on my computer yesterday
   pretty printing would take > 10s)
E. Is there someway to always pretty print?
F. Even after pretty printing, it doesn't seem to jump to the appropriate line?
G. Is there any reason we don't always do "Reveal in Tree"?
H. Can we hide the non-pretty printed versions from the buffer list? I found
   that very confusing (I was sure there was a bug there but while making this
   video I realized both versions appear there).
I. Can we set a max-width on the editor tabs? I seem to only ever be able to see
   one tab at a time when docking to side?
J. For some reason at the end of the video the breakpoint isn't hit. That might
   not be related to debugger at all though (perhaps the other event handler
   called stopPropagation etc.)

Sorry, that's a massive list of issues and no doubt some of them are known issues or already filed but hopefully there's something there that's actionable. I'm happy the file the necessary bugs if you let me know which are useful.

Comment 1

[Julian Descottes [:jdescottes]]

Thanks for filing! Lots of good bugs on that website.
Let's try to categorize and split this:

1. wrong event location (A+F)

Looks like there are issues with finding event sources on this specific website: https://m.traveloka.com/en-id/flight
The "jumping" works correctly, but the inspector fails to identify the correct location for the events. 
This might be an issue with our devtools/server/actors/inspector/event-parsers.js ? I will log a separated bug here on bugzilla. I assume this refers to the "line 11" in mobile-vendors... For the record, Chrome DevTools show the same file+line, so something on that website must be weird...

2. pretty printing issues
  - (C) Need to go back to inspector then jump again. 

That's tricky. Right now the inspector sends the user the a "line" in the debugger, not a line+column or an instruction. So from a debugger standpoint, there is no way to know which line to select after pretty printing. Maybe we can complexify our viewSourceInDebugger helper in devtools/client/shared/view-source.js ?

  - (E) add feature to always pretty print? 
  - (H) allow to hide pretty printed files

3. ux issues
  - (D) show indicator when pretty printing is in progress
  - (B) highlight is too subtle
  - (I) max-width of editor tabs
  - (G) always reveal in tree

4. breakpoint issue

  (J) There are several issues here. I am not sure where and when the function you set breakpoints on is supposed to be called. I tried to compare with Chrome devtools, but they are just as confused as Firefox DevTools with this website. So maybe the method is never called, maybe the breakpoint is not hit. Maybe pretty print is the issue? I know debugger team was working on breakpoints and stepping quality, and they can share some insights.

Comment 2

[Julian Descottes [:jdescottes]]

"Right now the inspector sends the user the a "line" in the debugger" -> " Right now the inspector sends a "line" to the debugger" ...

Comment 3

[Julian Descottes [:jdescottes]]

(In reply to Julian Descottes [:jdescottes][:julian] from comment #1)
> 4. breakpoint issue
> 
>   (J) There are several issues here. I am not sure where and when the
> function you set breakpoints on is supposed to be called. I tried to compare
> with Chrome devtools, but they are just as confused as Firefox DevTools with
> this website. So maybe the method is never called, maybe the breakpoint is
> not hit. Maybe pretty print is the issue? I know debugger team was working
> on breakpoints and stepping quality, and they can share some insights.

Jason, can you share anything about breakpoint and stepping quality (known issues that can be followed on Github for instance?)

Comment 4

[:Harald Kirschner :digitarald]

Those seem mostly known issues that we are aiming to address in 65 by either replacing or fixing the debugger's pretty print library.

B: https://github.com/devtools-html/debugger.html/issues/4100

>  - (C) Need to go back to inspector then jump again. 

Thanks for investigating. Looks like you might have found another root cause from what I have heard so far on what the issue might be; that is great!

For G, H, I we should get github issues filed; but I let Jason jump in before as I might have missed bugs.

Comment 5

[Julian Descottes [:jdescottes]]

For the record I logged 

https://github.com/devtools-html/debugger.html/issues/6948 for ux issues

https://github.com/devtools-html/debugger.html/issues/6947 for pretty print features

Bug 1489413 to work on passing a column when jumping to debugger

Comment 6

[Jason Laster [:jlast]]

D. There is no indication pretty printing is in progress so I'm never sure if
   I actually correctly triggered the action or not (on my computer yesterday
   pretty printing would take > 10s)

> good point.

E. Is there someway to always pretty print?

> auto pretty print is 90% done, and will be prioritized in 65.

F. Even after pretty printing, it doesn't seem to jump to the appropriate line?

> we are working on PP in 65

G. Is there any reason we don't always do "Reveal in Tree"?

> we had this, but it was slow. we should re-visit :)

H. Can we hide the non-pretty printed versions from the buffer list? I found
   that very confusing (I was sure there was a bug there but while making this
   video I realized both versions appear there).

> it can be nice to jump back and forth, but i agree the UX can be confusing

I. Can we set a max-width on the editor tabs? I seem to only ever be able to see
   one tab at a time when docking to side?

> hmm, i think this makes sense. lets discuss in the github issue

J. For some reason at the end of the video the breakpoint isn't hit. That might
   not be related to debugger at all though (perhaps the other event handler
   called stopPropagation etc.)

> hmm, we should check. sometimes if the PP mapping is bad it can place the breakpoint in the wrong spot