813456 - Display visual cues to make setting breakpoints more discoverable

Status: RESOLVED INVALID

(DevTools :: Debugger, defect, P1)

Description

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

In the last work week we had mentioned that it wasn't obvious that clicking on the gutter sets a breakpoint, and that showing an empty circle on hover would help make that more obvious.

Comment 1

[Victor Porof [:vporof][:vp]]

I would be happy if hovering the gutter would highlight the corresponding line.

Comment 2

[Paul Rouget [:paul]]

FYI, during the holidays, 2 people sent me messages (twitter) asking how to add breakpoints in the debugger. Adding breakpoint is such an important action, it really should be discoverable.

Comment 3

[Victor Porof [:vporof][:vp]]

My 2c: I think the biggest issue here is that clicking on the line numbers in the gutter doesn't add a breakpoint (expected behavior in all cases IMHO), but jumps to that line. Iirc there's bug 762979 for that.

Mihai, will the following Orion update address these issues? Or, alternatively, provide a clean way of specifying the desired behavior on gutter click/ line hover?

CodeMirror (bug 816756) gives us the expected gutter-click behavior for free. http://codemirror.net/demo/marker.html

Comment 4

[Mihai Sucan [:msucan]]

(In reply to Victor Porof [:vp] from comment #3)
> My 2c: I think the biggest issue here is that clicking on the line numbers
> in the gutter doesn't add a breakpoint (expected behavior in all cases
> IMHO), but jumps to that line. Iirc there's bug 762979 for that.
> 
> Mihai, will the following Orion update address these issues? Or,
> alternatively, provide a clean way of specifying the desired behavior on
> gutter click/ line hover?
> 
> CodeMirror (bug 816756) gives us the expected gutter-click behavior for
> free. http://codemirror.net/demo/marker.html

The expected gutter-click behavior is for free with Orion as well. The problem with that was back in the early days when I implemented the feature - I made a UX mistake then. It is up to us (in Source Editor) how we hook click event handlers for the gutter.

We kept postponing fixes due to Web Console work, and because we also wanted to do the new line gutter UI that shorlander suggested.

Comment 5

[Mihai Sucan [:msucan]]

Just opened bug 826388 and submitted a patch.

Comment 6

[Victor Porof [:vporof][:vp]]

(In reply to Mihai Sucan [:msucan] from comment #5)
> Just opened bug 826388 and submitted a patch.

Thank you, Mihai!

Comment 8

[Paul Rouget [:paul]]

During the App Day, a lot of people thought the debugger didn't have any breakpoint capabilities because of this.

This was a P3, but I allow myself to bump this bug into a P1. From my personal experience, I see too many people misunderstanding the debugger just because of that.

Showing a shadow-breakpoint icon while hovering a line would be enough I think (I know it's not easy).

Comment 9

[Mihai Sucan [:msucan]]

I can take this bug. I can add an onmouseover event handler and add a CSS class to indicate you can add breakpoints. Will do this ASAP.

Comment 10

[Mihai Sucan [:msucan]]

Attachment: proposed patch

This patch adds the breakpoint icon, faded, on mouseover for each line in the debugger.

Paul: please let me know if this is fine or if you'd like some changes.

Comment 11

[Victor Porof [:vporof][:vp]]

(In reply to Mihai Sucan [:msucan] from comment #10)
> Created attachment 715198 [details] [diff] [review]
> proposed patch
> 

Clicking in the gutter (on the left-hand side of the line numbers) doesn't seem to add a breakpoint. This is a bit weird, because the faded breakpoint icon has a pointer-styled cursor, and clicking it doesn't do anything. I think it should!

Clicking on a line number does work, but the breakpoint icon remains faded until the mouse is moved over another line. It should immediately become opaque, since a breakpoint *exists* and it's not just *suggested*.

I also expected that hovering on the gutter would highlight the current line (like Firebug does). It's really helpful when lines are long and it's relatively hard to mentally connect "that word on the far right of the editor" and the respective line number, but that's solely just a personal preference :)

Comment 12

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 715198 [details] [diff] [review]
proposed patch

Review of attachment 715198 [details] [diff] [review]:
-----------------------------------------------------------------

the mouseover, slightly transparent breakpoints look good, but they're a bit misleading. Since they show up on every line even if it's not a breakable line, they suggest I can click anywhere to set a breakpoint.

Worse, the mouseover seems to be preventing clicks in our gutter. You have to mouse out to successfully set a breakpoint which is tricky. I seem to be able to set breakpoints if I move out to the line number but can't set one with the suggested breakpoint showing.

And there is no hover text displayed.

::: browser/devtools/sourceeditor/source-editor-orion.jsm
@@ +991,5 @@
> +    let annotation = {
> +      type: ORION_ANNOTATION_TYPES.tempBreakpoint,
> +      start: lineStart,
> +      end: lineEnd,
> +      title: this._strClickToAddBreakpoint,

this doesn't appear to be showing up on hover.

Comment 13

[Paul Rouget [:paul]]

Comment on attachment 715198 [details] [diff] [review]
proposed patch

That's much better. I agree with what Victor said:
> Clicking in the gutter (on the left-hand side of the line numbers) doesn't
> seem to add a breakpoint. This is a bit weird, because the faded breakpoint
> icon has a pointer-styled cursor, and clicking it doesn't do anything.
> I think it should!

Comment 14

[Mihai Sucan [:msucan]]

Attachment: updated patch

Rebased the patch and tried to address review comments and comments from Victor and Paul.

Changes:

1. when you hover a line in the gutter, that line is also highlighted in the editor with (hopefully) a subtle color (better color suggestions are welcome).

2. tried to improve the click behavior.

3. removed the tooltip - it doesn't work with Orion without patching it.


Replies below...


(In reply to Victor Porof [:vp] from comment #11)
> Clicking in the gutter (on the left-hand side of the line numbers) doesn't
> seem to add a breakpoint. This is a bit weird, because the faded breakpoint
> icon has a pointer-styled cursor, and clicking it doesn't do anything. I
> think it should!

This is WFM. I can click the semi-transparent breakpoint to add a breakpoint. It turns into a non-temporary breakpoint for me. I can also click a breakpoint to see it turn into a temporary one.


> Clicking on a line number does work, but the breakpoint icon remains faded
> until the mouse is moved over another line. It should immediately become
> opaque, since a breakpoint *exists* and it's not just *suggested*.

True. I have fixed this case now. When you click on a line number the breakpoint icon turns to full visibility now.


> I also expected that hovering on the gutter would highlight the current line
> (like Firebug does). It's really helpful when lines are long and it's
> relatively hard to mentally connect "that word on the far right of the
> editor" and the respective line number, but that's solely just a personal
> preference :)

I like the idea. Fixed.


(In reply to Rob Campbell [:rc] (:robcee) from comment #12)
> the mouseover, slightly transparent breakpoints look good, but they're a bit
> misleading. Since they show up on every line even if it's not a breakable
> line, they suggest I can click anywhere to set a breakpoint.

This is true, but we get to the question Honza had about adding breakpoints. He also wanted to have some API to tell Firebug which lines accept breakpoints and which do not. This is not something we can fix in this bug.


> Worse, the mouseover seems to be preventing clicks in our gutter. You have
> to mouse out to successfully set a breakpoint which is tricky. I seem to be
> able to set breakpoints if I move out to the line number but can't set one
> with the suggested breakpoint showing.

This is odd. It works for me - I can click at any time, on the line number or on the suggested breakpoint and I get a breakpoint. Please retest with the fixes in this patch.


> And there is no hover text displayed.

True. It seems Orion might be preventing the actual element title tooltip from showing (meh). I removed the string from this patch.


Thank you for the reviews and for the comments. Apologies for the delays - web console work took over my list of priorities.

Comment 15

[Rob Campbell [:rc] (:robcee)]

I'm still seeing the issue with clicks in the gutter not adding a breakpoint. I can add them on the same line with a right click in the text area (which gives a popup menu) but not in the gutter (right clicks do nothing, same behavior as currently).

Comment 16

[Mihai Sucan [:msucan]]

Comment on attachment 777083 [details] [diff] [review]
updated patch

(In reply to Rob Campbell [:rc] (:robcee) from comment #15)
> I'm still seeing the issue with clicks in the gutter not adding a
> breakpoint. I can add them on the same line with a right click in the text
> area (which gives a popup menu) but not in the gutter (right clicks do
> nothing, same behavior as currently).

This bug is about left clicks in the gutter area, not about right-clicks. For right-clicks we have a different bug where we will add an event handler for right-click on the gutter.

Asking for re-review.

Comment 17

[Rob Campbell [:rc] (:robcee)]

This still doesn't work quite right. There's a "hit range" inside the gutter that lets you set a breakpoint (to the right of the ghosted breakpoint button). If you move outside of it, hovering over the ghosted breakpoint icon, the breakpoint doesn't get set.

Comment 18

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 777083 [details] [diff] [review]
updated patch

Review of attachment 777083 [details] [diff] [review]:
-----------------------------------------------------------------

clearing this review request because of the hit problem on OS X.

Comment 19

[Mihai Sucan [:msucan]]

This patch needs to be rewritten for codemirror. Unassigning myself, since I am not currently doing source editor work.

Comment 20

[Bryan Clark (DevTools PM) [:clarkbw]]

Closing this out.  This bug is very old and I'm closing out old bugs that have stalled out.  The idea is interesting and I've added it to the debugger ideas etherpad so it doesn't get lost ( https://public.etherpad-mozilla.org/p/debugger-ideas ) but we're not getting much value from leaving this bug open.