Closed Bug 893692 Opened 6 years ago Closed 6 years ago

The continue/stepping buttons in the debugger are very confusing when paused

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 28

People

(Reporter: vporof, Assigned: vporof)

References

Details

Attachments

(1 file)

I recently attended a Firefox OS hackathon and there were people making apps. Of those, around 20 or so played used the debugger.

There was extreme general confusion as to what the "play" button does. Almost everyone thought that it "runs code". So their workflow using the debugger was:
1. Add some breakpoints
2. Click the play button and expect the code to run in a way that results in those breakpoints being hit
3. Get confused
4. Start clicking on the page and get more confused and blaming Firefox for becoming unresponsive

What was actually happening:
1. Breakpoints were added
2. Debuggee was paused
3. People got confused as to
3.1. Why didn't the "code run?"
3.2. Why does interacting with the debuggee (clicking etc.) not work anymore

We used to have bug 766051 for the "play" button state being confusing, but I assert that the button itself, not only the pressed/unpressed state is being confusing. Moreover, it seems like the stepping buttons are also pretty useless while the debuggee is not paused, so it sounds to me like there are the following ways that we can (and really, really should!) alleviate the issue:

1. Hide the pause/stepping buttons while the debuggee is not paused
2. Disable them while the debuggee is not paused
3. Hide only the play button
4. Use a different icon (pause) and remove the checked state for the resume button

I think a combination of 2 and 4 is what we need. Hell, even just doing 2 would still be okay. In any case, we really need to do something about this because it looked like almost nobody was able to use our debugger because they were pausing the debuggee before getting a chance to do anything else :)

Panos what do you think about it?
Interesting. I wonder if those people had used any other debugger before, since that is precisely how they all work.

The good news is that bug 789430 should fix the main problem these users were facing, by no longer pausing execution without any visual indication (like a stack frame list or a populated variables view).

I still think we should make the change we agreed on in bug 766051, which amounts to item 4 in your list (regardless of whether we remove the checked state or not).

I also agree with item 2 and would even suggest that we should disable any button that can't function in the current state. I don't think hiding buttons is a good idea though, because people get frustrated when UI elements disappear for reasons they don't comprehend.
+1 for these changes from me as well, although I could live with hiding the buttons. It can be nice to remove clutter and as long as we do the Right Thing(tm) (as expected by users) they shouldn't get any state mismatch frustration.
(In reply to Panos Astithas [:past] from comment #1)
> Interesting. I wonder if those people had used any other debugger before,
> since that is precisely how they all work.
> 

That's a valid point, but if you take into consideration IDEs like Visual Studio or Eclipse, a "play" button usually means "build", and it's located right near the stepping buttons. So, in this case, users mistaking our interrupt/resume button for being responsible with "running code" is not that surprising.

Do we have a "pause" icon laying around somewhere? I seem to recall using something like that about 5000 years ago in our debugger.
(In reply to Victor Porof [:vp] from comment #3)
> Do we have a "pause" icon laying around somewhere? I seem to recall using
> something like that about 5000 years ago in our debugger.

Yup, e.g.: browser/themes/osx/devtools/debugger-pause.png
(In reply to Panos Astithas [:past] from comment #4)
> (In reply to Victor Porof [:vp] from comment #3)
> > Do we have a "pause" icon laying around somewhere? I seem to recall using
> > something like that about 5000 years ago in our debugger.
> 
> Yup, e.g.: browser/themes/osx/devtools/debugger-pause.png

Love it!
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Duplicate of this bug: 766051
(In reply to Panos Astithas [:past] from comment #1)
> 
> The good news is that bug 789430 should fix the main problem these users
> were facing, by no longer pausing execution without any visual indication
> (like a stack frame list or a populated variables view).
> 

Does that mean that we shouldn't disable the resume button while resumed anymore? I'm imagining yes, but I may be mistaken.
Priority: -- → P2
(In reply to Victor Porof [:vp] from comment #7)
> (In reply to Panos Astithas [:past] from comment #1)
> > 
> > The good news is that bug 789430 should fix the main problem these users
> > were facing, by no longer pausing execution without any visual indication
> > (like a stack frame list or a populated variables view).
> > 
> 
> Does that mean that we shouldn't disable the resume button while resumed
> anymore? I'm imagining yes, but I may be mistaken.

I'm not sure what you mean, we don't currently disable any button, do we?
+1 to this as well, and I can remember having the same exact initial experience full of confusion when first trying the debugger.

As far as hiding vs. disabling the button, I think either they need to be hidden, or the pause/play button needs to be positioned separately (maybe just a separator) from the other buttons. Having them as one contiguous block, even if they are disabled, is still confusing because it makes it seem like they perform similar functions. The play/pause button has a very distinct functionality from the step buttons and that should be represented visually.
(In reply to Panos Astithas [:past] from comment #8)
> 
> I'm not sure what you mean, we don't currently disable any button, do we?

We don't. In light of bug 789430, should we still disable it?
+1 for disabling the stepping buttons, don't hide them.
(In reply to Victor Porof [:vp] from comment #10)
> (In reply to Panos Astithas [:past] from comment #8)
> > 
> > I'm not sure what you mean, we don't currently disable any button, do we?
> 
> We don't. In light of bug 789430, should we still disable it?

I don't think we should disable the play/pause button at all, because it's always useful, since it changes functionality based on the current state (it will pause when running, and resume when paused). In your item 4 you mentioned removing the checkbox styling, for which I'm -0.5, because I think it gives a stronger indication of the current state compared to a single icon on a button (using chrome in parallel makes this more evident).

One case where disabling the pause button may be helpful, is after we implement bug 789430, where the new break-on-next behavior will make clicking it again before execution is actually paused, useless. That is, click once to break-on-next, and while waiting for execution to actually pause, click again for whatever reason. That said, I'm still not convinced that it will be a net win even then, because the user might be confused thinking that the debugger is busy or thrashing, having no other indication of activity or state change. Chrome does exactly that however, so maybe users are trained to expect it and my worries are exaggerated.
(In reply to Panos Astithas [:past] from comment #12)
> (In reply to Victor Porof [:vp] from comment #10)
> > (In reply to Panos Astithas [:past] from comment #8)
> > > 
> > > I'm not sure what you mean, we don't currently disable any button, do we?
> > 
> > We don't. In light of bug 789430, should we still disable it?
> 
> I don't think we should disable the play/pause button at all, because it's
> always useful, since it changes functionality based on the current state (it
> will pause when running, and resume when paused). In your item 4 you
> mentioned removing the checkbox styling, for which I'm -0.5, because I think
> it gives a stronger indication of the current state compared to a single
> icon on a button (using chrome in parallel makes this more evident).
> 

I don't think there's any actual use case for pausing when running. At the moment, "pausing" turns the debuggee into "paused" immediately, after which the debugger is pretty much useless if you don't unpause. Most users are having trouble understanding that.

Bug 789430 makes it worthwhile, because "pausing" triggers "pause on next", instead of "immediately paused", if you see what I mean. However, until then, pausing is just a pain in the butt :)

So what I'm trying to get at is: should we disable the 4 pause/stepping buttons altogether now, and undo the behavior for the pause button after/along with bug 789430, or would it be better to leave the pause button alone for now and hope for bug 789430 landing soon? I would vote for the former suggestion.
(In reply to Victor Porof [:vp] from comment #13)
> I don't think there's any actual use case for pausing when running. At the
> moment, "pausing" turns the debuggee into "paused" immediately, after which
> the debugger is pretty much useless if you don't unpause. Most users are
> having trouble understanding that.
> 
> Bug 789430 makes it worthwhile, because "pausing" triggers "pause on next",
> instead of "immediately paused", if you see what I mean. However, until
> then, pausing is just a pain in the butt :)

I've actually found it useful to stop timers from firing and messing the page while trying to inspect something. Other than that, I think we are in violent agreement here :-)

> So what I'm trying to get at is: should we disable the 4 pause/stepping
> buttons altogether now, and undo the behavior for the pause button
> after/along with bug 789430, or would it be better to leave the pause button
> alone for now and hope for bug 789430 landing soon? I would vote for the
> former suggestion.

Bug 789430 has been waiting on review for over a month now, but other than that it should be pretty close to done (tests and the UI bits are all that remains). But beyond that, I don't think it's a good idea to change a user-facing feature twice in short succession because it creates confusion (which usually leads to irksome blog posts). Imagine if we land these changes a week apart, and this one gets in 25 while the other one ends up in 26.
(In reply to Panos Astithas [:past] from comment #14)

Agreed. I was a bit afraid of keeping the current behavior for any longer, but it seems like bug 789430 should get some attention soon, so my worries aren't really justified.
Attached patch dbg-button.patchSplinter Review
As per comment 1.
Attachment #8342997 - Flags: review?(past)
Comment on attachment 8342997 [details] [diff] [review]
dbg-button.patch

Nice effects!
Attachment #8342997 - Flags: review?(past) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/68261fd312b0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Keywords: verifyme
User agents:
[1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
[2] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
[3] Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:30.0) Gecko/20100101 Firefox/30.0

I was able to confirm the fix for this issue on:
- the latest Beta (Build ID: 20140210161136) [1]
- the latest Aurora (Build ID: 20140210004002) [2]
- the latest Nightly (Build ID: 20140210030201) [3]
using Windows 7 64-bit, Mac OS X 10.8.5 and Ubuntu 13.10 64-bit.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.