Closed Bug 951828 Opened 9 years ago Closed 9 years ago

Always select the variables tab when pausing and stack frames are available

Categories

(DevTools :: Debugger, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 29

People

(Reporter: vporof, Assigned: vporof)

Details

Attachments

(1 file)

Scenario:

I went to the Events tab and checked an event. Then I did something to the debuggee that triggered that event. I was briefly confused as to why the debugger didn't break, then realized it did, but the Events tab was still selected and my instinct expected to see the Variables tab instead.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Attachment #8349629 - Flags: review?(nfitzgerald)
Comment on attachment 8349629 [details] [diff] [review]
dbg-always-switch.patch

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

What about when:

* I am in the event listener tab and am *not* paused

* I set a breakpoint by clicking in the gutter

* Because we aren't paused, the client interrupts and causes a pause so it can set the BP

* The event listener tab is switched away from, and as a user I am surprised. "Why did an "unrelated" action change tabs??"

-------------------

r+ once you have logic and a test to handle the above case.

::: browser/devtools/debugger/test/browser_dbg_break-on-dom-08.js
@@ +14,5 @@
> +    let gEvents = gView.EventListeners;
> +
> +    Task.spawn(function() {
> +      yield waitForSourceShown(aPanel, ".html");
> +      aDebuggee.addBodyClickEventListener();

Do we need to remove this event listener at the end of the test, or is that being too paranoid?
Attachment #8349629 - Flags: review?(nfitzgerald)
(In reply to Nick Fitzgerald [:fitzgen] from comment #2)
> Comment on attachment 8349629 [details] [diff] [review]
> dbg-always-switch.patch
> 
> Review of attachment 8349629 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> What about when:
> 
> * I am in the event listener tab and am *not* paused
> 
> * I set a breakpoint by clicking in the gutter
> 
> * Because we aren't paused, the client interrupts and causes a pause so it
> can set the BP
> 
> * The event listener tab is switched away from, and as a user I am
> surprised. "Why did an "unrelated" action change tabs??"
> 

Smart thinking. I'll take care of this.

> -------------------
> 
> r+ once you have logic and a test to handle the above case.
> 
> ::: browser/devtools/debugger/test/browser_dbg_break-on-dom-08.js
> @@ +14,5 @@
> > +    let gEvents = gView.EventListeners;
> > +
> > +    Task.spawn(function() {
> > +      yield waitForSourceShown(aPanel, ".html");
> > +      aDebuggee.addBodyClickEventListener();
> 
> Do we need to remove this event listener at the end of the test, or is that
> being too paranoid?

Dude, there are so many event listeners added by this test :) Tab gets anyway, event listeners are added to content, so it doesn't matter.
(In reply to Nick Fitzgerald [:fitzgen] from comment #2)
> 
> * The event listener tab is switched away from, and as a user I am
> surprised. "Why did an "unrelated" action change tabs??"


Wait, this actually doesn't happen. The instruments pane is shown only when we also have stack frames [0]. Do you still want me to write a test for this?

https://mxr.mozilla.org/mozilla-central/source/browser/devtools/debugger/debugger-controller.js#701
Flags: needinfo?(nfitzgerald)
Summary: Always select the variables tab when pausing → Always select the variables tab when pausing and stack frames are available
I'll leave it up to you to decide whether you think it is worth it or not.
Flags: needinfo?(nfitzgerald)
https://hg.mozilla.org/mozilla-central/rev/07b3207166ba
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.