790650 - It may be a good idea to have the debugger start with collapsed panels

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P3)

Description

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

Since there's no breakpoints/stackframes/variables information to show there yet anyway, and they take quite a lot of space.

Of course, we should verify user settings, check for previous user interaction etc.

Comment 1

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

Attachment: v1

I think this is a SMASHING idea! It works really well in cases like Inspector + Debugger both open (since the styles panel occupies a lot of the debugger's source editor space on smaller screens).

Comment 2

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

(not sure if we want to automatically show the debugger+stackframes pane on the left when the user adds a breakpoint; Panos what do you think?)

Comment 3

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

(another possibly good note from irc: it may be a good idea to do this only when the browser with is too low, since the only scenarios in which the debugger looks really bad afaict is when the inspector is open with the sidebar showing and the browser is less than ~1000px wide; however, we should be cautious about making the behavior too "magic")

Comment 4

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

Comment on attachment 660495 [details] [diff] [review]
v1

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

After playing with it for a while, I think it's a nice idea, but I'm worried about the hampered discoverability. Inexperienced users may look at the editor and wonder about the missing functionality. I suggest you create a try build so we can ask people to give it a try and tell us what they think. Maybe we could even get Paul to tweet about it.

I also discovered that with this patch the "fullscreen" button tooltips are now reversed: it shows the expander tooltip when the collapse icon is displayed and vice versa.

::: browser/themes/gnomestripe/devtools/debugger.css
@@ +4,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #body {
> +  background-color: white;

Why this change?

Comment 5

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

(In reply to Victor Porof [:vp] from comment #2)
> (not sure if we want to automatically show the debugger+stackframes pane on
> the left when the user adds a breakpoint; Panos what do you think?)

If you suggest we show *only* the left pane in that case, then I don't think that is a good idea. With the single toggle we have provided the user with, it would seem like a half-fullscreen state, which he has no way to go to. And a reasonable argument would then be that if we consider having only one pane collapsed a reasonable state, then why are we not providing UI to toggle that state?

Comment 6

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

(In reply to Panos Astithas [:past] from comment #5)
> (In reply to Victor Porof [:vp] from comment #2)
> > (not sure if we want to automatically show the debugger+stackframes pane on
> > the left when the user adds a breakpoint; Panos what do you think?)
> 
> If you suggest we show *only* the left pane in that case, then I don't think
> that is a good idea. With the single toggle we have provided the user with,
> it would seem like a half-fullscreen state, which he has no way to go to.
> And a reasonable argument would then be that if we consider having only one
> pane collapsed a reasonable state, then why are we not providing UI to
> toggle that state?

Nope, I don't want to break the single-fullscreen-button paradigm.

I'm talking about showing both panes when the user adds a breakpoint (the current functionality in the patch). Somehow i feel like this is making the UI to eager to show the panes.

For comparison, this doesn't happen in any IDEs I've ever had a chance to mess with, nor Firebug or other developer tools, they all hide a list of breakpoints under a menu item or command etc. Also, the left pane (i.e stackframes+breakpoints) is automatically shown when hitting the first breakpoint anyway.

So my alternate suggestion is: just automatically show the panes when the on the firs stackframe. If the user wants to edit the currently added breakpoints, he just toggles the panes.

This change reflects in just removing these two lines from the patch:
+    // Breakpoints are UI elements which benefit from visible panes.
+    DebuggerView.showPanesIfAllowed();

Comment 7

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

Way to grammar, Victor.
s/when the on the firs/when hitting the first/

Comment 8

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

(In reply to Panos Astithas [:past] from comment #4)
> Comment on attachment 660495 [details] [diff] [review]
> v1
> 
> Review of attachment 660495 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> After playing with it for a while, I think it's a nice idea, but I'm worried
> about the hampered discoverability. Inexperienced users may look at the
> editor and wonder about the missing functionality. I suggest you create a
> try build so we can ask people to give it a try and tell us what they think.
> Maybe we could even get Paul to tweet about it.

That's a great idea, let me fire up try.

> 
> ::: browser/themes/gnomestripe/devtools/debugger.css
> @@ +4,5 @@
> >   * License, v. 2.0. If a copy of the MPL was not distributed with this
> >   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> >  
> >  #body {
> > +  background-color: white;
> 
> Why this change?

Now that the editor occupies the entire debugger UI on first run, having an almost  significantly different color results in a brief perceived flicker. A white background manages to avoid this perception.

Comment 9

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

(In reply to Panos Astithas [:past] from comment #4)
> 
> I also discovered that with this patch the "fullscreen" button tooltips are
> now reversed: it shows the expander tooltip when the collapse icon is
> displayed and vice versa.
> 

Hmm, I can't seem to reproduce this.
How did you get in such a state? What were your initial stackframes-pane-visible and variables-pane-visible prefs before starting the debugger?

Comment 10

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

(In reply to Victor Porof [:vp] from comment #6)
> (In reply to Panos Astithas [:past] from comment #5)
> > (In reply to Victor Porof [:vp] from comment #2)
> > > (not sure if we want to automatically show the debugger+stackframes pane on
> > > the left when the user adds a breakpoint; Panos what do you think?)
> > 
> > If you suggest we show *only* the left pane in that case, then I don't think
> > that is a good idea. With the single toggle we have provided the user with,
> > it would seem like a half-fullscreen state, which he has no way to go to.
> > And a reasonable argument would then be that if we consider having only one
> > pane collapsed a reasonable state, then why are we not providing UI to
> > toggle that state?
> 
> Nope, I don't want to break the single-fullscreen-button paradigm.
> 
> I'm talking about showing both panes when the user adds a breakpoint (the
> current functionality in the patch). Somehow i feel like this is making the
> UI to eager to show the panes.

Ah, OK. Agreed then. It's not much use showing the breakpoint list when you add a breakpoint.

Comment 11

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

Builds can be found here
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fd249c304a76/

Comment 12

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

(In reply to Panos Astithas [:past] from comment #10)
> (In reply to Victor Porof [:vp] from comment #6)
> > (In reply to Panos Astithas [:past] from comment #5)
> > > (In reply to Victor Porof [:vp] from comment #2)
> > > > (not sure if we want to automatically show the debugger+stackframes pane on
> > > > the left when the user adds a breakpoint; Panos what do you think?)
> > > 
> > > If you suggest we show *only* the left pane in that case, then I don't think
> > > that is a good idea. With the single toggle we have provided the user with,
> > > it would seem like a half-fullscreen state, which he has no way to go to.
> > > And a reasonable argument would then be that if we consider having only one
> > > pane collapsed a reasonable state, then why are we not providing UI to
> > > toggle that state?
> > 
> > Nope, I don't want to break the single-fullscreen-button paradigm.
> > 
> > I'm talking about showing both panes when the user adds a breakpoint (the
> > current functionality in the patch). Somehow i feel like this is making the
> > UI to eager to show the panes.
> 
> Ah, OK. Agreed then. It's not much use showing the breakpoint list when you
> add a breakpoint.

And there are builds that take ^ that ^ comment into consideration.
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-7cdf3a8d6faa/

Comment 13

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

So, after talking about it with Victor some more, I think this feature makes sense and my fears were overrated. Let's land this (with the change mentioned in comment 12) and give people a chance to play with it.

Comment 14

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

Comment on attachment 660495 [details] [diff] [review]
v1

r=me with the changes mentioned in comment 13

Comment 15

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

https://hg.mozilla.org/integration/fx-team/rev/3c95dca80f3f

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/3c95dca80f3f

Comment 17

[Ed Morley [:emorley]]

Sorry backed this out for causing our current #1 top orange (has double the failure rate of the #2 orange), bug 782877.

Backout:
https://hg.mozilla.org/mozilla-central/rev/74c6fc2c4ad1

Comment 18

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

(In reply to Ed Morley [:edmorley UTC+1] from comment #17)
> Sorry backed this out for causing our current #1 top orange (has double the
> failure rate of the #2 orange), bug 782877.
> 
> Backout:
> https://hg.mozilla.org/mozilla-central/rev/74c6fc2c4ad1

It's.. highly unlikely that this patch was the cause of those oranges.

Comment 19

[Ed Morley [:emorley]]

Is there anything else in the timeframe that could be the cause?

Just getting incredibly frustrated by bug 782877. Alternatively we can reland this and use ignoreAllUncaughtExceptions() for bug instead (per my request in bug 782877 comment 49).

Comment 20

[Ed Morley [:emorley]]

Ok so bug 782877 occurred again after the backout of this, so have relanded:
https://hg.mozilla.org/mozilla-central/rev/9e8a91dfbc7e

Sorry for the churn! :-)