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

RESOLVED FIXED in Firefox 18

Status

P3
normal
RESOLVED FIXED
6 years ago
4 months ago

People

(Reporter: vporof, Assigned: vporof)

Tracking

unspecified
Firefox 18

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 660495 [details] [diff] [review]
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).
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Attachment #660495 - Flags: review?(past)
(Assignee)

Comment 2

6 years ago
(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?)
(Assignee)

Updated

6 years ago
Priority: -- → P3
(Assignee)

Comment 3

6 years ago
(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 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?
Attachment #660495 - Flags: review?(past)
(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?
(Assignee)

Comment 6

6 years ago
(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();
(Assignee)

Comment 7

6 years ago
Way to grammar, Victor.
s/when the on the firs/when hitting the first/
(Assignee)

Comment 8

6 years ago
(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.
(Assignee)

Comment 9

6 years ago
(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?
(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.
(Assignee)

Comment 12

6 years ago
(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/vporof@mozilla.com-7cdf3a8d6faa/
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 on attachment 660495 [details] [diff] [review]
v1

r=me with the changes mentioned in comment 13
Attachment #660495 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/3c95dca80f3f
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 18

Updated

6 years ago
Depends on: 782877
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

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 18

6 years ago
(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.
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).
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! :-)
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
No longer depends on: 782877
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 885591

Updated

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