The new pause indicator hides breakpoints

RESOLVED FIXED in Firefox 48

Status

defect
P2
normal
RESOLVED FIXED
3 years ago
10 months ago

People

(Reporter: pbro, Assigned: jlong)

Tracking

unspecified
Firefox 48

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

3 years ago
STR:

- be on fx-team (or nightly, not sure if this has landed), so that you see the new, nicer, breakpoints and pause indicators
- break anywhere, on some line
- start stepping over
- say you are paused at line 80 of your script
- no click in the gutter at line 80 to add a breakpoint here

=> the green arrow that represents the current pause hides the breakpoint indicator that I just added, so I don't know if clicking in the gutter actually worked and added a new breakpoint.

I suggest keeping the line background as green but switching the arrow to blue, so at least you see that adding the breakpoint worked.
Or maybe having a double-state arrow half green and half blue?
(Assignee)

Comment 1

3 years ago
I agree 100% with this. In fact, can we just remove the green arrow? I think it's super confusing that it looks like some kind of special breakpoint. I've been meaning to bring this up. We should just highlight the line that we want without messing with the gutter. Chrome does this as well. Otherwise we'll always be hitting this issue of it covering up breakpoints, and solution like multi-coloring it are just awkward.

I feel pretty strongly about this. Is there a good reason to have a green "breakpoint" in the gutter?
Marking this as P2. It doesn't actually break the debugger experience, but it's a nuisance.

FWIW, I agree with both Patrick and James on this.
Priority: -- → P2
(Assignee)

Comment 3

3 years ago
Posted patch 1245030.patch (obsolete) — Splinter Review
What about this? This reduces the visual clutter a lot. I made it so that we still have the green highlight, but the linenumber also just has a background color of the similar green. The blue breakpoints sit on top of this, so it's clear that your just stepping through that breakpoint.

I also removed the line highlight for the cursor, as I think we already have too many things going on and it help solidify that the highlighted line is what the debugger is paused on.

I see the problem that the designers hit: originally I just removed the breakpoint, but it wasn't entirely clear that the engine was paused on the line. Highlighting the line number helps. Chrome just highlights the line, but they also have an intrusive in-page overlay that makes it extremely clear that the debugger is paused. We want to do that too, but for now I think this solves the existing problem.
(Assignee)

Comment 4

3 years ago
Posted image same-line.png
(Assignee)

Comment 5

3 years ago
Posted image diff-line.png
(Assignee)

Comment 6

3 years ago
Helen I know you're traveling the world but when you get a chance can you take a look? No rush. We can always uplift this easily if we feel the need.
Assignee: nobody → jlong
Flags: needinfo?(hholmes)
I believe highlighting the line where the script execution stands is not enough. I believe there should also be some arrow indicating the line. I could imagine an arrow that only covers the right part besides the gutter like the Safari DevTools do it.[1]

Another solution would be to make the arrow bigger than the breakpoint and only show it outside the gutter. See the attached quick mockup.

Sebastian

[1] https://developer.apple.com/safari/images/5_Debugger.jpg
(Assignee)

Comment 8

3 years ago
Why are there two green arrows there? There should only ever be one: the place that the debugger is currently paused on.

Like I said, highlighting the line will be enough when we also show an overlay on the page that clearly indicates the debugger is paused. However until then we probably should do something more.
I like your multi-line solution, James—after playing with it I think it makes it pretty clear, and I like how it stays clean and unobtrusive.
Flags: needinfo?(hholmes)
(Assignee)

Comment 10

3 years ago
Posted patch 1245030.patchSplinter Review
New patch, similar to my screenshots but with a few tweaks. The main thing is I didn't remove the active line highlight because that affects other tools as well.

Helen has approved this so let's push this. Any tweaks can be made in the future; the current UX is really bad so we just need to fix it.
Attachment #8726411 - Attachment is obsolete: true

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/47ca980cf2a5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.